linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Jens Axboe <axboe@kernel.dk>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Tejun Heo <tj@kernel.org>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	AceLan Kao <acelan.kao@canonical.com>, Jan Kara <jack@suse.cz>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org
Subject: Re: System hangs if NVMe/SSD is removed during suspend
Date: Mon, 7 Oct 2019 12:08:38 +0200	[thread overview]
Message-ID: <20191007100838.GA24366@quack2.suse.cz> (raw)
In-Reply-To: <99b3ffb8-4205-9795-a48a-09125f5fceec@kernel.dk>

On Fri 04-10-19 07:32:40, Jens Axboe wrote:
> On 10/4/19 5:01 AM, Mika Westerberg wrote:
> > On Fri, Oct 04, 2019 at 11:59:26AM +0200, Rafael J. Wysocki wrote:
> >> On Friday, October 4, 2019 10:03:40 AM CEST Mika Westerberg wrote:
> >>> On Thu, Oct 03, 2019 at 09:50:33AM -0700, Tejun Heo wrote:
> >>>> Hello, Mika.
> >>>>
> >>>> On Wed, Oct 02, 2019 at 03:21:36PM +0300, Mika Westerberg wrote:
> >>>>> but from that discussion I don't see more generic solution to be
> >>>>> implemented.
> >>>>>
> >>>>> Any ideas we should fix this properly?
> >>>>
> >>>> Yeah, the only fix I can think of is not using freezable wq.  It's
> >>>> just not a good idea and not all that difficult to avoid using.
> >>>
> >>> OK, thanks.
> >>>
> >>> In that case I will just make a patch that removes WQ_FREEZABLE from
> >>> bdi_wq and see what people think about it :)
> >>
> >> I guess that depends on why WQ_FREEZABLE was added to it in the first place. :-)
> >>
> >> The reason might be to avoid writes to persistent storage after creating an
> >> image during hibernation, since wqs remain frozen throughout the entire
> >> hibernation including the image saving phase.
> > 
> > Good point.
> > 
> >> Arguably, making the wq freezable is kind of a sledgehammer approach to that
> >> particular issue, but in principle it may prevent data corruption from
> >> occurring, so be careful there.
> > 
> > I tried to find the commit that introduced the "freezing" and I think it
> > is this one:
> > 
> >    03ba3782e8dc writeback: switch to per-bdi threads for flushing data
> > 
> > Unfortunately from that commit it is not clear (at least to me) why it
> > calls set_freezable() for the bdi task. It does not look like it has
> > anything to do with blocking writes to storage while entering
> > hibernation but I may be mistaken.
> 
> Wow, a decade ago...
> 
> Honestly, I don't recall why these were marked freezable, and as I wrote
> in the other reply, I don't think there's a good reason for that to be
> the case.

Well, cannot it happen that the flush worker will get stuck in D state
because some subsystem is already suspended and thus hibernation fails
(because AFAIK processes in uninterruptible sleep block hibernation)?

I was also somewhat worried that the hibernation image could be
inconsistent if flush workers do something while hibernation image is being
taken but that does not seem to be a valid concern as all kernel processes
get frozen before hibernation image is taken.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

  reply	other threads:[~2019-10-07 10:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02 12:21 System hangs if NVMe/SSD is removed during suspend Mika Westerberg
2019-10-03 16:50 ` Tejun Heo
2019-10-04  8:03   ` Mika Westerberg
2019-10-04  9:59     ` Rafael J. Wysocki
2019-10-04 11:01       ` Mika Westerberg
2019-10-04 13:32         ` Jens Axboe
2019-10-07 10:08           ` Jan Kara [this message]
2019-10-09 13:22             ` 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=20191007100838.GA24366@quack2.suse.cz \
    --to=jack@suse.cz \
    --cc=acelan.kao@canonical.com \
    --cc=axboe@kernel.dk \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=rjw@rjwysocki.net \
    --cc=tj@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).