cvs commit: src/sys/compat/linux linux_misc.c src/sys/kern kern_time.c src/sys/sys systm.h

[John Baldwin]

On Tuesday 01 February 2005 10:30 am, Maxim Sobolev wrote:
> John Baldwin wrote:
> > On Monday 31 January 2005 07:03 pm, Maxim Sobolev wrote:
> >>John Baldwin wrote:
> >>>On Monday 31 January 2005 04:01 pm, Maxim Sobolev wrote:
> >>>>John Baldwin wrote:
> >>>>>On Tuesday 25 January 2005 04:28 pm, Maxim Sobolev wrote:
> >>>>>>sobomax     2005-01-25 21:28:28 UTC
> >>>>>>
> >>>>>>FreeBSD src repository
> >>>>>>
> >>>>>>Modified files:
> >>>>>>  sys/compat/linux     linux_misc.c
> >>>>>>  sys/kern             kern_time.c
> >>>>>>  sys/sys              systm.h
> >>>>>>Log:
> >>>>>>Split out kernel side of {get,set}itimer(2) into two parts: the first
> >>>>>>that pops data from the userland and pushes results back and the
> >>>>>> second which does actual processing. Use the latter to eliminate
> >>>>>> stackgap in the linux wrappers of those syscalls.
> >>>>>>
> >>>>>>MFC after:      2 weeks
> >>>>>
> >>>>>Hmm, I already implemented kern_[sg]etitimer() locally and fixed all
> >>>>> the ABIs, not just Linux to use them.  I haven't had time to test the
> >>>>> patches though. Would you be interested in them?
> >>>>
> >>>>I would be happy to, but I don't have any platforms other than i386
> >>>>(which is why I have not touched other arches). Therefore I am probably
> >>>>a wrong person to do the testing. Is your approach different/better
> >>>> than mine? I'd be happy to do the merge if so.
> >>>
> >>>I've already merged.  It's mostly the same except I put the logic to
> >>> fall back to kern_getitimer() when there is no new itimerval in
> >>>kern_setitimer() rather than duplicating that in all the ABIs.
> >>
> >>Well, I had considered such approach, but rejected it because actually
> >>it doesn't win you much since you have to add some logic to avoid
> >>copyin() and submit NULL instead of &aitv to signal kern_setitimer()
> >>that it has to call kern_getitimer() into each ABI instead. As to my
> >>taste such obfuscation (which is duplicated among all ABIs) isn't worth
> >>it. At the same time, linecount wise it is probably the same amount of
> >>duplication anyway.
> >
> > Yes, but modifying struct uap on the fly and assuming you can cast it to
> > the new type without problems is worse IMHO. :)  Especially when you have
> > to deal with emulating an ABI like freebsd32 where the pointer sizes are
> > different between the kernel and the userland APP you are running.
>
> But you are assigning two members of the same type, no? You aren't
> assigning "kernel pointer" to "userland pointer", but "userland pointer"
> to "userland pointer". I am failing to see how it can create any
> problems. If we can test that pointer to be NULL then we can do this
> assignment, since both cases require compiler to know the correct size
> of the pointer.

Well, if you call foo_getitimer() rather than getitimer() itself as 
setitimer() currently does you are fine I guess.  I guess I prefer to keep as 
much logic as possible out of the ABI wrappers as a general rule and relegate 
them to just copyin() / kern_foo() / copyout() so that when one has to fix 
bugs, one doesn't have to fix them in 47 different places.  I also already 
have it done this way (and keeping track of the pointer to kern_setitimer() 
is all of 1 local variable and 2 lines of code).  Similar to this vein, I 
changed linux_alarm() to now just call kern_setitimer() rather than doing all 
the guts of that syscall itself.

-- 
John Baldwin <jhb at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org