Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction

On Fri, 29 Jun 2007, Ulrich Drepper wrote:

On 6/29/07, Davide Libenzi <davidel@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages