All of lore.kernel.org
 help / color / mirror / Atom feed
From: Baoquan He <bhe@redhat.com>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Pingfan Liu <kernelfans@gmail.com>,
	Dave Young <dyoung@redhat.com>,
	Kexec Mailing List <kexec@lists.infradead.org>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	Linux PM <linux-pm@vger.kernel.org>
Subject: Re: [bug report] PM / reboot: Eliminate race between reboot and suspend
Date: Fri, 22 Jan 2021 15:38:59 +0800	[thread overview]
Message-ID: <20210122073859.GA4797@MiWiFi-R3L-srv> (raw)
In-Reply-To: <CAJZ5v0jgTyXQxPVQoM4nykk7gcWfNw9bXTQO8856vkYD8TJOCg@mail.gmail.com>

On 01/21/21 at 03:42pm, Rafael J. Wysocki wrote:
> On Thu, Jan 21, 2021 at 10:14 AM Pingfan Liu <kernelfans@gmail.com> wrote:
> >
> > On Wed, Jan 20, 2021 at 5:30 PM Baoquan He <bhe@redhat.com> wrote:
> > >
> > > Hi,
> > >
> > > On 11/18/20 at 09:59pm, Dan Carpenter wrote:
> > > > Hello Pingfan Liu,
> > > >
> > > > The patch 55f2503c3b69: "PM / reboot: Eliminate race between reboot
> > > > and suspend" from Jul 31, 2018, leads to the following static checker
> > > > warning:
> > > >
> > > >       kernel/power/main.c:27 lock_system_sleep()
> > > >       warn: called with lock held.  '&system_transition_mutex'
> > >
> > > This is a good finding. I think we can simply remove the lock/unlock
> > > pair of system_transition_mutex in kernel_kexec() function. The dead
> > > lock should be easily triggered, but it hasn't caused any failure report
> > > because the feature 'kexec jump' is almost not used by anyone as far as
> > > I know. We may need to find out who is using it and where it's used
> > > through an inquiry. Before that, we can just remove the lock operation
> > > inside CONFIG_KEXEC_JUMP ifdeffery scope. Thanks.
> > >
> > >
> > > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> > > index 80905e5aa8ae..a0b6780740c8 100644
> > > --- a/kernel/kexec_core.c
> > > +++ b/kernel/kexec_core.c
> > > @@ -1134,7 +1134,6 @@ int kernel_kexec(void)
> > >
> > >  #ifdef CONFIG_KEXEC_JUMP
> > >         if (kexec_image->preserve_context) {
> > > -               lock_system_sleep();
> > >                 pm_prepare_console();
> > >                 error = freeze_processes();
> > >                 if (error) {
> > > @@ -1197,7 +1196,6 @@ int kernel_kexec(void)
> > >                 thaw_processes();
> > >   Restore_console:
> > >                 pm_restore_console();
> > > -               unlock_system_sleep();
> >
> > This should work since the only caller syscall_reboot has already
> > placed kernel_kexec() under the protection of system_transition_mutex.
> >
> > Thanks for the fix.
> >
> > Reviewed-by: Pingfan Liu <kernelfans@gmail.com>
> 
> OK, so can anyone please submit that patch formally (Cc linux-pm, please)?

I will submit a patch with Pingfan's ack, thanks.


WARNING: multiple messages have this Message-ID (diff)
From: Baoquan He <bhe@redhat.com>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Kexec Mailing List <kexec@lists.infradead.org>,
	Dave Young <dyoung@redhat.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	Pingfan Liu <kernelfans@gmail.com>,
	Linux PM <linux-pm@vger.kernel.org>
Subject: Re: [bug report] PM / reboot: Eliminate race between reboot and suspend
Date: Fri, 22 Jan 2021 15:38:59 +0800	[thread overview]
Message-ID: <20210122073859.GA4797@MiWiFi-R3L-srv> (raw)
In-Reply-To: <CAJZ5v0jgTyXQxPVQoM4nykk7gcWfNw9bXTQO8856vkYD8TJOCg@mail.gmail.com>

On 01/21/21 at 03:42pm, Rafael J. Wysocki wrote:
> On Thu, Jan 21, 2021 at 10:14 AM Pingfan Liu <kernelfans@gmail.com> wrote:
> >
> > On Wed, Jan 20, 2021 at 5:30 PM Baoquan He <bhe@redhat.com> wrote:
> > >
> > > Hi,
> > >
> > > On 11/18/20 at 09:59pm, Dan Carpenter wrote:
> > > > Hello Pingfan Liu,
> > > >
> > > > The patch 55f2503c3b69: "PM / reboot: Eliminate race between reboot
> > > > and suspend" from Jul 31, 2018, leads to the following static checker
> > > > warning:
> > > >
> > > >       kernel/power/main.c:27 lock_system_sleep()
> > > >       warn: called with lock held.  '&system_transition_mutex'
> > >
> > > This is a good finding. I think we can simply remove the lock/unlock
> > > pair of system_transition_mutex in kernel_kexec() function. The dead
> > > lock should be easily triggered, but it hasn't caused any failure report
> > > because the feature 'kexec jump' is almost not used by anyone as far as
> > > I know. We may need to find out who is using it and where it's used
> > > through an inquiry. Before that, we can just remove the lock operation
> > > inside CONFIG_KEXEC_JUMP ifdeffery scope. Thanks.
> > >
> > >
> > > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> > > index 80905e5aa8ae..a0b6780740c8 100644
> > > --- a/kernel/kexec_core.c
> > > +++ b/kernel/kexec_core.c
> > > @@ -1134,7 +1134,6 @@ int kernel_kexec(void)
> > >
> > >  #ifdef CONFIG_KEXEC_JUMP
> > >         if (kexec_image->preserve_context) {
> > > -               lock_system_sleep();
> > >                 pm_prepare_console();
> > >                 error = freeze_processes();
> > >                 if (error) {
> > > @@ -1197,7 +1196,6 @@ int kernel_kexec(void)
> > >                 thaw_processes();
> > >   Restore_console:
> > >                 pm_restore_console();
> > > -               unlock_system_sleep();
> >
> > This should work since the only caller syscall_reboot has already
> > placed kernel_kexec() under the protection of system_transition_mutex.
> >
> > Thanks for the fix.
> >
> > Reviewed-by: Pingfan Liu <kernelfans@gmail.com>
> 
> OK, so can anyone please submit that patch formally (Cc linux-pm, please)?

I will submit a patch with Pingfan's ack, thanks.


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

  reply	other threads:[~2021-01-22  7:40 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-18 18:59 [bug report] PM / reboot: Eliminate race between reboot and suspend Dan Carpenter
2020-11-18 18:59 ` Dan Carpenter
2021-01-20  9:30 ` Baoquan He
2021-01-20  9:30   ` Baoquan He
2021-01-21  9:10   ` Pingfan Liu
2021-01-21  9:10     ` Pingfan Liu
2021-01-21 14:42     ` Rafael J. Wysocki
2021-01-21 14:42       ` Rafael J. Wysocki
2021-01-22  7:38       ` Baoquan He [this message]
2021-01-22  7:38         ` Baoquan He
  -- strict thread matches above, loose matches on Subject: below --
2019-11-19  6:18 Dan Carpenter

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=20210122073859.GA4797@MiWiFi-R3L-srv \
    --to=bhe@redhat.com \
    --cc=dan.carpenter@oracle.com \
    --cc=dyoung@redhat.com \
    --cc=kernelfans@gmail.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rafael@kernel.org \
    /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.