Arnd,
On Wed, Jul 2, 2008 at 3:14 PM, Arnd Bergmann <[EMAIL PROTECTED]> wrote:
> On Wednesday 02 July 2008, stephane eranian wrote:
>
>> 2- all system calls taking structure parameters have reserved
>> fields. They can be used to support extensions.
>>
>> I see 3 problems with reserved fields:
>>
>> - They are supposed to be all zeroes. Let's say, a new
>> perfmon version uses one to support an extension.
>> What if value 0 is valid for this extension, and we
>> execute an old application which does not know about it
>> and passes 0 in the corresponding reserved field?
>>
>> This does not systematically translate into a problem.
>> Worst case, it may generate events which the old
>> application does not know about.
>
> A common way to deal with this is to have a "flags" argument in
> each syscall that may get extended. Each bit in the flags will
> mean that a certain extension or feature is used, and its
> arguments are unused otherwise.
>
> With this, you may even add new arguments to the syscall, if
> necessary, in the same way that the 'open' syscall has a 'mode'
> argument that is only used when the flag contains O_CREAT.
That is indeed a good solution. When I look at the full interface
(GIT tree) and the minimal interface (quilt series), a value of
zero is valid for all new fields, however, it always means 'do
nothing'. I think we are lucky here but I'd like to have a more
robust mechanism. What you are suggesting looks interesting.
I think this could work even with vector arguments, where not all
elements necessarily use the same features.
If I take the example of the create session call:
It is currently defined as:
int pfm_create_context(struct pfarg_ctx *ctx, char *smpl_name, void
*smpl_arg, size_t smpl_size);
struct pfarg_ctx {
__u32 flags;
__u32 reserved1;
__u64 reserved2[7];
}
Initially, the patch does not contain sampling support thus no
in-kernel sampling buffer format.
The call can be simplified to:
int pfm_create_context(struct pfarg_ctx *ctx);
If I use your suggested mechanisms:
int pfm_create_context(struct pfarg_ctx *ctx, int flags);
To add in-kernel sampling support, we could then do:
#define PFM_CTX_SMPL_FMT 0x1
int pfm_create_context(struct pfarg_ctx *ctx, int flags, char
*smpl_fmt, void *smpl_arg, size_ smpl_size);
And when using sampling format "default", it could be invoked as:
pfm_create_context(&ctx, PFM_CTX_SMPL_FMT, "default", NULL, 0);
The kernel would interpret PFM_CTX_SMPL_FMT as meaning that there are
new parameters to copy-in.
For this particular call, we could push this a bit further and drop
the pfarg_ctx alltogether and merge it
with flags.
The main worry is the limit on the number of syscall parameters which
is typically fairly low (8).
Of course, this could be compensated by passing a struct instead.
>> I am tempted to transform the interface to have a mix of the two
>> approaches. I could clearly see that load/unload
>> and start/stop could be implemented by a generic pfm_control(int fd,
>> int CMD, void *arg, size_t arg_sz) call. To go
>> further, we could also group the write syscalls : pfm_write_regs(int
>> fd, int type, void *arg, size_t arg_sz) with type
>> indicating either config or data registers.
>>
>> What do you think?
>
> No, please don't do unstructured (void*) arguments in syscalls, they
> cause all sorts of problems, e.g. in 32 bit emulation (if it becomes
> necessary to convert the structure), in strace, gdb and anything that
> tries to make sense of the arguments.
>
All the data structures shared between user and kernel have fixed size.
There is no marshalling necessary. So the only argument I can see about
this is the abscence of strong type checking.
-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
perfmon2-devel mailing list
perfmon2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
