2009-09-01 16:09:36

by Roland Dreier

[permalink] [raw]
Subject: Re: [PATCH 1/1] introduce __cancel_delayed_work()


> cancel_delayed_work() has to use del_timer_sync() to guarantee the
> timer function is not running after return. But most users doesn't
> actually need this, and del_timer_sync() has problems: it is not
> useable from interrupt, and it depends on every lock which could
> be taken from irq.
>
> Introduce __cancel_delayed_work() which calls del_timer() instead.
>
> The immediate reason for this patch is
> http://bugzilla.kernel.org/show_bug.cgi?id=13757
> but hopefully this helper makes sense anyway.

Thanks, Oleg!

Andrew -- how do you want to handle this? This seems to be useful for
the bug with IB that Oleg linked to, as well as by Dmitry in input, so
I'm not sure what the best way to merge all this into 2.6.32 is.

I could take Oleg's patch and the corresponding fix to
drivers/infiniband through my tree, and merge as early I as I see Linus
open 2.6.32. That leaves Dmitry to wait on it (and possibly causes
problems in -next with tree ordering) though. But I don't see any way
to get the number of cross-tree dependencies below 1... (unless maybe
Dmitry can take the identical workqueue patch into his tree and trust
git to sort it out?)

- R.


2009-09-01 16:40:38

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 1/1] introduce __cancel_delayed_work()

On Tuesday 01 September 2009 09:09:36 am Roland Dreier wrote:
>
> > cancel_delayed_work() has to use del_timer_sync() to guarantee the
> > timer function is not running after return. But most users doesn't
> > actually need this, and del_timer_sync() has problems: it is not
> > useable from interrupt, and it depends on every lock which could
> > be taken from irq.
> >
> > Introduce __cancel_delayed_work() which calls del_timer() instead.
> >
> > The immediate reason for this patch is
> > http://bugzilla.kernel.org/show_bug.cgi?id=13757
> > but hopefully this helper makes sense anyway.
>
> Thanks, Oleg!
>
> Andrew -- how do you want to handle this? This seems to be useful for
> the bug with IB that Oleg linked to, as well as by Dmitry in input, so
> I'm not sure what the best way to merge all this into 2.6.32 is.
>
> I could take Oleg's patch and the corresponding fix to
> drivers/infiniband through my tree, and merge as early I as I see Linus
> open 2.6.32. That leaves Dmitry to wait on it (and possibly causes
> problems in -next with tree ordering) though. But I don't see any way
> to get the number of cross-tree dependencies below 1... (unless maybe
> Dmitry can take the identical workqueue patch into his tree and trust
> git to sort it out?)

I wonder if Linus would not just take it in 31 - it is a completely
new function with no current users (but users will surely follow) so
introducing regression is highly unlikely... That would resolve all
inter-tree dependencies.

Otherwise we'll have to leave our fate in the hands of git ;)

--
Dmitry

2009-09-01 22:29:44

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 1/1] introduce __cancel_delayed_work()

On Tue, 1 Sep 2009 09:40:33 -0700
Dmitry Torokhov <[email protected]> wrote:

> > Andrew -- how do you want to handle this? This seems to be useful for
> > the bug with IB that Oleg linked to, as well as by Dmitry in input, so
> > I'm not sure what the best way to merge all this into 2.6.32 is.
> >
> > I could take Oleg's patch and the corresponding fix to
> > drivers/infiniband through my tree, and merge as early I as I see Linus
> > open 2.6.32. That leaves Dmitry to wait on it (and possibly causes
> > problems in -next with tree ordering) though. But I don't see any way
> > to get the number of cross-tree dependencies below 1... (unless maybe
> > Dmitry can take the identical workqueue patch into his tree and trust
> > git to sort it out?)
>
> I wonder if Linus would not just take it in 31 - it is a completely
> new function with no current users (but users will surely follow) so
> introducing regression is highly unlikely... That would resolve all
> inter-tree dependencies.
>
> Otherwise we'll have to leave our fate in the hands of git ;)

I'll try to sneak it into 2.6.31 once Linus returns, but it'll need
good timing.

Or someone else can merge it into their tree, in which case I'll
promptly forget about it ;)