All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tao Cui <cui.tao@linux.dev>
To: Bart Van Assche <bvanassche@acm.org>, axboe@kernel.dk, hch@lst.de
Cc: cui.tao@linux.dev, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org, Tao Cui <cuitao@kylinos.cn>
Subject: Re: [PATCH] loop: defer the queue limits clear to a workqueue
Date: Mon, 31 Aug 2026 21:34:47 +0800	[thread overview]
Message-ID: <db25197c-b25e-4a05-8e1d-894d19c880d4@linux.dev> (raw)
In-Reply-To: <638dd174-99ba-43d9-b379-5ade06982259@acm.org>

Hi Bart,

在 2026/8/29 00:34, Bart Van Assche 写道:
> On 8/28/26 12:20 AM, Tao Cui wrote:
>> loop_clear_limits() calls queue_limits_commit_update() directly from
>> the loop workqueue that processes the request.  That does a
>> non-atomic struct assignment to q->limits without freezing the queue,
>> which races with lockless readers of q->limits on other CPUs - bio
>> splitting reads max_hw_sectors, the discard path reads
>> max_hw_discard_sectors - and can let them observe torn values.  The
>> trigger is a discard or write-zeroes request on a loop device whose
>> backing file does not support the corresponding fallocate operation.
>>
>> The code already has an XXX comment saying this should move to a
>> workqueue.  Do that: schedule a work item on the system workqueue,
>> where it is safe to freeze the queue and update the limits using
>> queue_limits_commit_update_frozen().  Accumulate pending modes in
>> lo->clear_limits_mode so that failures between scheduling and
>> execution of the work item are not lost, and cancel the work item
>> before the device is freed.  If the device is reconfigured to a
>> backing file that does support the operation in that window, the
>> stale clear takes effect and discard is disabled until the next
>> reconfiguration.
> Please help with reviewing this patch, which seems more complete to me
> than this patch:
> https://lore.kernel.org/linux-block/c2ab2547-63b3-48cf-87c1-fc53219e360a@I-love.SAKURA.ne.jp/
> 

Thanks for the pointer.  The two patches fix different races though,
so they are not alternatives to each other.

Tetsuo's v7 fixes the teardown path: __loop_clr_fd() racing with
in-flight requests, which is the syzbot NULL deref in lo_rw_aio().
My patch fixes a runtime race: loop_clear_limits() still updates
q->limits from the loop workqueue without freezing the queue when a
discard or write-zeroes request fails with -EOPNOTSUPP, so lockless
readers of q->limits can observe torn values.  The freeze in
Tetsuo's patch only happens at teardown and does not cover that
path - the XXX comment and the unfrozen queue_limits_commit_update()
in loop_clear_limits() are still there in his tree.  The two patches
can coexist.

I'll go review Tetsuo's v7 and reply to his thread.

Thanks,
Tao

> Thanks,
> 
> Bart.


  reply	other threads:[~2026-08-31 13:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-28  7:20 [PATCH] loop: defer the queue limits clear to a workqueue Tao Cui
2026-08-28 16:34 ` Bart Van Assche
2026-08-31 13:34   ` Tao Cui [this message]
2026-08-31 16:25 ` Bart Van Assche
2026-09-01 13:14   ` Tao Cui
2026-09-01 17:29 ` Bart Van Assche

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=db25197c-b25e-4a05-8e1d-894d19c880d4@linux.dev \
    --to=cui.tao@linux.dev \
    --cc=axboe@kernel.dk \
    --cc=bvanassche@acm.org \
    --cc=cuitao@kylinos.cn \
    --cc=hch@lst.de \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.