Date:	Thu, 1 Mar 2007 10:18:40 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Kevin O'Connor <kevin@...onnor.net>
Cc:	linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Arjan van de Ven <arjan@...radead.org>,
	Christoph Hellwig <hch@...radead.org>,
	Andrew Morton <akpm@....com.au>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Ulrich Drepper <drepper@...hat.com>,
	Zach Brown <zach.bro@...E.mta.everyone.net>
Subject: Re: [patch 02/12] syslets: add syslet.h include file, user API/ABI definitions


* Kevin O'Connor <kevin@...onnor.net> wrote:

> I'd like to propose a simpler userspace API for syslets.  I believe 
> this revised API is just as capable as yours (anything done purely in 
> kernel space with the existing API can also be done with this one).
> 
> An "atom" would look like:
> 
> struct syslet_uatom {
> 	u32		nr;
> 	u64		ret_ptr;
> 	u64		next;
> 	u64		arg_nr;
> 	u64		args[6];
> };
> 
> The sys_nr, ret_ptr, and next fields would be unchanged.  The args 
> array would directly store the arguments to the system call.  To 
> optimize the case where only a few arguments are necessary, an 
> explicit argument count would be set in the arg_nr field.

i dont see the advantage of arg_nr - if the arguments are direct then 
the best way is to just fetch them all, not to do testing upon arg_nr. 
Furthermore, regarding the indirect pointers, they are quite essential 
for some uses, see:

  http://lkml.org/lkml/2007/2/28/292

> Of course, the above lacks the syscall return testing capabilities in 
> your atoms.  To obtain that capability, one could add a new syscall:
> 
> long sys_syslet_helper(long flags, long *ptr, long inc, u64 new_next)

yes. But a 'flags' field is handy anyway, to signal things like 
NOCOMPLETE or SYSLET_SYNC/SYSLET_ASYNC (a flags field is always useful 
in such structures). So the condition testing comes 'for free' in 
essence.

but ... as you can see it with sys_umem_add(), i like the addition of 
helper syscalls - i just think that this particular one wouldnt be too 
helpful.

> I would also change the event ring notification system.  Instead of 
> building that support into all syslets, one could introduce an "add to 
> head" syscall specifically for that purpose.  If done this way, 
> userspace could arrange for this new sys_addtoring call to always be 
> the last uatom executed in a syslet.  This would make the support 
> optional - those userspace applications that prefer to use a futex or 
> signal as an event system could arrange to have those system calls as 
> the last one in the chain instead. [...]

the problem is signals: a syslet has to return to user-space upon 
signals or upon a stop condition. So to notify about the precise place 
of stoppage, the notification ring is a 'built in' property.

(nevertheless, as i mentioned it in a prior mail, i'll create separate 
ring syscalls, because they are useful for other stuff too.)

> For example, the open & stat case could be done with a chain like the 
> following:
> 
> atom1: &atom3->args[1] = sys_open(...)
> atom2: sys_syslet_helper(SYSLET_BRANCH_ON_NON_POSITIVE,
>                          &atom3->args[1], 0, atom4)

i dont see a huge conceptual difference between having the syslet helper 
in flags versus having it in a separate syscall. Other than yours has 
twice the number of atoms.

> It is also possible to use sys_syslet_helper to push a return value to 
> multiple syslet parameters (for example, propagating an fd from open 
> to multiple reads).  For example:
> 
> atom1: &atom3->args[1] = sys_open(...)
> atom2: &atom4->args[1] = sys_syslet_helper(0, &atom3->args[1], 0, 0)
> atom3: sys_read([arg1 filled in atom1], ...)
> atom4: sys_read([arg1 filled in atom2], ...)

try to do this in FIO. You'd have to create many extra atoms to push the 
fd into the argument fields - instead of just sharing the variable. 
Sharing is /good/. These 'simplifications' complicate the whole syslet 
programming model down to being near unusable.

> Although this is a bit ugly, I must wonder how many times one would 
> build chains complex enough to require it.

take a look at FIO.

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

Powered by blists - more mailing lists