* Re: [PATCH 18/27] loop: Add lock context annotations
From: Nilay Shroff @ 2026-06-10 9:21 UTC (permalink / raw)
To: Bart Van Assche, Jens Axboe
Cc: linux-block, Christoph Hellwig, Marco Elver, Nathan Chancellor
In-Reply-To: <1391f73ea3af981cd2e76b56cdfe47b5bb56ffa0.1781042470.git.bvanassche@acm.org>
On 6/10/26 3:35 AM, Bart Van Assche wrote:
> Add lock context annotations that are compatible with Clang.
>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/block/loop.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 1a33536000e0..ff7eff102c5a 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -106,6 +106,8 @@ static DEFINE_MUTEX(loop_validate_mutex);
> * loop_configure()/loop_change_fd()/__loop_clr_fd() calls.
> */
> static int loop_global_lock_killable(struct loop_device *lo)
> + __cond_acquires(0, &loop_validate_mutex)
> + __cond_acquires(0, &lo->lo_mutex)
> {
> int err;
>
> @@ -124,6 +126,8 @@ static int loop_global_lock_killable(struct loop_device *lo)
> * @lo: struct loop_device
> */
> static void loop_global_unlock(struct loop_device *lo)
> + __releases(&lo->lo_mutex)
> + __releases(&loop_validate_mutex)
> {
> mutex_unlock(&lo->lo_mutex);
> mutex_unlock(&loop_validate_mutex);
>
This looks good to me.
One thing I noticed while looking through the loop driver is that it also defines
@loop_ctl_mutex, which protects @loop_index_idr. It might be worth annotating
@loop_index_idr with `__guarded_by(&loop_ctl_mutex) as well so that Clang can
validate accesses to the IDR against the corresponding locking requirements.
Thanks,
--Nilay
^ permalink raw reply
* Re: [PATCH] rust: block: require `Sync` for `Operations::QueueData`
From: Gary Guo @ 2026-06-10 10:01 UTC (permalink / raw)
To: Andreas Hindborg, Boqun Feng, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Daniel Almeida
Cc: linux-block, rust-for-linux, linux-kernel
In-Reply-To: <20260608-queue-data-sync-v1-1-0efff051aaf3@kernel.org>
On Mon Jun 8, 2026 at 9:24 AM BST, Andreas Hindborg wrote:
> The queue data installed in a `GenDisk` is stored in the request queue and
> handed back to the driver as a shared borrow through the `queue_rq` and
> `commit_rqs` callbacks. Both callbacks obtain that borrow via
> `ForeignOwnable::borrow` and may execute concurrently on several CPUs,
> since the block layer runs one hardware queue per CPU. That means a shared
> reference to the same queue data can be live on multiple threads at once,
> which is only sound when the referent is `Sync`.
>
> The initial `GenDisk` private data support omitted this bound, so a
> driver could install a non-`Sync` type as queue data and then access
> it concurrently from multiple CPUs without synchronization. Add a
> `Sync` bound to the `QueueData` associated type to rule that out.
>
> Fixes: 90d952fac8ac ("rust: block: add `GenDisk` private data support")
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> rust/kernel/block/mq/operations.rs | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
^ permalink raw reply
* Re: [PATCH v2] rust: add procedural macro for declaring configfs attributes
From: Gary Guo @ 2026-06-10 10:05 UTC (permalink / raw)
To: Malte Wechter, Gary Guo, Andreas Hindborg, Breno Leitao,
Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Jens Axboe
Cc: linux-kernel, rust-for-linux, linux-block
In-Reply-To: <f6a513e8-227f-4b22-bd4d-81d88536eff1@gmail.com>
On Wed Jun 10, 2026 at 10:15 AM BST, Malte Wechter wrote:
>>> +
>>> + Ok(ConfigfsAttrs {
>>> + container,
>>> + data,
>>> + child,
>>> + attributes,
>>> + })
>>> + }
>>> +}
>>> +
>>> +fn make_static_ident<T: ToTokens>(ty: &T, suffix: &str) -> syn::Ident {
>>> + let raw_id = quote! { #ty }.to_string();
>>> +
>>> + // Sanitizing syn::Type::Path, this is safe since it is
>>> + // only used as the identifier.
>>> + let normalized = raw_id
>>> + .split("::")
>>> + .map(|s| String::from(s.trim()))
>>> + .reduce(|a, b| format!("{a}_{b}"))
>>> + .expect("Cannot be empty")
>>> + .to_uppercase()
>>> + .replace(|c: char| !c.is_alphanumeric(), "_");
>>> +
>>> + Ident::new(&format!("{}_{}", normalized, suffix), ty.span())
>>> +}
>>> +
>>> +pub(crate) fn configfs_attrs(cfs_attrs: ConfigfsAttrs) -> proc_macro2::TokenStream {
>>> + let (container_ty, data_ty) = (&cfs_attrs.container, &cfs_attrs.data);
>>> +
>>> + let data_tp_ident = make_static_ident(data_ty, "TPE");
>>> + let data_attr_ident = make_static_ident(data_ty, "ATTR_LIST");
>> Instead of creating identifers like this, just scope them properly so that
>> a fixed identifier is used without colliding.
>
> I believe it is nice to keep the identifiers unique, especially if you
> have many fields
> you can easily differentiate them.
What make_static_ident is doing I think is quite hacky. I'd rather not having
them. Plus, the identifiers are not seen by users anyway.
Best,
Gary
^ permalink raw reply
* Re: [PATCH v1] floppy: Drop unused pnp driver data
From: Denis Efremov (Oracle) @ 2026-06-10 10:59 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub), Jens Axboe
Cc: linux-block, linux-kernel
In-Reply-To: <99dbf851ffb99229ea1dcfd8f58e9ee6a1f05349.1781075967.git.u.kleine-koenig@baylibre.com>
On 10/06/2026 11:27, Uwe Kleine-König (The Capable Hub) wrote:
> The pnp_device_id array is only used for module data to support
> auto-loading the floppy module. So the .driver_data member is unused and
> this assignment can be dropped.
>
> While touching that array, align the coding style to what is used most
> for these.
>
> This patch doesn't modify the compiled array, only its representation
> in source form benefits. The former was confirmed with x86 and arm64
> builds.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Reviewed-by: Denis Efremov (Oracle) <efremov@linux.com>
> ---
> Hello,
>
> I intend to change struct pnp_device_id and for that it's required that
> .driver_data is not assigned using a list initializer. Instead of
> converting it to a named initializer, drop the assignment as the value
> is unused. So this is both a cleanup and a preparation for a later patch
> series.
>
> See
> https://lore.kernel.org/all/cover.1779878004.git.u.kleine-koenig@baylibre.com/
> for a more verbose description on the mentioned change to struct
> pnp_device_id (though this is about platform_device_id, but the idea
> here is the same).
>
> Best regards
> Uwe
>
> drivers/block/floppy.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index 0509746f8aed..dca495be0683 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -5012,8 +5012,8 @@ MODULE_LICENSE("GPL");
>
> /* This doesn't actually get used other than for module information */
> static const struct pnp_device_id floppy_pnpids[] = {
> - {"PNP0700", 0},
> - {}
> + { .id = "PNP0700" },
> + { }
> };
>
> MODULE_DEVICE_TABLE(pnp, floppy_pnpids);
>
> base-commit: 49e02880ec0a8c378e811bc9d85da188d7c6204c
^ permalink raw reply
* Re: [PATCH 00/27] Enable lock context analysis in drivers/block/
From: Christoph Hellwig @ 2026-06-10 11:30 UTC (permalink / raw)
To: Marco Elver; +Cc: Christoph Hellwig, Bart Van Assche, Jens Axboe, linux-block
In-Reply-To: <CANpmjNORsF2oExcOphnGgnTt3hJEWP0tB6zua0sTG=n7gJbEZA@mail.gmail.com>
Cool, thanks!
Bart: maytbe for next version just enable it on a per-driver basis.
Once all are covered we can switch to directory-wide.
^ permalink raw reply
* Re: [PATCH v1] floppy: Drop unused pnp driver data
From: Jens Axboe @ 2026-06-10 12:28 UTC (permalink / raw)
To: Denis Efremov, Uwe Kleine-König (The Capable Hub)
Cc: linux-block, linux-kernel
In-Reply-To: <99dbf851ffb99229ea1dcfd8f58e9ee6a1f05349.1781075967.git.u.kleine-koenig@baylibre.com>
On Wed, 10 Jun 2026 09:27:55 +0200, Uwe Kleine-König (The Capable Hub) wrote:
> The pnp_device_id array is only used for module data to support
> auto-loading the floppy module. So the .driver_data member is unused and
> this assignment can be dropped.
>
> While touching that array, align the coding style to what is used most
> for these.
>
> [...]
Applied, thanks!
[1/1] floppy: Drop unused pnp driver data
commit: 64e335790272d9b0468af3a70e34f377924b156f
Best regards,
--
Jens Axboe
^ permalink raw reply
* Re: configurable block error injection v4
From: Christoph Hellwig @ 2026-06-10 13:11 UTC (permalink / raw)
To: Jens Axboe
Cc: Jonathan Corbet, Damien Le Moal, Hannes Reinecke, Keith Busch,
linux-block, linux-doc
In-Reply-To: <20260610051015.1906799-1-hch@lst.de>
Sashiko had two comments on this:
- the one about the race of removal vs. addition is spot on, and
trivially fixed by actually removing the code (unlock/lock cycle).
- the other about zero-sized commands is valid, but more of an
enhancement. And one that if implemented right now actually
make things worse, as flushed actually show up as empty writes
with the preflush bit and not as REQ_OP_FLUSH. So if we'd special
case zero-sized bios, we'd make a flush hit all write rules,
which would defeat the point. We really need to do flushes as
REQ_OP_FLUSH at the bio level :(
Below is what I plan to fold in, and I'm thinking of the empty bio
issue above can be caught in a comment or the documentation nicely.
diff --git a/block/error-injection.c b/block/error-injection.c
index 7f7f0d3327bc..3bc91f199dc7 100644
--- a/block/error-injection.c
+++ b/block/error-injection.c
@@ -119,11 +119,7 @@ static void error_inject_removeall(struct gendisk *disk)
while ((inj = list_first_entry_or_null(&disk->error_injection_list,
struct blk_error_inject, entry))) {
list_del_rcu(&inj->entry);
- mutex_unlock(&disk->error_injection_lock);
-
kfree_rcu_mightsleep(inj);
-
- mutex_lock(&disk->error_injection_lock);
}
static_branch_dec(&blk_error_injection_enabled);
mutex_unlock(&disk->error_injection_lock);
^ permalink raw reply related
* Re: [PATCH v7 00/43] btrfs: add fscrypt support
From: Daniel Vacek @ 2026-06-10 13:19 UTC (permalink / raw)
To: Eric Biggers
Cc: David Sterba, linux-block, linux-fscrypt, linux-btrfs,
linux-kernel, Chris Mason, Josef Bacik, Theodore Y. Ts'o,
Jaegeuk Kim, Jens Axboe
In-Reply-To: <20260531002812.GA2302@sol>
On Sun, 31 May 2026 at 02:29, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Fri, May 22, 2026 at 09:00:46AM +0200, Daniel Vacek wrote:
> > Hi Eric,
> >
> > This is just a gentle ping.
> > I was wondering if you had a chance to look at this version?
> > I believe all your previous feedback has been addressed and this
> > version is solid.
> > Please, let me know your thoughts.
> >
> > Regards,
> > Daniel
>
> It's been really hard to find time to review this huge patchset. I've
> started going through it and will try to leave comments next week.
Hi Eric,
First, thank you for looking into this.
> In the mean time it would be really helpful if you went through the
> Sashiko reviews
> (https://sashiko.dev/#/patchset/20260513085340.3673127-1-neelx%40suse.com)
> and address the ones that make sense to. It found 93 issues including
> 16 critical ones, which is kind of a lot. Some of them are the same
> things I'm noticing already. Same for the issue that Christoph noticed
> where new devices can be added; Sashiko had already found that too.
>
> If I'm going to have to use my limited human review time to point out
> issues that were already found, that's not a great use of time.
I already went through some of the Sashiko reviews (they were slowly
coming up one by one) before leaving for my vacation. And I found them
mostly confusing or misleading. I'm gonna have a look into the rest I
haven't seen yet to see if there are any useful points. And I'll
compare it to your notes to see if it was Sashiko being confused or if
it was me.
> I also don't see any information about how this was tested (and will
> continue to be tested in the future).
I'm using the `encrypt` group of xfstests as the acceptance criteria
and the full test run to ensure no regressions.
General support for btrfs had to be added. And since we only support
the v2 fscrypt policy, some tests had to be split into two tests - one
testing v1 and the other testing v2 policy.
There are also additional new btrfs specific tests (reflinks,
snapshots). xfstests also need fscrypt support in btrfs-progs with
related changes to export nonces and other metadata like device
offsets of encrypted blocks for validation.
I'll send the fscrypt updated btrfs-progs and xfstests next. I just
need to clean them up a bit first. I'm sorry about the delay, but I
only managed to post the kernel part before my vacation. I was hoping
it would give you all more time to review.
Thanks,
Daniel
> - Eric
^ permalink raw reply
* Re: [PATCH v2 23/83] block: rnull: add discard support
From: Malte Wechter @ 2026-06-10 13:55 UTC (permalink / raw)
To: Andreas Hindborg, Liam R. Howlett, Alice Ryhl, Anna-Maria Behnsen,
Benno Lossin, Björn Roy Baron, Boqun Feng, Danilo Krummrich,
FUJITA Tomonori, Frederic Weisbecker, Gary Guo, Jens Axboe,
John Stultz, Lorenzo Stoakes, Lyude Paul, Miguel Ojeda,
Stephen Boyd, Thomas Gleixner, Trevor Gross
Cc: linux-block, linux-kernel, linux-mm, rust-for-linux
In-Reply-To: <20260609-rnull-v6-19-rc5-send-v2-23-82c7404542e2@kernel.org>
On Tue Jun 9, 2026 at 9:08 PM CEST, Andreas Hindborg wrote:
> Add support for discard operations to the rnull block driver:
> - Add discard module parameter and configfs attribute.
> - Set max_hw_discard_sectors when discard is enabled.
> - Add sector occupancy tracking.
> - Add discard handling that frees sectors and removes empty pages.
> - Discard operations require memory backing to function.
>
> The discard feature uses a bitmap to track which sectors in each page are
> occupied, allowing cleanup of pages when they are empty.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> drivers/block/rnull/configfs.rs | 15 +++++
> drivers/block/rnull/rnull.rs | 120 +++++++++++++++++++++++++++++++++++-----
> 2 files changed, 121 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
> index 2f3fa81ea121..e47399cd45a4 100644
> --- a/drivers/block/rnull/configfs.rs
> +++ b/drivers/block/rnull/configfs.rs
...
> }
> })
> );
> +
> +configfs_attribute!(DeviceConfig, 10,
> + show: |this, page| show_field(this.data.lock().discard, page),
> + store: |this, page| store_with_power_check(this, page, |data, page| {
> + if !data.memory_backed {
> + return Err(EINVAL);
> + }
> + data.discard = kstrtobool_bytes(page)?;
> + Ok(())
> + })
> +);
Should it be ok to set 'discard' to 0 if 'emory_backed' is not set?
In the C null_blk driver, 'discard' defaults to 0 if 'memory_backed' is not set,
it is also ignored (and defaulted to 0) if 'zoned' is enabled.
Best regards,
Malte
^ permalink raw reply
* Re: [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state
From: Andy Shevchenko @ 2026-06-10 14:43 UTC (permalink / raw)
To: Kaitao Cheng
Cc: Christian König, Thierry Reding, Jonathan Hunter,
Sowjanya Komatineni, Davidlohr Bueso, Paul E . McKenney,
Josh Triplett, Peter Zijlstra, Ingo Molnar, Will Deacon,
Boqun Feng, Liam Girdwood, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Neil Armstrong, Robert Foss, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Matthew Auld, Matthew Brost,
Waiman Long, drbd-dev, linux-block, linux1394-devel, dri-devel,
intel-gfx, linux-spi, linux-stm32, linux-arm-kernel, linux-tegra,
linux-sound, linux-kernel, Andrew Morton, Randy Dunlap,
Christian Brauner, David Howells, Luca Ceresoli, Kaito Cheng,
Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Jaroslav Kysela, Takashi Iwai
In-Reply-To: <5152089a-2808-4fe9-b633-b03018105dd2@linux.dev>
On Wed, Jun 10, 2026 at 02:14:06PM +0800, Kaitao Cheng wrote:
> 在 2026/6/9 18:33, Christian König 写道:
> > On 6/9/26 08:13, Kaitao Cheng wrote:
> >>
> >> This series prepares for, and then updates, the list_for_each_entry()
> >> family so the common entry iterators cache their next or previous cursor
> >> before the loop body runs.
> >
> > Why in the world would we want to do that?
> >
> > The safe and non-safe variants have very distinct use cases and that is completely intentional.
> >
> > What we could improve maybe is the documentation, from my experience an astonishing large amount of people have misconceptions about the safe variants.
> >
> >> The first 13 patches open-code loops that intentionally depend on the
> >> old "derive the next entry from the current cursor at the end of the
> >> iteration" behaviour. These loops append work to the list being walked,
> >> restart traversal after dropping a lock, skip an entry consumed by the
> >> current iteration, or otherwise adjust the cursor in the loop body.
> >
> > Well I have to clearly reject the changes for subsystems/components I'm maintaining, that just looks horrible to me and I clearly don't see a good reason for that.
>
> Hi Christian and Andy Shevchenko,
>
> Thanks for taking a look. I would like to clarify the point you raised.
>
> The reason I started looking at this is the original motivation behind
> the _safe() variants. They exist because some users need to remove, move
> or otherwise consume the current entry while walking the list. In that
> case the next cursor has to be preserved before the loop body can modify
> the current entry.
>
> The unfortunate part is that this could not be expressed with the
> existing list_for_each_entry() interface without changing its calling
> convention. The _safe() variants had to grow an extra argument for the
> temporary cursor, and that is why we ended up with a separate family of
> macros.
>
> But conceptually, the distinction does not have to be exposed as two
> different iterator families forever. The difference is an implementation
> detail: whether the iterator keeps the next/previous cursor before the
> body runs. This series makes the common list_for_each_entry() iterators
> do that internally, so the safe and non-safe forms can effectively be
> folded together, or at least the need for a separate public _safe()
> interface becomes much weaker.
>
> There is also a usability issue with the current _safe() interface. The
> caller is forced to define a temporary cursor outside the macro and pass
> it in, even though almost all users never use that cursor directly. It is
> just boilerplate required by the macro implementation. I find that
> redundant and awkward: the temporary cursor is an internal detail of the
> iteration, but every caller has to spell it out.
Ah, I think the distinct macro families is that what we want.
But the hiding of the parameter can be done inside list_for_each_*_safe().
You can do a treewide change with coccinelle.
Sorry if I didn't get the whole idea from your previous contributions.
Note, even cases that would need a temporary cursor may be switched to
new list_for_each_*_safe(), see how PCI macros for iterating over resources
are implemented (include/linux/pci.h).
> With the updated list_for_each_entry() implementation, that extra cursor
> can be kept inside the iterator itself. Callers that only want to walk
> the list, including callers that delete or consume the current entry, no
> longer need to carry an otherwise-unused temporary variable just to make
> the macro work.
>
> >> The final patch changes include/linux/list.h to keep a private cursor in
> >> the common entry iterators while preserving the public macro interface.
> >> The safe variants remain available when callers need the temporary
> >> cursor explicitly or have stronger mutation requirements.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2] rust: add procedural macro for declaring configfs attributes
From: Malte Wechter @ 2026-06-10 14:48 UTC (permalink / raw)
To: Gary Guo, Andreas Hindborg, Breno Leitao, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jens Axboe
Cc: linux-kernel, rust-for-linux, linux-block
In-Reply-To: <DJ5A9HIJ43EJ.2K3SCFI7UR3NF@garyguo.net>
On 6/10/26 12:05 PM, Gary Guo wrote:
> On Wed Jun 10, 2026 at 10:15 AM BST, Malte Wechter wrote:
>>>> +
>>>> + Ok(ConfigfsAttrs {
>>>> + container,
>>>> + data,
>>>> + child,
>>>> + attributes,
>>>> + })
>>>> + }
>>>> +}
>>>> +
>>>> +fn make_static_ident<T: ToTokens>(ty: &T, suffix: &str) -> syn::Ident {
>>>> + let raw_id = quote! { #ty }.to_string();
>>>> +
>>>> + // Sanitizing syn::Type::Path, this is safe since it is
>>>> + // only used as the identifier.
>>>> + let normalized = raw_id
>>>> + .split("::")
>>>> + .map(|s| String::from(s.trim()))
>>>> + .reduce(|a, b| format!("{a}_{b}"))
>>>> + .expect("Cannot be empty")
>>>> + .to_uppercase()
>>>> + .replace(|c: char| !c.is_alphanumeric(), "_");
>>>> +
>>>> + Ident::new(&format!("{}_{}", normalized, suffix), ty.span())
>>>> +}
>>>> +
>>>> +pub(crate) fn configfs_attrs(cfs_attrs: ConfigfsAttrs) -> proc_macro2::TokenStream {
>>>> + let (container_ty, data_ty) = (&cfs_attrs.container, &cfs_attrs.data);
>>>> +
>>>> + let data_tp_ident = make_static_ident(data_ty, "TPE");
>>>> + let data_attr_ident = make_static_ident(data_ty, "ATTR_LIST");
>>> Instead of creating identifers like this, just scope them properly so that
>>> a fixed identifier is used without colliding.
>> I believe it is nice to keep the identifiers unique, especially if you
>> have many fields
>> you can easily differentiate them.
> What make_static_ident is doing I think is quite hacky. I'd rather not having
> them. Plus, the identifiers are not seen by users anyway.
>
> Best,
> Gary
I will simplify the naming of the static variables greatly (removing
make_static_ident), but still generate unique identifiers.
Best regards,
Malte
^ permalink raw reply
* Re: [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state
From: Andy Shevchenko @ 2026-06-10 15:02 UTC (permalink / raw)
To: Christian König
Cc: Kaitao Cheng, Thierry Reding, Jonathan Hunter,
Sowjanya Komatineni, Davidlohr Bueso, Paul E . McKenney,
Josh Triplett, Peter Zijlstra, Ingo Molnar, Will Deacon,
Boqun Feng, Liam Girdwood, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Neil Armstrong, Robert Foss, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Matthew Auld, Matthew Brost,
Waiman Long, drbd-dev, linux-block, linux1394-devel, dri-devel,
intel-gfx, linux-spi, linux-stm32, linux-arm-kernel, linux-tegra,
linux-sound, linux-kernel, Andrew Morton, Randy Dunlap,
Christian Brauner, David Howells, Luca Ceresoli, Kaito Cheng,
Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Jaroslav Kysela, Takashi Iwai
In-Reply-To: <d974a2ea-6102-45ff-bf36-3b25a2404e40@amd.com>
On Wed, Jun 10, 2026 at 11:11:34AM +0200, Christian König wrote:
> On 6/10/26 10:18, Kaitao Cheng wrote:
> > 在 2026/6/10 16:07, Christian König 写道:
...
> > Should we revert to v1, or keep list_for_each_entry() and
> > list_for_each_entry_safe() as they are, close this thread, and make no
> > changes?
> >
> > Link to v1:
> > https://lore.kernel.org/all/20260529082149.76764-1-kaitao.cheng@linux.dev/
> >
> > Or do you have any better suggestions?
>
> v1 looks perfectly reasonable to me.
But why not just hiding that once for all (in case they don't use the temporary
iterator)? Easy to automate, robust — everyone is happy?
> You should just include some patches in the same patch set to actually use
> the new macros.
>
> If you modify the files under drivers/dma-buf or drivers/gpu/drm/amd to use
> the new macro I'm happy to review that.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH] iomap: enforce DIO alignment check in iomap]
From: Carlos Maiolino @ 2026-06-10 15:27 UTC (permalink / raw)
To: brauner, linux-block
Sorry, I screwed up two addresses, bouncing them to the right addresses
to avoid spamming the lists again.
----- Forwarded message from cem@kernel.org -----
Date: Wed, 10 Jun 2026 16:52:11 +0200
From: cem@kernel.org
To: brauner@vger.kernel.org
Cc: linux-block@vger.kernel.devel, linux-fsdevel@vger.kernel.org,
linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org, Carlos Maiolino
<cem@kernel.org>, Keith Busch <kbusch@kernel.org>, Hannes Reinecke
<hare@suse.de>, "Martin K. Petersen" <martin.petersen@oracle.com>,
Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>
Subject: [PATCH] iomap: enforce DIO alignment check in iomap
Message-ID: <20260610145218.141369-1-cem@kernel.org>
X-Mailer: git-send-email 2.54.0
Tags: sent
From: Carlos Maiolino <cem@kernel.org>
The DIO alignment check has been lifted from iomap layer to rely on the
block layer to enforce proper alignment when issuing direct IO
operations. This though, depending on the IO size and buffer address
passed to the IO operation may lead to user-visible behavior change.
This has been caught initially by LTP test diotest4 running on
PPC architecture, where the test fails because a read() operation
with a supposedly misaligned buffer succeeds instead of an expected
-EINVAL.
This has no direct relationship with PPC, but seems to do with the
IO size crossing page borders or not.
The test allocates a 4k buffer, and then increments the buffer pointer
by a single byte to enforce a misaligned address. It then issues a 4k
read() using such buffer. The read is supposed to return an -EINVAL but
it ends up succeeding.
The allocated buffer is at least a single page, so the read() size being
smaller will end up most of the time within the very same page initially
allocated which seems to suffice the block layer to accept the IO.
On x86 though, the same 4k read will end up crossing page boundaries
causing a bio_split which ends up properly checking the address and
rejecting it due to misalignment.
The test itself is buggy (which seems by design) because it ends up
attempting to read 4096 bytes into a 4095, but I believe the test
expected the address to be rejected prior to any write attempt.
The problematic behavior is reproducible on x86 by reducing the IO size
to something < PAGE_SIZE, so the misaligned read()s will also be accepted
by the block layer.
Fixing this is just a matter of enforcing daddr and memory
alignment back into iomap.
This behavior is reproducible in ext4 and xfs due to both relying on
iomap layer, btrfs does not present this behavior change as it does its
own DIO alignment checking.
Fixes: 7eac33186957 ("iomap: simplify direct io validity check")
Cc: Keith Busch <kbusch@kernel.org>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
---
While I didn't spot any memory/disk corruption looking into this, it
changes the user behavior that dictates buffer addresses must be
properly aligned when issuing direct IO operations so I thought making
iomap check again for the buffer address alignment is reasonable.
fs/iomap/direct-io.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 95254aa1b654..0064984e64e5 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -400,6 +400,9 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
if ((pos | length) & (alignment - 1))
return -EINVAL;
+ if (iov_iter_alignment(dio->submit.iter) & (alignment - 1))
+ return -EINVAL;
+
if (dio->flags & IOMAP_DIO_WRITE) {
bool need_completion_work = true;
--
2.54.0
----- End forwarded message -----
^ permalink raw reply related
* Re: [PATCH] iomap: enforce DIO alignment check in iomap]
From: Keith Busch @ 2026-06-10 15:37 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: brauner, linux-block
In-Reply-To: <aimCIEpCwUUyoORf@nidhogg.toxiclabs.cc>
On Wed, Jun 10, 2026 at 05:27:42PM +0200, Carlos Maiolino wrote:
> The DIO alignment check has been lifted from iomap layer to rely on the
> block layer to enforce proper alignment when issuing direct IO
> operations. This though, depending on the IO size and buffer address
> passed to the IO operation may lead to user-visible behavior change.
>
> This has been caught initially by LTP test diotest4 running on
> PPC architecture, where the test fails because a read() operation
> with a supposedly misaligned buffer succeeds instead of an expected
> -EINVAL.
It's not supposed to matter where in the stack we determined this be an
invalid request: it should still fail if it's misaligned. Could you
clarify how this is succeeding?
^ permalink raw reply
* Re: [PATCH v3 3/4] block: drop shared-tag fairness throttling
From: Sumit Saxena @ 2026-06-10 15:46 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin K . Petersen, Jens Axboe, James E . J . Bottomley,
linux-scsi, linux-block, Adam Radford, Khalid Aziz,
Adaptec OEM Raid Solutions, Matthew Wilcox, Hannes Reinecke,
Juergen E . Fischer, Russell King, linux-arm-kernel, Finn Thain,
Michael Schmitz, Anil Gurumurthy, Sudarsana Kalluru,
Oliver Neukum, Ali Akcaagac, Jamie Lenehan, Ram Vegesna,
target-devel, Bradley Grove, Satish Kharat, Sesidhar Baddela,
Karan Tilak Kumar, Yihang Li, Don Brace, storagedev,
HighPoint Linux Team, Tyrel Datwyler, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy, linuxppc-dev,
Brian King, Lee Duncan, Chris Leech, Mike Christie, open-iscsi,
Justin Tee, Paul Ely, Kashyap Desai, Shivasharan S,
Chandrakanth Patil, megaraidlinux.pdl, Sathya Prakash Veerichetty,
Sreekanth Reddy, mpi3mr-linuxdrv.pdl, Suganath Prabu Subramani,
Ranjan Kumar, MPT-FusionLinux.pdl, Daniel Palmer, GOTO Masanori,
YOKOTA Hiroshi, Jack Wang, Geoff Levand, Michael Reed,
Nilesh Javali, GR-QLogic-Storage-Upstream, Narsimhulu Musini,
K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
linux-hyperv, Michael S . Tsirkin, Jason Wang, Paolo Bonzini,
Stefan Hajnoczi, Eugenio Perez, virtualization, Vishal Bhakta,
bcm-kernel-feedback-list, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, xen-devel, Bart Van Assche
In-Reply-To: <aikAs4X-2NWTuwCc@infradead.org>
[-- Attachment #1.1: Type: text/plain, Size: 1889 bytes --]
On Wed, Jun 10, 2026 at 11:44 AM Christoph Hellwig <hch@infradead.org>
wrote:
>
> Just dropping the fairness was rejected before and there is no
> explanation here on why any of that has changed.
I missed the fact that this patch had been previously rejected.
The motivation for this change stems from performance issue we
encountered due to false sharing of the 'nr_active_requests_shared_tags'
counter
on certain CPU architectures. I initially submitted a patch to move that
counter to
its own cache line to avoid conflicts with 'nr_requests' and other hot
fields
(see:
https://patchwork.kernel.org/project/linux-scsi/patch/20260402074637.92417-3-sumit.saxena@broadcom.com/
).
During the review, Bart shared his work, which eliminates the
counter entirely by removing the fairness throttling. My testing confirmed
that
this approach resolved the performance issues and improved IOPS.
This patch is part of a larger set, and I have reported the cumulative
performance
improvements in the cover letter.
Thanks,
Sumit
>
> On Tue, Jun 09, 2026 at 05:48:02PM +0530, Sumit Saxena wrote:
> > From: Bart Van Assche <bvanassche@acm.org>
> >
> > Original patch [1] by Bart Van Assche; this version is rebased onto the
> > current tree. In testing it improves IOPS by roughly 16-18% by removing
> > the fair-sharing throttle on shared tag queues.
> >
> > This patch removes the following code and structure members:
> > - The function hctx_may_queue().
> > - blk_mq_hw_ctx.nr_active and
request_queue.nr_active_requests_shared_tags
> > and also all the code that modifies these two member variables.
>
> .. and besides that, this commit message is still entirely useless
> as it doesn't explain any of the thoughts of why this change is safe
> and desirable. While the mechanics above are totally obvious from
> the code change itself.
>
[-- Attachment #1.2: Type: text/html, Size: 2485 bytes --]
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/4] block: drop shared-tag fairness throttling
From: Bart Van Assche @ 2026-06-10 16:03 UTC (permalink / raw)
To: open-iscsi, Christoph Hellwig, Sumit Saxena
Cc: Martin K . Petersen, Jens Axboe, James E . J . Bottomley,
linux-scsi, linux-block
In-Reply-To: <aikAs4X-2NWTuwCc@infradead.org>
On 6/9/26 11:14 PM, Christoph Hellwig wrote:
> Just dropping the fairness was rejected before and there is no
> explanation here on why any of that has changed.
Hmm ... has anyone ever rejected this patch? Jens' latest feedback
(May 2024) is available here:
https://lore.kernel.org/linux-block/7a69eba2-42e4-4c67-8a54-37b5b41675f9@kernel.dk/
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH v3 3/4] block: drop shared-tag fairness throttling
From: Bart Van Assche @ 2026-06-10 16:05 UTC (permalink / raw)
To: open-iscsi, Hannes Reinecke, Sumit Saxena, Martin K . Petersen,
Jens Axboe
Cc: James E . J . Bottomley, linux-scsi, linux-block
In-Reply-To: <93a82831-608d-4462-a019-26b3adc7089c@suse.de>
On 6/9/26 11:18 PM, Hannes Reinecke wrote:
> The whole point of this was to increase fairness between drives, so
> of course removing it will make an individual drive going faster ...
Data that shows that fairness is preserved even with this patch applied
is available here:
https://lore.kernel.org/linux-block/20240529213921.3166462-1-bvanassche@acm.org/
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH v3 3/4] block: drop shared-tag fairness throttling
From: Keith Busch @ 2026-06-10 16:35 UTC (permalink / raw)
To: Sumit Saxena
Cc: Christoph Hellwig, Martin K . Petersen, Jens Axboe,
James E . J . Bottomley, linux-scsi, linux-block, Adam Radford,
Khalid Aziz, Adaptec OEM Raid Solutions, Matthew Wilcox,
Hannes Reinecke, Juergen E . Fischer, Russell King,
linux-arm-kernel, Finn Thain, Michael Schmitz, Anil Gurumurthy,
Sudarsana Kalluru, Oliver Neukum, Ali Akcaagac, Jamie Lenehan,
Ram Vegesna, target-devel, Bradley Grove, Satish Kharat,
Sesidhar Baddela, Karan Tilak Kumar, Yihang Li, Don Brace,
storagedev, HighPoint Linux Team, Tyrel Datwyler,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, linuxppc-dev, Brian King, Lee Duncan,
Chris Leech, Mike Christie, open-iscsi, Justin Tee, Paul Ely,
Kashyap Desai, Shivasharan S, Chandrakanth Patil,
megaraidlinux.pdl, Sathya Prakash Veerichetty, Sreekanth Reddy,
mpi3mr-linuxdrv.pdl, Suganath Prabu Subramani, Ranjan Kumar,
MPT-FusionLinux.pdl, Daniel Palmer, GOTO Masanori, YOKOTA Hiroshi,
Jack Wang, Geoff Levand, Michael Reed, Nilesh Javali,
GR-QLogic-Storage-Upstream, Narsimhulu Musini, K . Y . Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, linux-hyperv,
Michael S . Tsirkin, Jason Wang, Paolo Bonzini, Stefan Hajnoczi,
Eugenio Perez, virtualization, Vishal Bhakta,
bcm-kernel-feedback-list, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, xen-devel, Bart Van Assche
In-Reply-To: <CAL2rwxr1uGshb1o=jvP2OnBffNz2cKXj8tHuAUCN5HFuy2vB_g@mail.gmail.com>
On Wed, Jun 10, 2026 at 09:16:11PM +0530, Sumit Saxena wrote:
> The motivation for this change stems from performance issue we
> encountered due to false sharing of the 'nr_active_requests_shared_tags'
> counter
> on certain CPU architectures. I initially submitted a patch to move that
> counter to
> its own cache line to avoid conflicts with 'nr_requests' and other hot
> fields
> (see:
> https://patchwork.kernel.org/project/linux-scsi/patch/20260402074637.92417-3-sumit.saxena@broadcom.com/
> ).
>
> During the review, Bart shared his work, which eliminates the
> counter entirely by removing the fairness throttling. My testing confirmed
> that
> this approach resolved the performance issues and improved IOPS.
> This patch is part of a larger set, and I have reported the cumulative
> performance
> improvements in the cover letter.
So the problem is just the atomic operation accounting overhead? I
previously thought the device just really needed to consume all the tags
to hit performance.
^ permalink raw reply
* Re: [PATCH 09/27] drbd: Split drbd_nl_get_connections_dumpit()
From: Bart Van Assche @ 2026-06-10 16:48 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-block, Marco Elver, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <aij0fuVz_Bxi6NXi@infradead.org>
On 6/9/26 10:22 PM, Christoph Hellwig wrote:
> On Tue, Jun 09, 2026 at 03:04:56PM -0700, Bart Van Assche wrote:
>> +static int drbd_nl_put_dump_connections_result(
>> + struct sk_buff *skb, struct netlink_callback *cb,
>> + struct drbd_resource *resource, struct drbd_connection *connection,
>> + enum drbd_ret_code retcode)
>
> Weird indenation here. The usual style is either lining up after the
> opening brace or two tabs.
This formatting comes from git clang-format. Anyway, I will fix the
formatting.
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH] iomap: enforce DIO alignment check in iomap]
From: Carlos Maiolino @ 2026-06-10 16:58 UTC (permalink / raw)
To: Keith Busch; +Cc: brauner, linux-block
In-Reply-To: <aimErft1NoW-4Map@kbusch-mbp>
On Wed, Jun 10, 2026 at 09:37:17AM -0600, Keith Busch wrote:
> On Wed, Jun 10, 2026 at 05:27:42PM +0200, Carlos Maiolino wrote:
> > The DIO alignment check has been lifted from iomap layer to rely on the
> > block layer to enforce proper alignment when issuing direct IO
> > operations. This though, depending on the IO size and buffer address
> > passed to the IO operation may lead to user-visible behavior change.
> >
> > This has been caught initially by LTP test diotest4 running on
> > PPC architecture, where the test fails because a read() operation
> > with a supposedly misaligned buffer succeeds instead of an expected
> > -EINVAL.
>
> It's not supposed to matter where in the stack we determined this be an
> invalid request: it should still fail if it's misaligned. Could you
> clarify how this is succeeding?
Fair enough, can you point me to where the alignment is supposed to be
checked? I've been seeing kind of different behaviors with different
machines so I kind of am not sure where the alignment is supposed to be
validated within the block layer (I should probably have tagged this
patch as RFC as my understanding of the block layer is superficial).
A few runs when the test failed (because the read() call succeded) I
could see this for example:
openat(dirfd=AT_FDCWD, pathname="testdata-4.2063", flags=O_RDWR|O_DIRECT) = 3
_llseek(fd=3, offset=4096, result=[4096], whence=SEEK_SET) = 0
read(arg1=0x3, arg2=0x25960001, arg3=0x1000) = 0x1000
FWIW, on my laptop, the test fails because read() returns an -EIO (while
the test expects -EINVAL), I'm pointing the fingers to a faulty hardware
now but I didn't discard the possibility of an EIO being returned EINVAL
should have been returned instead.
I'm trying to find another machine to test and see whatever differences
appear...
Cheers
^ permalink raw reply
* [PATCH 1/1] block: partitions: bound sysv68 slice table count
From: Ren Wei @ 2026-06-10 16:58 UTC (permalink / raw)
To: linux-block
Cc: kees, axboe, objecting, akpm, phdm, yuantan098, zcliangcn, bird,
zzhan461, n05ec
In-Reply-To: <cover.1781036698.git.zzhan461@ucr.edu>
From: Zhao Zhang <zzhan461@ucr.edu>
sysv68_partition() reads a single sector for the slice table, but it
trusts ios_slccnt from disk and walks that many entries after skipping
the synthetic whole-disk slice. A crafted image can set ios_slccnt
larger than the 64 struct slice records that fit in one sector and
trigger an out-of-bounds read while scanning partitions.
Limit the slice count to the number of records that fit in the sector
returned by read_part_sector(), then drop the whole-disk entry only
when the bounded count is non-zero.
Fixes: 19d0e8ce856a ("partition: add support for sysv68 partitions")
Cc: stable@vger.kernel.org
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Zhengchuan Liang <zcliangcn@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Assisted-by: Codex:GPT-5.4
Signed-off-by: Zhao Zhang <zzhan461@ucr.edu>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
block/partitions/sysv68.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/block/partitions/sysv68.c b/block/partitions/sysv68.c
index 470e0f9de7be..5110ed83c541 100644
--- a/block/partitions/sysv68.c
+++ b/block/partitions/sysv68.c
@@ -48,7 +48,8 @@ struct slice {
int sysv68_partition(struct parsed_partitions *state)
{
- int i, slices;
+ sector_t slice_sector;
+ unsigned int i, slices;
int slot = 1;
Sector sect;
unsigned char *data;
@@ -65,14 +66,16 @@ int sysv68_partition(struct parsed_partitions *state)
return 0;
}
slices = be16_to_cpu(b->dk_ios.ios_slccnt);
- i = be32_to_cpu(b->dk_ios.ios_slcblk);
+ slice_sector = be32_to_cpu(b->dk_ios.ios_slcblk);
put_dev_sector(sect);
- data = read_part_sector(state, i, §);
+ data = read_part_sector(state, slice_sector, §);
if (!data)
return -1;
- slices -= 1; /* last slice is the whole disk */
+ slices = min_t(unsigned int, slices, SECTOR_SIZE / sizeof(*slice));
+ if (slices)
+ slices -= 1; /* last slice is the whole disk */
seq_buf_printf(&state->pp_buf, "sysV68: %s(s%u)", state->name, slices);
slice = (struct slice *)data;
for (i = 0; i < slices; i++, slice++) {
--
2.47.3
^ permalink raw reply related
* Re: [PATCH 00/27] Enable lock context analysis in drivers/block/
From: Bart Van Assche @ 2026-06-10 17:00 UTC (permalink / raw)
To: Christoph Hellwig, Marco Elver; +Cc: Jens Axboe, linux-block
In-Reply-To: <ailK6eqeWrwRLPkz@infradead.org>
On 6/10/26 4:30 AM, Christoph Hellwig wrote:
> Bart: maytbe for next version just enable it on a per-driver basis.
> Once all are covered we can switch to directory-wide.
I can do that. Since the merge window likely will open this Monday, I
probably should wait with reposting any patches from this series until
after the merge window has closed.
Thanks for having taken the time to review this patch series.
Bart.
^ permalink raw reply
* Re: [PATCH 11/27] drbd: Split drbd_req_state()
From: Bart Van Assche @ 2026-06-10 17:05 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-block, Marco Elver, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <aij1DtWM8N7O3q-E@infradead.org>
On 6/9/26 10:24 PM, Christoph Hellwig wrote:
>> +{
>> + enum drbd_state_rv rv;
>> +
>> + if (f & CS_SERIALIZE)
>> + mutex_lock(device->state_mutex);
>> + rv = __drbd_req_state(device, mask, val, f & ~CS_SERIALIZE);
>> if (f & CS_SERIALIZE)
>> mutex_unlock(device->state_mutex);
>
> Wouldn't something like:
>
> if (f & CS_SERIALIZE) {
> mutex_lock(device->state_mutex);
> rv = __drbd_req_state(device, mask, val, f & ~CS_SERIALIZE);
> mutex_unlock(device->state_mutex);
> } else {
> rv = __drbd_req_state(device, mask, val, f & ~CS_SERIALIZE);
> }
>
> be either to follow? Is there much of a point in the CS_SERIALIZE
> clearing here?
Do you perhaps want me to combine this patch and the next patch in this
series (12/27)?
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH 11/27] drbd: Split drbd_req_state()
From: Bart Van Assche @ 2026-06-10 17:07 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-block, Marco Elver, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <61eb3024-f9bb-465a-938c-c12db1218923@acm.org>
On 6/10/26 10:05 AM, Bart Van Assche wrote:
> Do you perhaps want me to combine this patch and the next patch in this
> series (12/27)?
Answering my own question: I just noticed that this has been requested
in the review comment on the next patch. I will combine these two
patches.
Bart.
^ permalink raw reply
* Re: [PATCH 18/27] loop: Add lock context annotations
From: Bart Van Assche @ 2026-06-10 17:13 UTC (permalink / raw)
To: Nilay Shroff, Jens Axboe
Cc: linux-block, Christoph Hellwig, Marco Elver, Nathan Chancellor
In-Reply-To: <c71180a3-c049-490f-a8ab-b3faa9714de1@linux.ibm.com>
On 6/10/26 2:21 AM, Nilay Shroff wrote:
> One thing I noticed while looking through the loop driver is that it
> also defines
> @loop_ctl_mutex, which protects @loop_index_idr. It might be worth
> annotating
> @loop_index_idr with `__guarded_by(&loop_ctl_mutex) as well so that
> Clang can
> validate accesses to the IDR against the corresponding locking
> requirements.
I'm considering to add the changes below as an additional patch:
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index ff7eff102c5a..30a2b2696368 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -90,8 +90,8 @@ struct loop_cmd {
#define LOOP_IDLE_WORKER_TIMEOUT (60 * HZ)
#define LOOP_DEFAULT_HW_Q_DEPTH 128
-static DEFINE_IDR(loop_index_idr);
static DEFINE_MUTEX(loop_ctl_mutex);
+static __guarded_by(&loop_ctl_mutex) DEFINE_IDR(loop_index_idr);
static DEFINE_MUTEX(loop_validate_mutex);
/**
@@ -2326,6 +2326,8 @@ static void __exit loop_exit(void)
struct loop_device *lo;
int id;
+ guard(mutex_init)(&loop_ctl_mutex);
+
unregister_blkdev(LOOP_MAJOR, "loop");
misc_deregister(&loop_misc);
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox