Re: [patch 05/11] syslets: core code

[Linus Torvalds]

On Thu, 15 Feb 2007, Evgeniy Polyakov wrote:
> 
> So we just need to describe the way we want to see new interface -
> that's it.

Agreed. Absolutely.

But please keep the kernel interface as part of that. Not just a strange 
and complex kernel interface and then _usable_ library interfaces that use 
the strange and complex one internally. Because if the complex one has no 
validity on its own, it's just (a) a bitch to debug and (b) if we ever 
change any details inside the kernel we'll end up with a lot of subtle 
code where user land creates complex data, and the kernel just reads it, 
and both just (unnecessarily) work around the fact that the other doesn't 
do the straightforward thing.

> Here is a stub for async_stat() - probably broken example, but that does
> not matter - this interface is really easy to change.
> 
> static void syslet_setup(struct syslet *s, int nr, void *arg1...)
> {
> 	s->flags = ...
> 	s->arg[1] = arg1;
> 	....
> }
> 
> long glibc_async_stat(const char *path, struct stat *buf)
> {
> 	/* What about making syslet and/or set of atoms per thread and preallocate 
> 	 * them when working threads are allocated? */
> 	struct syslet s;
> 	syslet_setup(&s, __NR_stat, path, buf, NULL, NULL, NULL, NULL);
> 	return async_submit(&s);
> }

And this is a classic example of potentially totally buggy code.

Why? You're releasing the automatic variable on the stack before it's 
necessarily all used!

So now you need to do a _longterm_ allocation, and that in turn means that 
you need to do a long-term de-allocation!

Ok, so do we make the rule be that all atoms *have* to be read fully 
before we start the async submission (so that the caller doesn't need to 
do a long-term allocation)?

Or do we make the rule be that just the *first* atom is copied by the 
kernel before the async_sumbit() returns, and thus it's ok to do the above 
*IFF* you only have a single system call?

See? The example you tried to use to show how "simple" the interface iswas 
actually EXACTLY THE REVERSE. It shows how subtle bugs can creep in!

		Linus
-
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/