linux-kernel - Re: [PATCH v7 3/4] driver core: shut down devices asynchronously

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240702050414.GA22160@lst.de>
Date: Tue, 2 Jul 2024 07:04:14 +0200
From: Christoph Hellwig <hch@....de>
To: stuart hayes <stuart.w.hayes@...il.com>
Cc: Christoph Hellwig <hch@....de>, linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J . Wysocki" <rafael@...nel.org>,
	Martin Belanger <Martin.Belanger@...l.com>,
	Oliver O'Halloran <oohall@...il.com>,
	Daniel Wagner <dwagner@...e.de>, Keith Busch <kbusch@...nel.org>,
	Lukas Wunner <lukas@...ner.de>, David Jeffery <djeffery@...hat.com>,
	Jeremy Allison <jallison@....com>, Jens Axboe <axboe@...com>,
	Sagi Grimberg <sagi@...mberg.me>, linux-nvme@...ts.infradead.org
Subject: Re: [PATCH v7 3/4] driver core: shut down devices asynchronously

On Mon, Jul 01, 2024 at 12:57:40PM -0500, stuart hayes wrote:
>> We discussed this before, but there is no summary of it and I of course
>> forgot the conclusion:
>>
>>   - why don't we do this by default?
>
> It is done by default in this version, for devices whose drivers opt-in.
>
> In the previous discussion, you mentioned that you thought "safe" was the
> only sensible option (where "safe" was driver opt-in to async shutdown)...
> that is the default (and only) option with this version.  Greg K-H also
> requested opt-in as well, and suggested that "on" (driver opt-out) could
> be removed.
>
>>   - why is it safe to user enable it?
>
> I guess it isn't necessarily safe, if there are any drivers that can't
> handle their devices shutting down asynchronously. I thought it would be
> nice to be able to enable driver opt-in from user space for testing, before
> changing the default setting for the driver.

I was mostly getting into the contradiction that either we think the
async shutdown is safe everywhere, in which case we don't need a driver
opt-in, or it is not, in which case allowing user to just enabled it
also seems like a bad idea.

> I can correct these lines. I thought that an 80 character line length limit
> was no longer required, and saw another line a few lines above these that was
> even longer... and the checkpatch script didn't flag it either.

checkpatch is unfortunately completely broken, it flags totally harmless
things and doesn't catch other things.  > 80 characters are allowed for
individual lines where it improves readability.  The exact definition
of that depends on the maintainers and reviewers, but outside of
string constants I can't really find anything where it does.