Re: [PATCH] timer: document TIMER_PINNED

From: Peter Xu
Date: Wed Jun 26 2019 - 21:34:07 EST


On Thu, Jun 27, 2019 at 12:43:03AM +0200, Thomas Gleixner wrote:
> Peter,
>
> On Tue, 18 Jun 2019, Peter Xu wrote:
> > -/*
> > - * A deferrable timer will work normally when the system is busy, but
> > - * will not cause a CPU to come out of idle just to service it; instead,
> > - * the timer will be serviced when the CPU eventually wakes up with a
> > - * subsequent non-deferrable timer.
> > +/**
> > + * @TIMER_DEFERRABLE: A deferrable timer will work normally when the
> > + * system is busy, but will not cause a CPU to come out of idle just
> > + * to service it; instead, the timer will be serviced when the CPU
> > + * eventually wakes up with a subsequent non-deferrable timer.
> > *
> > - * An irqsafe timer is executed with IRQ disabled and it's safe to wait for
> > - * the completion of the running instance from IRQ handlers, for example,
> > - * by calling del_timer_sync().
> > + * @TIMER_IRQSAFE: An irqsafe timer is executed with IRQ disabled and
> > + * it's safe to wait for the completion of the running instance from
> > + * IRQ handlers, for example, by calling del_timer_sync().
> > *
> > * Note: The irq disabled callback execution is a special case for
> > * workqueue locking issues. It's not meant for executing random crap
> > * with interrupts disabled. Abuse is monitored!
> > + *
> > + * @TIMER_PINNED: A pinned timer will not be affected by timer
> > + * migration so it will always be run on a static cpu that was
> > + * specified.
>
> That's a bit misleading.
>
> The timer pinned flag prevents the MOHZ timer placement heuristics to take
> effect. That means that the timer is enqueued on the CPU on which the
> enqueue function is invoked.

Right. I thought these details could be hidden from the API layer so
timer users won't need to know these details (neither about NOHZ, nor
about the fact that the action of queuing the timer is the thing that
really matters, because add_timer_on() can hide all these), but it's
also ok to me if you prefer them to be exposed to the users.

>
> > + * Note: neither timer_setup() nor mod_timer() will
> > + * guarantee correct pinning of timers. One should always use
> > + * add_timer_on() when arm the timer to guarantee that the timer will
> > + * be pinned to the target CPU correctly.
>
> I'd rather say:
>
> That means that pinned timers are not guaranteed to stay on the
> initialy selected CPU. They move to the CPU on which the enqueue
> function is invoked via mod_timer() or add_timer().
>
> If the timer should be placed on a particular CPU then add_timer_on()
> has to be used.
>
> Or something like that. Too tired to think about it right now, but you get
> the idea.

Yes, let me give it another shot soon... Thanks,

--
Peter Xu