From: Toshi Kani <toshi.kani@hp.com>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Linux PM list <linux-pm@vger.kernel.org>,
Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>,
linux-mm@kvack.org
Subject: Re: [PATCH 3/3] PM / hibernate / memory hotplug: Rework mutual exclusion
Date: Fri, 30 Aug 2013 18:35:12 -0600 [thread overview]
Message-ID: <1377909312.10300.908.camel@misato.fc.hp.com> (raw)
In-Reply-To: <1984629.rbRksDyNff@vostro.rjw.lan>
On Sat, 2013-08-31 at 02:39 +0200, Rafael J. Wysocki wrote:
> On Friday, August 30, 2013 06:23:19 PM Toshi Kani wrote:
> > On Thu, 2013-08-29 at 23:18 +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >
> > > Since all of the memory hotplug operations have to be carried out
> > > under device_hotplug_lock, they won't need to acquire pm_mutex if
> > > device_hotplug_lock is held around hibernation.
> > >
> > > For this reason, make the hibernation code acquire
> > > device_hotplug_lock after freezing user space processes and
> > > release it before thawing them. At the same tim drop the
> > > lock_system_sleep() and unlock_system_sleep() calls from
> > > lock_memory_hotplug() and unlock_memory_hotplug(), respectively.
> > >
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > ---
> > > kernel/power/hibernate.c | 4 ++++
> > > kernel/power/user.c | 2 ++
> > > mm/memory_hotplug.c | 4 ----
> > > 3 files changed, 6 insertions(+), 4 deletions(-)
> > >
> > > Index: linux-pm/kernel/power/hibernate.c
> > > ===================================================================
> > > --- linux-pm.orig/kernel/power/hibernate.c
> > > +++ linux-pm/kernel/power/hibernate.c
> > > @@ -652,6 +652,7 @@ int hibernate(void)
> > > if (error)
> > > goto Exit;
> > >
> > > + lock_device_hotplug();
> >
> > Since hibernate() can be called from sysfs, do you think the tool may
> > see this as a circular dependency with p_active again? This shouldn't
> > be a problem in practice, though.
>
> /sys/power/state isn't a device attribute even and is never removed, so it
> would be very sad and disappointing if lockdep reported that as a circular
> dependency. The deadlock is surely not possible here anyway.
Agreed. The code looks good otherwise, and this is a nice cleanup. If
it is OK to ignore the possible warning from the tool (which I do not
know the rule here), feel free to add my ack to patch 2/3 and 3/3 as
well.
Thanks,
-Toshi
--
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>
WARNING: multiple messages have this Message-ID (diff)
From: Toshi Kani <toshi.kani@hp.com>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Linux PM list <linux-pm@vger.kernel.org>,
Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>,
linux-mm@kvack.org
Subject: Re: [PATCH 3/3] PM / hibernate / memory hotplug: Rework mutual exclusion
Date: Fri, 30 Aug 2013 18:35:12 -0600 [thread overview]
Message-ID: <1377909312.10300.908.camel@misato.fc.hp.com> (raw)
In-Reply-To: <1984629.rbRksDyNff@vostro.rjw.lan>
On Sat, 2013-08-31 at 02:39 +0200, Rafael J. Wysocki wrote:
> On Friday, August 30, 2013 06:23:19 PM Toshi Kani wrote:
> > On Thu, 2013-08-29 at 23:18 +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >
> > > Since all of the memory hotplug operations have to be carried out
> > > under device_hotplug_lock, they won't need to acquire pm_mutex if
> > > device_hotplug_lock is held around hibernation.
> > >
> > > For this reason, make the hibernation code acquire
> > > device_hotplug_lock after freezing user space processes and
> > > release it before thawing them. At the same tim drop the
> > > lock_system_sleep() and unlock_system_sleep() calls from
> > > lock_memory_hotplug() and unlock_memory_hotplug(), respectively.
> > >
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > ---
> > > kernel/power/hibernate.c | 4 ++++
> > > kernel/power/user.c | 2 ++
> > > mm/memory_hotplug.c | 4 ----
> > > 3 files changed, 6 insertions(+), 4 deletions(-)
> > >
> > > Index: linux-pm/kernel/power/hibernate.c
> > > ===================================================================
> > > --- linux-pm.orig/kernel/power/hibernate.c
> > > +++ linux-pm/kernel/power/hibernate.c
> > > @@ -652,6 +652,7 @@ int hibernate(void)
> > > if (error)
> > > goto Exit;
> > >
> > > + lock_device_hotplug();
> >
> > Since hibernate() can be called from sysfs, do you think the tool may
> > see this as a circular dependency with p_active again? This shouldn't
> > be a problem in practice, though.
>
> /sys/power/state isn't a device attribute even and is never removed, so it
> would be very sad and disappointing if lockdep reported that as a circular
> dependency. The deadlock is surely not possible here anyway.
Agreed. The code looks good otherwise, and this is a nice cleanup. If
it is OK to ignore the possible warning from the tool (which I do not
know the rule here), feel free to add my ack to patch 2/3 and 3/3 as
well.
Thanks,
-Toshi
next prev parent reply other threads:[~2013-08-31 0:35 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-29 21:12 [PATCH 0/3] ACPI / hotplug / mm: Rework mutual exclusion between hibernation and memory hotplug Rafael J. Wysocki
2013-08-29 21:12 ` Rafael J. Wysocki
2013-08-29 21:15 ` [PATCH 1/3] ACPI / scan: Change ordering of locks for device hotplug Rafael J. Wysocki
2013-08-29 21:15 ` Rafael J. Wysocki
2013-08-31 0:17 ` Toshi Kani
2013-08-31 0:17 ` Toshi Kani
2013-08-29 21:17 ` [PATCH 2/3] PM / hibernate: Create memory bitmaps after freezing user space Rafael J. Wysocki
2013-08-29 21:17 ` Rafael J. Wysocki
2013-08-29 21:18 ` [PATCH 3/3] PM / hibernate / memory hotplug: Rework mutual exclusion Rafael J. Wysocki
2013-08-29 21:18 ` Rafael J. Wysocki
2013-08-31 0:23 ` Toshi Kani
2013-08-31 0:23 ` Toshi Kani
2013-08-31 0:39 ` Rafael J. Wysocki
2013-08-31 0:39 ` Rafael J. Wysocki
2013-08-31 0:35 ` Toshi Kani [this message]
2013-08-31 0:35 ` Toshi Kani
2013-08-31 0:50 ` Rafael J. Wysocki
2013-08-31 0:50 ` Rafael J. Wysocki
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=1377909312.10300.908.camel@misato.fc.hp.com \
--to=toshi.kani@hp.com \
--cc=isimatu.yasuaki@jp.fujitsu.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-pm@vger.kernel.org \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.