Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On Mon, 2 Jul 2007, Ulrich Drepper wrote: > > But there are > > examples (and the signal stuff is one of them), where you do need the > > set_context+syscall+unset_context abstraction, for all cases where the > > kernel already has its own internal data strctures. In those cases you'd > > have to spread sys_internal context knowledge all around the kernel, > > whereas the current solution allows you to confine the code inside > > kernel/indirect.c > > Nonsense. Whether you have a new word in the task structure or a > pointer to a structure, you have to embed knowledge of this > indirection in every affected code paths. There is no difference > here. The only difference is that trying to force everything through > an artificially complicated common entry code. > > I hope that Andrew+Linus will see through this. I'm done arguing. I simply asked you to look how the code/patch would look like to handle the ppoll/pselect/epoll_pwait stuff, and precisely where your changes had to go. Or even in the exmaple Linus made of "make the fsuid/fsgid temporarily be my _real_ uid/gid for this single system call" for what it matters. - Davide - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On 7/2/07, Davide Libenzi <[EMAIL PROTECTED]> wrote: Never be usable? I made you a concrete example that is like 8 months old. And *that* could not have been cleanly handled with the flat structure idea. First of all, sigmasks are not widely needed. Second, why on earth shouldn't it be possible with a flat structure? Just have the sigmask in the same position. It's no magic. But there are examples (and the signal stuff is one of them), where you do need the set_context+syscall+unset_context abstraction, for all cases where the kernel already has its own internal data strctures. In those cases you'd have to spread sys_internal context knowledge all around the kernel, whereas the current solution allows you to confine the code inside kernel/indirect.c Nonsense. Whether you have a new word in the task structure or a pointer to a structure, you have to embed knowledge of this indirection in every affected code paths. There is no difference here. The only difference is that trying to force everything through an artificially complicated common entry code. I hope that Andrew+Linus will see through this. I'm done arguing. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On Mon, 2 Jul 2007, Ulrich Drepper wrote: > On 7/1/07, Davide Libenzi <[EMAIL PROTECTED]> wrote: > > With the current API design you'd able to easily confine the "pre" code > > inside the "set" function, and the "post" code inside the "unset" > > function. It looks pretty clean to me, and allows to limit the knowledge > > of sys_indirect, the more as possible inside kernel/indirect.c. > > But this will not be applicable. We already discussed that each > syscall likely needs its own set of flags etc. There really isn't > much overlap if any which cannot be handled at least as well using a > flat structure. You're adding major complications for something which > IMO will never be usable. With the flat structure to whole overhead > of sys_indirect is limited to a test for valid syscalls, copying the > struct, making the call to the syscall function, and resetting the > value in current. Very simple and fast. Never be usable? I made you a concrete example that is like 8 months old. And *that* could not have been cleanly handled with the flat structure idea. The extra flags parameter is one example where we'd need an extra flags field in the task_struct in any case. So you need in any case code that does extra checks and merges normal parameters/flags with the shared context ones. This independently of the method used. But there are examples (and the signal stuff is one of them), where you do need the set_context+syscall+unset_context abstraction, for all cases where the kernel already has its own internal data strctures. In those cases you'd have to spread sys_internal context knowledge all around the kernel, whereas the current solution allows you to confine the code inside kernel/indirect.c (through the set/unset abstraction). And this w/out even try to hit the weak spot of about how this structure will look after a few additions. - Davide - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On 7/1/07, Davide Libenzi <[EMAIL PROTECTED]> wrote: With the current API design you'd able to easily confine the "pre" code inside the "set" function, and the "post" code inside the "unset" function. It looks pretty clean to me, and allows to limit the knowledge of sys_indirect, the more as possible inside kernel/indirect.c. But this will not be applicable. We already discussed that each syscall likely needs its own set of flags etc. There really isn't much overlap if any which cannot be handled at least as well using a flat structure. You're adding major complications for something which IMO will never be usable. With the flat structure to whole overhead of sys_indirect is limited to a test for valid syscalls, copying the struct, making the call to the syscall function, and resetting the value in current. Very simple and fast. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On 7/1/07, Davide Libenzi [EMAIL PROTECTED] wrote: With the current API design you'd able to easily confine the pre code inside the set function, and the post code inside the unset function. It looks pretty clean to me, and allows to limit the knowledge of sys_indirect, the more as possible inside kernel/indirect.c. But this will not be applicable. We already discussed that each syscall likely needs its own set of flags etc. There really isn't much overlap if any which cannot be handled at least as well using a flat structure. You're adding major complications for something which IMO will never be usable. With the flat structure to whole overhead of sys_indirect is limited to a test for valid syscalls, copying the struct, making the call to the syscall function, and resetting the value in current. Very simple and fast. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On Mon, 2 Jul 2007, Ulrich Drepper wrote: On 7/1/07, Davide Libenzi [EMAIL PROTECTED] wrote: With the current API design you'd able to easily confine the pre code inside the set function, and the post code inside the unset function. It looks pretty clean to me, and allows to limit the knowledge of sys_indirect, the more as possible inside kernel/indirect.c. But this will not be applicable. We already discussed that each syscall likely needs its own set of flags etc. There really isn't much overlap if any which cannot be handled at least as well using a flat structure. You're adding major complications for something which IMO will never be usable. With the flat structure to whole overhead of sys_indirect is limited to a test for valid syscalls, copying the struct, making the call to the syscall function, and resetting the value in current. Very simple and fast. Never be usable? I made you a concrete example that is like 8 months old. And *that* could not have been cleanly handled with the flat structure idea. The extra flags parameter is one example where we'd need an extra flags field in the task_struct in any case. So you need in any case code that does extra checks and merges normal parameters/flags with the shared context ones. This independently of the method used. But there are examples (and the signal stuff is one of them), where you do need the set_context+syscall+unset_context abstraction, for all cases where the kernel already has its own internal data strctures. In those cases you'd have to spread sys_internal context knowledge all around the kernel, whereas the current solution allows you to confine the code inside kernel/indirect.c (through the set/unset abstraction). And this w/out even try to hit the weak spot of about how this structure will look after a few additions. - Davide - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On 7/2/07, Davide Libenzi [EMAIL PROTECTED] wrote: Never be usable? I made you a concrete example that is like 8 months old. And *that* could not have been cleanly handled with the flat structure idea. First of all, sigmasks are not widely needed. Second, why on earth shouldn't it be possible with a flat structure? Just have the sigmask in the same position. It's no magic. But there are examples (and the signal stuff is one of them), where you do need the set_context+syscall+unset_context abstraction, for all cases where the kernel already has its own internal data strctures. In those cases you'd have to spread sys_internal context knowledge all around the kernel, whereas the current solution allows you to confine the code inside kernel/indirect.c Nonsense. Whether you have a new word in the task structure or a pointer to a structure, you have to embed knowledge of this indirection in every affected code paths. There is no difference here. The only difference is that trying to force everything through an artificially complicated common entry code. I hope that Andrew+Linus will see through this. I'm done arguing. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On Mon, 2 Jul 2007, Ulrich Drepper wrote: But there are examples (and the signal stuff is one of them), where you do need the set_context+syscall+unset_context abstraction, for all cases where the kernel already has its own internal data strctures. In those cases you'd have to spread sys_internal context knowledge all around the kernel, whereas the current solution allows you to confine the code inside kernel/indirect.c Nonsense. Whether you have a new word in the task structure or a pointer to a structure, you have to embed knowledge of this indirection in every affected code paths. There is no difference here. The only difference is that trying to force everything through an artificially complicated common entry code. I hope that Andrew+Linus will see through this. I'm done arguing. I simply asked you to look how the code/patch would look like to handle the ppoll/pselect/epoll_pwait stuff, and precisely where your changes had to go. Or even in the exmaple Linus made of make the fsuid/fsgid temporarily be my _real_ uid/gid for this single system call for what it matters. - Davide - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On Sun, 1 Jul 2007, Ulrich Drepper wrote: > On 6/30/07, Davide Libenzi <[EMAIL PROTECTED]> wrote: > > This is how all those overloaded syscalls looks like, BTW: > > [...] > > How would you do that with a single shared strcture, w/out adding in all > > signal paths the knowledge of the structure? > > You said it yourself: each individual wrapper would look like this. > Generalization really isn't possible, you'll have each wrapper syscall > looking different. This means there is no reason to try coming up > with some overly complicated data structure which would only be useful > if the processing of that data structure could be centralized. With the current API design you'd able to easily confine the "pre" code inside the "set" function, and the "post" code inside the "unset" function. It looks pretty clean to me, and allows to limit the knowledge of sys_indirect, the more as possible inside kernel/indirect.c. - Davide - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On 6/30/07, Davide Libenzi <[EMAIL PROTECTED]> wrote: This is how all those overloaded syscalls looks like, BTW: [...] How would you do that with a single shared strcture, w/out adding in all signal paths the knowledge of the structure? You said it yourself: each individual wrapper would look like this. Generalization really isn't possible, you'll have each wrapper syscall looking different. This means there is no reason to try coming up with some overly complicated data structure which would only be useful if the processing of that data structure could be centralized. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On 6/30/07, Davide Libenzi [EMAIL PROTECTED] wrote: This is how all those overloaded syscalls looks like, BTW: [...] How would you do that with a single shared strcture, w/out adding in all signal paths the knowledge of the structure? You said it yourself: each individual wrapper would look like this. Generalization really isn't possible, you'll have each wrapper syscall looking different. This means there is no reason to try coming up with some overly complicated data structure which would only be useful if the processing of that data structure could be centralized. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On Sun, 1 Jul 2007, Ulrich Drepper wrote: On 6/30/07, Davide Libenzi [EMAIL PROTECTED] wrote: This is how all those overloaded syscalls looks like, BTW: [...] How would you do that with a single shared strcture, w/out adding in all signal paths the knowledge of the structure? You said it yourself: each individual wrapper would look like this. Generalization really isn't possible, you'll have each wrapper syscall looking different. This means there is no reason to try coming up with some overly complicated data structure which would only be useful if the processing of that data structure could be centralized. With the current API design you'd able to easily confine the pre code inside the set function, and the post code inside the unset function. It looks pretty clean to me, and allows to limit the knowledge of sys_indirect, the more as possible inside kernel/indirect.c. - Davide - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On Sat, 30 Jun 2007, Davide Libenzi wrote:
> Think about if we had this for the latest pselect/ppoll/epoll_pwait.
> Think about how your solution and mine would apply to that very much
> concrete case.
This is how all those overloaded syscalls looks like, BTW:
if (sigmask) {
if (sigsetsize != sizeof(sigset_t))
return -EINVAL;
if (copy_from_user(, sigmask, sizeof(ksigmask)))
return -EFAULT;
sigdelsetmask(, sigmask(SIGKILL) | sigmask(SIGSTOP));
sigprocmask(SIG_SETMASK, , );
}
error = sys_XXX(...);
if (sigmask) {
if (error == -EINTR) {
memcpy(>saved_sigmask, ,
sizeof(sigsaved));
set_thread_flag(TIF_RESTORE_SIGMASK);
} else
sigprocmask(SIG_SETMASK, , NULL);
}
How would you do that with a single shared strcture, w/out adding in all
signal paths the knowledge of the structure?
- Davide
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On Sat, 30 Jun 2007, Ulrich Drepper wrote: > On 6/30/07, Davide Libenzi <[EMAIL PROTECTED]> wrote: > > 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. > > Don't generalize. The 4G/4G kernel, for instance, doesn't have this > property. Who knows what else will come up. Anyway, two memcpy are > slower than one. > > > 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: > > Of course it does. It just requires an appropriate union element. > I've listed flags and sigset_t as separate union elements since I > cannot think of a place where we need both extensions. Should there > be one this can easily be changed. Why do you want to stick everything inside a structure, with nested strctures and unions, when they clearly are separate contexts? Also, unions don't work if you want to pass multiple contexts at a time. You need your structure to contains *all* the possible contexts. You do not want to sell the monster-struct-union by branding an extra __get_user(), do you? On top of that, the single pointer to the compat multi-context strcture, will force the kernel to decide which one of the members is actually valid or not. The set/unset callbacks separate each context from a structure POV and from a setup/teardown POV, and allow the kernel to use their native data structures. Think about if we had this for the latest pselect/ppoll/epoll_pwait. Think about how your solution and mine would apply to that very much concrete case. - Davide - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On 6/30/07, Davide Libenzi <[EMAIL PROTECTED]> wrote: 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. Don't generalize. The 4G/4G kernel, for instance, doesn't have this property. Who knows what else will come up. Anyway, two memcpy are slower than one. 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: Of course it does. It just requires an appropriate union element. I've listed flags and sigset_t as separate union elements since I cannot think of a place where we need both extensions. Should there be one this can easily be changed. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On Fri, 29 Jun 2007, Ulrich Drepper wrote:
> On 6/29/07, Davide Libenzi <[EMAIL PROTECTED]> wrote:
> > [include/linux/indirect.h]
> > #define SYSIND_CTX_OPENFLAGS0
> > 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_OPENFLAGS0
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 *)
ctxs[1] = (struct indirect_ctx *)
params[0] = blah;
params[1] = blew;
...
res = indirect(__NR_, 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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On Fri, 29 Jun 2007, Ulrich Drepper wrote:
On 6/29/07, Davide Libenzi [EMAIL PROTECTED] wrote:
[include/linux/indirect.h]
#define SYSIND_CTX_OPENFLAGS0
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_OPENFLAGS0
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_, 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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On 6/30/07, Davide Libenzi [EMAIL PROTECTED] wrote: 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. Don't generalize. The 4G/4G kernel, for instance, doesn't have this property. Who knows what else will come up. Anyway, two memcpy are slower than one. 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: Of course it does. It just requires an appropriate union element. I've listed flags and sigset_t as separate union elements since I cannot think of a place where we need both extensions. Should there be one this can easily be changed. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On Sat, 30 Jun 2007, Ulrich Drepper wrote: On 6/30/07, Davide Libenzi [EMAIL PROTECTED] wrote: 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. Don't generalize. The 4G/4G kernel, for instance, doesn't have this property. Who knows what else will come up. Anyway, two memcpy are slower than one. 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: Of course it does. It just requires an appropriate union element. I've listed flags and sigset_t as separate union elements since I cannot think of a place where we need both extensions. Should there be one this can easily be changed. Why do you want to stick everything inside a structure, with nested strctures and unions, when they clearly are separate contexts? Also, unions don't work if you want to pass multiple contexts at a time. You need your structure to contains *all* the possible contexts. You do not want to sell the monster-struct-union by branding an extra __get_user(), do you? On top of that, the single pointer to the compat multi-context strcture, will force the kernel to decide which one of the members is actually valid or not. The set/unset callbacks separate each context from a structure POV and from a setup/teardown POV, and allow the kernel to use their native data structures. Think about if we had this for the latest pselect/ppoll/epoll_pwait. Think about how your solution and mine would apply to that very much concrete case. - Davide - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On Sat, 30 Jun 2007, Davide Libenzi wrote:
Think about if we had this for the latest pselect/ppoll/epoll_pwait.
Think about how your solution and mine would apply to that very much
concrete case.
This is how all those overloaded syscalls looks like, BTW:
if (sigmask) {
if (sigsetsize != sizeof(sigset_t))
return -EINVAL;
if (copy_from_user(ksigmask, sigmask, sizeof(ksigmask)))
return -EFAULT;
sigdelsetmask(ksigmask, sigmask(SIGKILL) | sigmask(SIGSTOP));
sigprocmask(SIG_SETMASK, ksigmask, sigsaved);
}
error = sys_XXX(...);
if (sigmask) {
if (error == -EINTR) {
memcpy(current-saved_sigmask, sigsaved,
sizeof(sigsaved));
set_thread_flag(TIF_RESTORE_SIGMASK);
} else
sigprocmask(SIG_SETMASK, sigsaved, NULL);
}
How would you do that with a single shared strcture, w/out adding in all
signal paths the knowledge of the structure?
- Davide
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On 6/29/07, Davide Libenzi <[EMAIL PROTECTED]> wrote:
[include/linux/indirect.h]
#define SYSIND_CTX_OPENFLAGS0
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.
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;
};
};
long sys_indirect(unsigned int nr,
int ctx_version,
const struct indirect_ctx *ctx,
const unsigned long *params);
This reduces the number of accesses to userlevel data to one and still
has all the flexibility needed.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[patch 0/6] sys_indirect RFC - sys_indirect introduction
This patch-set implements the skeleton for a new sys_indirect() system call.
The reason for such system call would be to avoid the proliferation of new
system calls, introduced to only wrap old ones by setting/unsetting a given
context before/after the real system call.
The internal operation of sys_indirect() would be (pseudo-code):
for_each_ctx(wrap_ctx, _ctxs) {
save_prev_ctx(_chain, tsk->xxx);
tsk->xxx = wrap_ctx;
}
err = call_system_call(nr, params);
for_each_saved_ctx(ctx, _chain) {
restore_ctx(tsk->xxx, ctx);
}
return err;
To made sys_indirect() decently flexible, and to avoid a sys_indirect2()
anytime soon, a simple "flags" parameter is clearly not sufficent to be
passed down with it.
To be flexible we need to allow 1) more than a simple "flags" to be passed
down 2) more than one context possibly to be set before the real system
call. Also, the data structures passed down as new-context-to-be-set should
be compat-free, in order to simplify the compat code.
The prototype for sys_indirect() is:
struct indirect_ctx {
__u32 ctx;
};
long sys_indirect(unsigned int nr,
const struct indirect_ctx **ctxs,
unsigned int nctxs,
const unsigned long *params);
The "struct indirect_ctx" is a stub structure that is supposed to be the
base for all the context set/unset operations.
An example usage for userspace POV (referring to the example in the next
patches) is:
[include/linux/indirect.h]
#define SYSIND_CTX_OPENFLAGS0
struct sysind_ctx_OPENFLAGS {
__u32 ctx;
__u32 flags;
};
[userspace]
struct sysind_ctx_OPENFLAGS octx;
struct indirect_ctx *ctxs[1];
unsigned long params[6];
octx.ctx = SYSIND_CTX_OPENFLAGS;
octx.flags = O_CLOEXEC;
ctxs[0] = (struct indirect_ctx *)
params[0] = domain;
params[1] = type;
params[2] = proto;
res = indirect(__NR_socket, ctxs, 1, params);
The sys_indirect() system call requires a small arch-dependent asm function
call_syscall() in order to call a system call by passing call number and
parameters (compat_call_syscall() is also needed for archs having 32 bit
compat).
I currently implemented them for i386 and x86-64.
The following patches builds but are totally untested at the moment.
Comments?
- Davide
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[patch 0/6] sys_indirect RFC - sys_indirect introduction
This patch-set implements the skeleton for a new sys_indirect() system call.
The reason for such system call would be to avoid the proliferation of new
system calls, introduced to only wrap old ones by setting/unsetting a given
context before/after the real system call.
The internal operation of sys_indirect() would be (pseudo-code):
for_each_ctx(wrap_ctx, user_ctxs) {
save_prev_ctx(saved_chain, tsk-xxx);
tsk-xxx = wrap_ctx;
}
err = call_system_call(nr, params);
for_each_saved_ctx(ctx, saved_chain) {
restore_ctx(tsk-xxx, ctx);
}
return err;
To made sys_indirect() decently flexible, and to avoid a sys_indirect2()
anytime soon, a simple flags parameter is clearly not sufficent to be
passed down with it.
To be flexible we need to allow 1) more than a simple flags to be passed
down 2) more than one context possibly to be set before the real system
call. Also, the data structures passed down as new-context-to-be-set should
be compat-free, in order to simplify the compat code.
The prototype for sys_indirect() is:
struct indirect_ctx {
__u32 ctx;
};
long sys_indirect(unsigned int nr,
const struct indirect_ctx **ctxs,
unsigned int nctxs,
const unsigned long *params);
The struct indirect_ctx is a stub structure that is supposed to be the
base for all the context set/unset operations.
An example usage for userspace POV (referring to the example in the next
patches) is:
[include/linux/indirect.h]
#define SYSIND_CTX_OPENFLAGS0
struct sysind_ctx_OPENFLAGS {
__u32 ctx;
__u32 flags;
};
[userspace]
struct sysind_ctx_OPENFLAGS octx;
struct indirect_ctx *ctxs[1];
unsigned long params[6];
octx.ctx = SYSIND_CTX_OPENFLAGS;
octx.flags = O_CLOEXEC;
ctxs[0] = (struct indirect_ctx *) octx;
params[0] = domain;
params[1] = type;
params[2] = proto;
res = indirect(__NR_socket, ctxs, 1, params);
The sys_indirect() system call requires a small arch-dependent asm function
call_syscall() in order to call a system call by passing call number and
parameters (compat_call_syscall() is also needed for archs having 32 bit
compat).
I currently implemented them for i386 and x86-64.
The following patches builds but are totally untested at the moment.
Comments?
- Davide
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] sys_indirect RFC - sys_indirect introduction
On 6/29/07, Davide Libenzi [EMAIL PROTECTED] wrote:
[include/linux/indirect.h]
#define SYSIND_CTX_OPENFLAGS0
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.
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;
};
};
long sys_indirect(unsigned int nr,
int ctx_version,
const struct indirect_ctx *ctx,
const unsigned long *params);
This reduces the number of accesses to userlevel data to one and still
has all the flexibility needed.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
