[PATCH] Staging: hv: fix sleeping while atomic issue

Greg KH gregkh at suse.de
Fri Dec 10 18:12:37 UTC 2010


On Fri, Dec 10, 2010 at 08:00:49PM +0200, Timo Teräs wrote:
> On 12/10/2010 07:35 PM, Greg KH wrote:
> > On Fri, Dec 10, 2010 at 05:00:01PM +0200, Timo Teräs wrote:
> >> osd_schedule_callback() is called from VmbusOnMsgDPC() which runs
> >> in a tasklet. Avoid possible sleeping by using GFP_ATOMIC for the
> >> memory allocation.
> >>
> >> Seems to fix #16701.
> >>
> >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=16701
> >> Cc: Haiyang Zhang <haiyangz at microsoft.com>
> >> Cc: Hank Janssen <hjanssen at microsoft.com>
> >> Signed-off-by: Timo Teräs <timo.teras at iki.fi>
> >> ---
> >>  drivers/staging/hv/osd.c |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/staging/hv/osd.c b/drivers/staging/hv/osd.c
> >> index 8c3eb27..eb9b20d 100644
> >> --- a/drivers/staging/hv/osd.c
> >> +++ b/drivers/staging/hv/osd.c
> >> @@ -214,7 +214,7 @@ int osd_schedule_callback(struct workqueue_struct *wq,
> >>  {
> >>  	struct osd_callback_struct *cb;
> >>  
> >> -	cb = kmalloc(sizeof(*cb), GFP_KERNEL);
> >> +	cb = kmalloc(sizeof(*cb), GFP_ATOMIC);
> > 
> > How about just fixing the one place where this is needed?  You don't
> > need this for all of the osd_schedule_callback() functions, right?
> > 
> > Actually, we should unroll this function entirely and use the "real"
> > functions the kernel provides us.  I'd much rather see that fix than
> > this one which just papers over the problem.
> 
> Yes, I agree. The osd_schedule_callback should definitely go away, we
> could also get rid of this kmalloc() by allocating the workqueue_struct
> as part of the callback context data which is the usual convention.

Yes.

> This was intended as minimum intrusion fix which would be suitable for
> -stable. As this bug seems to be a major issue in certain configurations.

No, fix the bug properly now, we will worry about older kernel releases
later.

> I'm not entirely sure about the usage of osd_schedule_callback() on
> other cases. It would appear that only VmbusOnMsgDPC() needs the atomic
> memory allocation.

Then just "open code" the callback properly in that function to solve
the issue.

> I preferred to not change the signature of an exported symbol.

Why not?  It's just for these few modules, no big dea.

> I can resubmit a version which adds the gfp_mask as parameter to
> osd_schedule_callback if that would be acceptable for -stable.

No, please fix it properly as described above.

thanks,

greg k-h



More information about the devel mailing list