Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: System hangs if NVMe/SSD is removed during suspend
To:     Mika Westerberg <mika.westerberg@linux.intel.com>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc:     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
References: <20191002122136.GD2819@lahna.fi.intel.com>
 <20191003165033.GC3247445@devbig004.ftw2.facebook.com>
 <20191004080340.GB2819@lahna.fi.intel.com> <2367934.HCQFgJ56tP@kreacher>
 <20191004110151.GH2819@lahna.fi.intel.com>
From:   Jens Axboe <axboe@kernel.dk>
Message-ID: <99b3ffb8-4205-9795-a48a-09125f5fceec@kernel.dk>
Date:   Fri, 4 Oct 2019 07:32:40 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <20191004110151.GH2819@lahna.fi.intel.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.

-- 
Jens Axboe