Re: [PATCH v3] block: assign caller-specific lockdep class to disk->open_mutex

[Andreas Hindborg]

Andreas Hindborg <a.hindborg@xxxxxxxxxx> writes:

> "Tetsuo Handa" <penguin-kernel@xxxxxxxxxxxxxxxxxxx> writes:
>
>> On 2026/06/19 18:49, Andreas Hindborg wrote:
>>>> My understanding is that we don't have infrastructure for lock class keys
>>>> that can be applied to
>>>>
>>>>   +struct gendisk_lkclass {
>>>>   +	struct lock_class_key bio_lkclass;
>>>>   +	struct lock_class_key open_mutex_lkclass;
>>>>   +};
>>>>
>>>>   -	static struct lock_class_key __key;
>>>>   +	static struct gendisk_lkclass __key;
>>>>
>>>> change. Alternative approach is welcomed if you have one.
>>>
>>> Sorry, I did not pay enough attention. I would suggest this approach:
>>
>> I'm OK with your module-specific lockdep class approach
>>
>>> @@ -164,14 +127,21 @@ pub fn build<T: Operations>(
>>>              lim.features = bindings::BLK_FEAT_ROTATIONAL;
>>>          }
>>>
>>> -        // SAFETY: `tagset.raw_tag_set()` points to a valid and initialized tag set
>>> +        let keys = KBox::pin_init(
>>> +            Opaque::ffi_init(|ptr: *mut bindings::gendisk_lkclass| {
>>> +                // SAFETY: `ptr` is valid for writes
>>> +                unsafe { bindings::lockdep_register_key(&raw mut (*ptr).bio_lkclass) };
>>> +                // SAFETY: `ptr` is valid for writes
>>> +                unsafe { bindings::lockdep_register_key(&raw mut (*ptr).open_mutex_lkclass) };
>>> +            }),
>>> +            GFP_KERNEL,
>>> +        )?;
>>> +
>>> +        // SAFETY:
>>> +        // - `tagset.raw_tag_set()` points to a valid and initialized tag set.
>>> +        // - We keep `keys` alive for the lifetime of the returned gendisk.
>>>          let gendisk = from_err_ptr(unsafe {
>>> -            bindings::__blk_mq_alloc_disk(
>>> -                tagset.raw_tag_set(),
>>> -                &mut lim,
>>> -                data,
>>> -                lkclass.as_ptr(),
>>> -            )
>>> +            bindings::__blk_mq_alloc_disk(tagset.raw_tag_set(), &mut lim, data, keys.get())
>>>          })?;
>>>
>>>          const TABLE: bindings::block_device_operations = bindings::block_device_operations {
>>
>> if we can assume that there is (up to) only one
>>
>>   let mut disk = gen_disk::GenDiskBuilder::new().capacity_sectors(4096).build(fmt!("myblk"), tagset, ())?;
>>
>> call for each rust module.
>
> With the approach I suggest here, we get a new set of keys for each
> invocation of `GenDiskBuilder::build`. No statics involved, keys are
> allocated dynamically.
>
> The current code makes one key for to be shared by _all_ callers, which
> is probably a bad idea.
>
>>
>> By the way, are you aware that rust-enabled linux-next build is recently failing
>> ( https://syzkaller.appspot.com/bug?extid=1f14a35d0c73d31555e4 ) ?
>
> I was not aware, thanks for pointing that out. I actually did build
> linux-next for both June 16 and June 18 and I did not see any issue. It
> looks like a tooling problem to me. I'll see if I can figure out what it
> is.

I think this is [1]. We saw it for lkp when they used bindgen 0.72.0
with clang 22 I think. Probably syzkaller needs to bump bindgen version
to 0.72.2.

Cc: Miguel Ojeda <ojeda@xxxxxxxxxx>

Not sure who to Cc on syzkaller side.

Best regards,
Andreas Hindborg


[1] https://github.com/rust-lang/rust-bindgen/issues/3264