lkml.org 
[lkml]   [2007]   [Jun]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On Fri, 29 Jun 2007, Ulrich Drepper wrote:

> On 6/29/07, Davide Libenzi <davidel@xmailserver.org> wrote:
> > [include/linux/indirect.h]
> > #define SYSIND_CTX_OPENFLAGS 0
> > struct sysind_ctx_OPENFLAGS {
> > __u32 ctx;
> > __u32 flags;
>
> I agree that this interface is more than any other in danger of
> needing an interface change. But I think your solution is a bit too
> expensive and complex. You need two reads from userlevel.

But, a __get_user(), once you scrap off all the gcc wrapping, is bacially
a move. That could even be removed, but really I don't see the reason
since it allows for a cleaner strcture definition in userland.




> The standard way to handle this is a versioned struct. I.e., define a
> struct for the current needs, define an initial version. To use the
> syscall pass the version number and the struct pointer to the syscall.
> If the kernel doesn't know the version number it fails. Otherwise it
> might have to read old versions of the struct which is trivial to do.
> E.g.:
>
> #define SYSIND_VERSION 1
> #define SYSIND_CTX_OPENFLAGS 0
> #define SYSIND_CTX_SIGMASK 1
> struct sysind_ctx {
> int ctx;
> union {
> int flags;
> kernel_sigset_t sset;
> };
> };

But this does not allow more than one context to be set at a time, like
the current implementation does. Ie, the current implementation allow you
to:

#define SYSIND_CTX_OPENFLAGS 0
struct sysind_ctx_OPENFLAGS {
__u32 ctx;
__u32 flags;
};

#define SYSIND_CTX_SIGSET 1
struct sysind_ctx_SIGSET {
__u32 ctx;
__compat_sigset set;
};

struct sysind_ctx_OPENFLAGS octx;
struct sysind_ctx_SIGSET sctx;
struct indirect_ctx *ctxs[2];
unsigned long params[6];

octx.ctx = SYSIND_CTX_OPENFLAGS;
octx.flags = O_CLOEXEC;
sctx.ctx = SYSIND_CTX_SIGSET;
sctx.set = SIG_XYZ | SIG_ABC;
ctxs[0] = (struct indirect_ctx *) &octx;
ctxs[1] = (struct indirect_ctx *) &sctx;
params[0] = blah;
params[1] = blew;
...
res = indirect(__NR_xxxx, ctxs, 2, params);


So you basically keep the context strcture separated, and allow to pass
down more then one context to be set.
Another solution would be to have:

struct sysind_ctx_OPENFLAGS {
__u32 flags;
};
struct sysind_ctx_SIGSET {
__compat_sigset set;
};
struct monster_struct {
__u32 size;
__u64 flags;
struct sysind_ctx_OPENFLAGS octx;
struct sysind_ctx_SIGSET sctx;
...
};

Where size gives you the size of the monster structure, and every bit on
flags tells you which strcture inside monster is valid.
But I'd clearly prefer the former.



- Davide


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-06-30 21:25    [W:0.047 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site