Re: [PATCH V8 08/13] posix clocks: cleanup the CLOCK_DISPTACH macro

[Richard Cochran]

On Tue, Jan 11, 2011 at 01:57:23PM +0100, Thomas Gleixner wrote:
> I wonder whether we should be a bit more clever here and create an
> extra entry for posix_cpu_timers in the posix_clocks array and do the
> following:
...
> That reduces the code significantly and the MAX_CLOCKS check in
> clock_get_array_id() replaces the invalid_clock() check in the
> syscalls as well. It does not matter whether we check this before or
> after copying stuff from user.

Well, this does reduce the number of LOC, but the number of
comparisons is the same. I think the code size would be also no
different.

> Adding your new stuff requires just another entry in the array, the
> setup of the function pointers and the CLOCKFD check in
> clock_get_array_id(). Everything else just falls in place.

For me, I am not sure if either version is really very pretty or easy
to understand.

My instinct is to keep the posix_cpu_X and pc_X functions out of the
array of static clock id functions, if only to make the distinction
between the legacy static ids and new dynamic ids clear.

The conclusion from the "dynamic clock as file" discussion was that we
don't want to add any more static clock ids, and new clocks should use
the new, dynamic clock API. So, we should discourage any future
attempt to add to that function array!

Having said that, if you insist on it, I won't mind reworking the
dispatch code as you suggested.

> > +
> > +#define CLOCK_DISPATCH(clock, call, arglist) dispatch_##call arglist
> > +
> 
> Can we get rid of this completely please ?

Yes, gladly.

> >  clock_nanosleep_restart(struct restart_block *restart_block)
> >  {
> > -	clockid_t which_clock = restart_block->arg0;
> > -
> 
> How does that compile ?

Because the CLOCK_DISPATCH macro, above, makes no use of the first
argument! I have removed the macro for the next round.

> >  	return CLOCK_DISPATCH(which_clock, nsleep_restart,
> >  			      (restart_block));
> >  }

Thanks,
Richard
--
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/