Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 4 Jul 2014 08:57:52 -0700
From:      John-Mark Gurney <jmg@funkthat.com>
To:        Hans Petter Selasky <hps@selasky.org>
Cc:        arch@freebsd.org
Subject:   Re: callout(9) really this complicated?
Message-ID:  <20140704155752.GB45513@funkthat.com>
In-Reply-To: <53B64694.9030100@selasky.org>
References:  <20140704041521.GW45513@funkthat.com> <53B64694.9030100@selasky.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Hans Petter Selasky wrote this message on Fri, Jul 04, 2014 at 08:15 +0200:
> On 07/04/14 06:15, John-Mark Gurney wrote:
> >So, I was going to look at using callout(9) for some time delayed
> >actions...  But upon reading the docs, a) the docs are inconsistent,
> >and b) the docs only talk about requirements in other section...
> >
> >Is there a better interface?  If so, can we mark callout(9) deprecated?
> >If not, I have some questions...
> >
> >If you want callout_drain to work properly, you have to add extra code
> >to both your callout, and around the usage of it...
> >
> >callout_drain does not drain the callout:
> >      However, the callout subsystem does guarantee that the callout will 
> >      be
> >      fully stopped before callout_drain() returns.
> >
> >Yet other parse of the docs say that you can depend upon the callout
> >being fully stopped..  I've sent email to Ian (iedowse) about why he
> >added this statement...
> >
> >Second, the amount of work you have to do to make sure you drain
> >seems pretty crazy as documented in Avoiding Race Conditions...
> >
> >It seems like if I have created a callout w/ callout_init_mtx,
> >that I shouldn't have to do extra work to make it work correctly...
> >
> >When calling _callout_stop_safe as callout_drain, there is no assert
> >that the lock is held, though it is documented as requiring it by:
> >      The function callout_drain() is identical to callout_stop() except 
> >      that
> >      it will wait for the callout to be completed if it is already in
> >      progress.
> >
> >Maybe we need to add an additional statement here?  and assert that it
> >isn't locked?
> >
> >Also, I have tried to follow the code, but it's complicated, so I'm
> >hoping that I can get some help here.


> Probably the documentation needs an update. The implementation is fine.

Probably...  But w/ bad docs, it makes you wonder about the
implementation...

> Basically like with many other multi processor APIs in the kernel:
> 
> start and stop operations needs to be locked, not because of callout 
> internals, but because of your driver staying sync.

That's what I'd assume, but as above, the docs say otherwise, and the
docs are there to clarify your assumptions...

> drain is always called unlocked.

Then why the whole long section about avoiding races?   Point #3 is
the main one that I'm talking about...  It seems like it'd be easier
for callout to properly maintain the active flag (maybe set a flag to
tell callout to do so), or not even maintain it since it really isn't
used for anything important if you can munge it all you want...  There
aren't any statements about bad things happening if you call _deactivate
before the callout is called...

-- 
  John-Mark Gurney				Voice: +1 415 225 5579

     "All that I will do, has been done, All that I have, has not."



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140704155752.GB45513>