RE: [PATCH] mmc: fix a deadlock between system suspend and MMC block IO

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Chris,

> -----Original Message-----
> From: Chris Ball [mailto:cjb@xxxxxxxxxx]
> Sent: Wednesday, January 04, 2012 8:34 PM
> To: Nath, Arindam
> Cc: Guennadi Liakhovetski; linux-mmc@xxxxxxxxxxxxxxx; Andrei Warkentin
> Subject: Re: [PATCH] mmc: fix a deadlock between system suspend and MMC
> block IO
> 
> Hi,
> 
> Adding Andrei to CC.  Thanks,
> 
> - Chris.
> 
> On Wed, Jan 04 2012, Nath, Arindam wrote:
> > Hi Chris,
> >
> >> -----Original Message-----
> >> From: linux-mmc-owner@xxxxxxxxxxxxxxx [mailto:linux-mmc-
> >> owner@xxxxxxxxxxxxxxx] On Behalf Of Chris Ball
> >> Sent: Wednesday, January 04, 2012 8:07 PM
> >> To: Guennadi Liakhovetski
> >> Cc: linux-mmc@xxxxxxxxxxxxxxx
> >> Subject: Re: [PATCH] mmc: fix a deadlock between system suspend and
> MMC
> >> block IO
> >>
> >> Hi,
> >>
> >> On Wed, Jan 04 2012, Guennadi Liakhovetski wrote:
> >> > Performing MMC block IO with simultaneous STR can lead to a
> deadlock:
> >> the
> >> > mmc_pm_notify() function claims the host and then calls bus
> .remove()
> >> > method, which lands in mmc_blk_remove(), which calls
> >> mmc_blk_remove_req()
> >> > then it goes to -> mmc_cleanup_queue() -> kthread_stop(), which
> waits
> >> for
> >> > the mmc-block thread to stop. If the mmc-block thread at that time
> is
> >> > processing block requests, it will also try to claim the host in
> >> > mmc_blk_issue_rq() and block there. This patch fixes the problem
> by
> >> > calling .remove() before claiming the host.
> >> >
> >> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> >> > ---
> >> >  drivers/mmc/core/core.c |    4 ++--
> >> >  1 files changed, 2 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> >> > index a2aa860..a68f085 100644
> >> > --- a/drivers/mmc/core/core.c
> >> > +++ b/drivers/mmc/core/core.c
> >> > @@ -2476,11 +2476,11 @@ int mmc_pm_notify(struct notifier_block
> >> *notify_block,
> >> >  		if (!host->bus_ops || host->bus_ops->suspend)
> >> >  			break;
> >> >
> >> > -		mmc_claim_host(host);
> >> > -
> >> > +		/* On 2 occasions above bus_ops->remove() is called
> >> unlocked */
> >> >  		if (host->bus_ops->remove)
> >> >  			host->bus_ops->remove(host);
> >> >
> >> > +		mmc_claim_host(host);
> >> >  		mmc_detach_bus(host);
> >> >  		mmc_power_off(host);
> >> >  		mmc_release_host(host);
> >>
> >> Thanks.  The commit message explanation is very good, but the
> comment
> >> is
> >> a bit cryptic.  Shall we make it longer?  I think even just:
> >>
> >> /* Calling bus_ops->remove() with a claimed host can deadlock */
> >>
> >> would be better.
> >
> > This patch will actually fix a long standing issue for most of us.
> But
> > if I remember correctly, Andrei had some comments on the same patch
> > some time back. Would it be good to include him for comments?

I think we can push this patch now. If someone has any objections, we can discuss later.

Acked-by: Arindam Nath <arindam.nath@xxxxxxx>

Thanks,
Arindam

> 
> --
> Chris Ball   <cjb@xxxxxxxxxx>   <http://printf.net/>
> One Laptop Per Child


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux