Re: [PATC] block: update queue limits atomically

[Ming Lei <ming.lei@redhat.com>]

On Tue, Mar 18, 2025 at 03:26:10PM +0100, Mikulas Patocka wrote:
> The block limits may be read while they are being modified. The statement

It is supposed to not be so for IO path, that is why queue is usually down
or frozen when updating limit.

For other cases, limit lock can be held for sync the read/write.

Or you have cases not covered by both queue freeze and limit lock?

> "q->limits = *lim" is not really atomic. The compiler may turn it into
> memcpy (clang does).
> 
> On x86-64, the kernel uses the "rep movsb" instruction for memcpy - it is
> optimized on modern CPUs, but it is not atomic, it may be interrupted at
> any byte boundary - and if it is interrupted, the readers may read
> garbage.
> 
> On sparc64, there's an instruction that zeroes a cache line without
> reading it from memory. The kernel memcpy implementation uses it (see
> b3a04ed507bf) to avoid loading the destination buffer from memory. The
> problem is that if we copy a block of data to q->limits and someone reads
> it at the same time, the reader may read zeros.
> 
> This commit changes it to use WRITE_ONCE, so that individual words are
> updated atomically.

It isn't necessary, for this particular problem, it is also fragile to
provide atomic word update in this low level way, such as, what if
sizeof(struct queue_limits) isn't 8byte aligned?

> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Cc: stable@vger.kernel.org

stable often requires bug description.



Thanks,
Ming