From: Alan Stern <stern@rowland.harvard.edu>
To: Ming Lei <ming.lei@canonical.com>
Cc: linux-kernel@vger.kernel.org, Oliver Neukum <oneukum@suse.de>,
Minchan Kim <minchan@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Rafael J. Wysocki" <rjw@sisk.pl>, Jens Axboe <axboe@kernel.dk>,
"David S. Miller" <davem@davemloft.net>,
Andrew Morton <akpm@linux-foundation.org>,
netdev@vger.kernel.org, linux-usb@vger.kernel.org,
linux-pm@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH v3 2/6] PM / Runtime: introduce pm_runtime_set[get]_memalloc_noio()
Date: Tue, 30 Oct 2012 11:38:46 -0400 (EDT) [thread overview]
Message-ID: <Pine.LNX.4.44L0.1210301112270.1363-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <CACVXFVOPDu6wVgPmvtTkokn7VV41x3XVvL4g_E0pz0mikUbvUg@mail.gmail.com>
On Tue, 30 Oct 2012, Ming Lei wrote:
> >> +bool pm_runtime_get_memalloc_noio(struct device *dev)
> >> +{
> >> + bool ret;
> >> + spin_lock_irq(&dev->power.lock);
> >> + ret = dev->power.memalloc_noio_resume;
> >> + spin_unlock_irq(&dev->power.lock);
> >> + return ret;
> >> +}
> >
> > You don't need to acquire and release a spinlock just to read the
> > value. Reading bitfields _is_ SMP-safe; writing them is not.
>
> Thanks for your review.
>
> As you pointed out before, the flag need to be checked before
> resetting usb devices, so the lock should be held to make another
> context(CPU) see the updated value suppose one context(CPU)
> call pm_runtime_set_memalloc_noio() to change the flag at the
> same time.
Okay, I see your point. But acquiring the lock here doesn't solve the
problem. Suppose a thread is about to reset a USB mass-storage device.
It acquires the lock and sees that the noio flag is clear. But before
it can issue the reset, another thread sets the noio flag.
I'm not sure what the best solution is.
> The lock needn't to be held when the function is called inside
> pm_runtime_set_memalloc_noio(), so the bitfield flag should
> be checked directly without holding power lock in dev_memalloc_noio().
Yes.
A couple of other things... Runtime resume can be blocked by runtime
suspend, if a resume is requested while the suspend is in progress.
Therefore the runtime suspend code also needs to save-set-restore the
noio flag.
Also, we should set the noio flag at the start of
usb_stor_control_thread, because everything that thread does can
potentially block an I/O operation.
Lastly, pm_runtime_get_memalloc_noio always returns false when
CONFIG_PM_RUNTIME is disabled. But we still need to prevent I/O during
usb_reset_device even when there's no runtime PM. Maybe the simplest
answer is always to set noio during resets. That would also help with
the race described above.
Alan Stern
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2012-10-30 15:38 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-29 12:23 [PATCH v3 0/6] solve deadlock caused by memory allocation with I/O Ming Lei
2012-10-29 12:23 ` [PATCH v3 1/6] mm: teach mm by current context info to not do I/O during memory allocation Ming Lei
2012-10-29 12:23 ` [PATCH v3 2/6] PM / Runtime: introduce pm_runtime_set[get]_memalloc_noio() Ming Lei
2012-10-29 15:41 ` Alan Stern
2012-10-30 3:21 ` Ming Lei
2012-10-30 10:57 ` Oliver Neukum
2012-10-30 11:30 ` Ming Lei
2012-10-30 15:38 ` Alan Stern [this message]
2012-10-30 16:00 ` Ming Lei
2012-10-30 16:15 ` Ming Lei
2012-10-30 16:30 ` Oliver Neukum
2012-10-31 2:08 ` Ming Lei
2012-10-31 3:05 ` Ming Lei
2012-10-31 8:37 ` Oliver Neukum
2012-10-31 15:20 ` Alan Stern
2012-10-31 15:41 ` Alan Stern
2012-10-31 23:18 ` Ming Lei
2012-10-30 16:54 ` Alan Stern
2012-10-29 12:23 ` [PATCH v3 3/6] block/genhd.c: apply pm_runtime_set_memalloc_noio on block devices Ming Lei
2012-10-29 12:23 ` [PATCH v3 4/6] net/core: apply pm_runtime_set_memalloc_noio on network devices Ming Lei
2012-10-29 15:44 ` Alan Stern
2012-10-29 12:23 ` [PATCH v3 5/6] PM / Runtime: force memory allocation with no I/O during runtime_resume callbcack Ming Lei
2012-10-29 12:24 ` [PATCH v3 6/6] USB: forbid memory allocation with I/O during bus reset Ming Lei
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.LNX.4.44L0.1210301112270.1363-100000@iolanthe.rowland.org \
--to=stern@rowland.harvard.edu \
--cc=akpm@linux-foundation.org \
--cc=axboe@kernel.dk \
--cc=davem@davemloft.net \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=minchan@kernel.org \
--cc=ming.lei@canonical.com \
--cc=netdev@vger.kernel.org \
--cc=oneukum@suse.de \
--cc=rjw@sisk.pl \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox