Re: [PATCH v2 1/8] time: add *_to_jiffies_timeout helpers to guarantee a minimum duration

[Arnd Bergmann <arnd@arndb.de>]

On Wednesday 15 May 2013, Imre Deak wrote:
> On Wed, 2013-05-15 at 17:26 +0200, Arnd Bergmann wrote:
>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > 
> > This certainly looks like a reasonable change, but I wonder if we could take
> > it one step further and add milisecond based interfaces for some of those
> > functions that currently take a jiffies value, something like
> > 
> > int timer_set_msecs(struct timer_list *timer, unsigned long msecs)
> > {
> > 	unsigned long j = msec_to_jiffies(msecs);
> > 	return mod_timer(timer, min_t(msecs, MAX_JIFFY_OFFSET));
> > }
> 
> Ok, but I think we'd still need the *_to_jiffies_timeout helpers, so
> that we don't have to open code the +1 magic anywhere.

Yes, but we would not change any driver code to use those.
We probably would not need all of the helpers and add only the
ones that are required by other infrastructure.

> > > +#define __define_time_to_jiffies_timeout(tname, ttype)                 \
> > > +unsigned long tname ## _to_jiffies_timeout(const ttype v)              \
> > > +{                                                                      \
> > > +       unsigned long j = tname ## _to_jiffies(v);                      \
> > > +       return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);           \
> > > +}                                                                      \
> > > +EXPORT_SYMBOL(tname ## _to_jiffies_timeout);
> > 
> > The macro has a few disadvantages:
> > 
> > * It's impossible to grep for the function or use tags if you generate
> >   the identifier using the macro.
> 
> They are fully spelled in include/linux/jiffies.h . Would it be ok if I
> moved the kernel doc there with a reference to kernel/time.c?

Yes, I guess that's ok. I would prefer to have them open-coded, but
it's not a big issue.

> > * msecs_to_jiffies is what puts MAX_JIFFY_OFFSET there in the first
> >   place, which means you add an extra comparison here that should
> >   not really be needed.
> 
> Yes, but that allows us to keep things simple across all the helpers. I
> haven't checked but I'd assume compiler inlining/optimization should
> make this a non-issue anyway.

msecs_to_jiffies is a global function, so it won't normally get inlined.

	Arnd.