Re: [PATCH v2 06/12] null_blk: Enable lock context analysis

On 8/3/26 6:35 AM, Nilay Shroff wrote:
> On 7/31/26 1:28 AM, Bart Van Assche wrote:
>> +DEFINE_CLASS(null_zone, struct nullb_dev_and_zone, ({
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 if (!_T.dev->memory_backed)
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0 spin_unlock_irq(&_T.zone->spinlock);
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 else
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0 mutex_unlock(&_T.zone->mutex);
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }), ({
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 if (!dev->memory_backed)
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0 spin_lock_irq(&zone->spinlock);
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 else
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0 mutex_lock(&zone->mutex);
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (struct nullb_dev_and_zone){ dev, zone };
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }),
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct nullb_device =
*dev, struct nullb_zone *zone)
>> +
>> +DEFINE_CLASS_IS_UNCONDITIONAL(null_zone)
>=20
> What do we gain by introducing a guard class here?

What we gain is that __context_unsafe() annotations are avoided
entirely. __context_unsafe() is a big hammer that shouldn't be used
unless absolutely necessary. Additionally, introducing this class makes
it possible to convert multiple goto statements into return statements.
I think that's a significant improvement.

> Since both the constructor and destructor are marked
> __context_unsafe, the lock context analysis still
> won't reason about the conditional locking.

Such reasoning is not needed when using guard() or scoped_guard() since
these macro's make it impossible to leave a scope without releasing the
acquired synchronization object.

> I'm not opposed to using a guard class here, but it does make the
> code a bit more complex to follow.

Hmm ... I think this means that you are not familiar yet with
DEFINE_CLASS(). The code that uses the null_zone class becomes simpler.

Thanks,

Bart.