linux-kernel - Re: [PATCH v8 10/12] blk-mq: use hk cpus only when isolcpus=io

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 for Android: free password hash cracker in your pocket

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

Message-ID: <87cy7vrbc4.ffs@tglx>
Date: Fri, 12 Sep 2025 16:31:55 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Daniel Wagner <dwagner@...e.de>
Cc: Hannes Reinecke <hare@...e.de>, Daniel Wagner <wagi@...nel.org>, Jens
 Axboe <axboe@...nel.dk>, Keith Busch <kbusch@...nel.org>, Christoph
 Hellwig <hch@....de>, Sagi Grimberg <sagi@...mberg.me>, "Michael S.
 Tsirkin" <mst@...hat.com>, Aaron Tomlin <atomlin@...mlin.com>, "Martin K.
 Petersen" <martin.petersen@...cle.com>, Costa Shulyupin
 <costa.shul@...hat.com>, Juri Lelli <juri.lelli@...hat.com>, Valentin
 Schneider <vschneid@...hat.com>, Waiman Long <llong@...hat.com>, Ming Lei
 <ming.lei@...hat.com>, Frederic Weisbecker <frederic@...nel.org>, Mel
 Gorman <mgorman@...e.de>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, linux-kernel@...r.kernel.org,
 linux-block@...r.kernel.org, linux-nvme@...ts.infradead.org,
 megaraidlinux.pdl@...adcom.com, linux-scsi@...r.kernel.org,
 storagedev@...rochip.com, virtualization@...ts.linux.dev,
 GR-QLogic-Storage-Upstream@...vell.com
Subject: Re: [PATCH v8 10/12] blk-mq: use hk cpus only when
 isolcpus=io_queue is enabled

On Fri, Sep 12 2025 at 10:32, Daniel Wagner wrote:
> On Wed, Sep 10, 2025 at 10:20:26AM +0200, Thomas Gleixner wrote:
>> > The cpu_online_mask might change over time, it's not a static bitmap.
>> > Thus it's necessary to update the blk_hk_online_mask. Doing some sort of
>> > caching is certainly possible. Given that we have plenty of cpumask
>> > logic operation in the cpu_group_evenly code path later, I am not so
>> > sure this really makes a huge difference.
>> 
>> Sure,  but none of this is serialized against CPU hotplug operations. So
>> the resulting mask, which is handed into the spreading code can be
>> concurrently modified. IOW it's not as const as the code claims.
>
> Thanks for explaining.
>
> In group_cpu_evenly:
>
> 	/*
> 	 * Make a local cache of 'cpu_present_mask', so the two stages
> 	 * spread can observe consistent 'cpu_present_mask' without holding
> 	 * cpu hotplug lock, then we can reduce deadlock risk with cpu
> 	 * hotplug code.
> 	 *
> 	 * Here CPU hotplug may happen when reading `cpu_present_mask`, and
> 	 * we can live with the case because it only affects that hotplug
> 	 * CPU is handled in the 1st or 2nd stage, and either way is correct
> 	 * from API user viewpoint since 2-stage spread is sort of
> 	 * optimization.
> 	 */
> 	cpumask_copy(npresmsk, data_race(cpu_present_mask));

The present mask is very different from the online mask. The present
mask only changes on physical hotplug when:

     - a offline CPU is removed from the present set of CPUs

     - a offline CPU is added to it.

In neither case the CPU can be involved in any operation related to the
actual offline/online operations.

Also contrary to your approach, this code takes the possibility of
a concurrently changing mask into account by taking a racy snapshot,
which is immutable for the following operation.

What you are doing with that static mask, makes it a target of
concurrent modification, which is obviously a recipe for subtle bugs.

>   Turns out the two stage spread just needs consistent 'cpu_present_mask',
>   and remove the CPU hotplug lock by storing it into one local cache.  This
>   way doesn't change correctness, because all CPUs are still covered.
>
> This sounds like I should do something similar with cpu_online_mask.

Indeed.

Thanks,

        tglx