Re: [PATCH v2 20/26] null_blk: Enable lock context analysis

[Bart Van Assche <bvanassche@acm.org>]

(+Marco)

On 3/30/26 3:23 AM, Nilay Shroff wrote:
> On 3/26/26 3:15 AM, Bart Van Assche wrote:
>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/ 
>> main.c
>> index f8c0fd57e041..677ac829ef80 100644
>> --- a/drivers/block/null_blk/main.c
>> +++ b/drivers/block/null_blk/main.c
>> @@ -1004,8 +1004,7 @@ static struct nullb_page 
>> *null_lookup_page(struct nullb *nullb,
>>   static struct nullb_page *null_insert_page(struct nullb *nullb,
>>                          sector_t sector, bool ignore_cache)
>> -    __releases(&nullb->lock)
>> -    __acquires(&nullb->lock)
>> +    __must_hold(&nullb->lock)
> 
> This function temporarily drops the &nullb->lock and requires it.
> So why do we need to replace __releases/__acquires with __must_hold?
> This is not clear.

__must_hold() has the same meaning as __releases() + __acquires(). The
only difference between these two annotations is that __must_hold() is
more compact. I can leave out this change if this change is considered
confusing.

>> diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/ 
>> zoned.c
>> index 384bdce6a9b7..a7f94e76034f 100644
>> --- a/drivers/block/null_blk/zoned.c
>> +++ b/drivers/block/null_blk/zoned.c
>> @@ -32,6 +32,7 @@ static inline void null_init_zone_lock(struct 
>> nullb_device *dev,
>>   static inline void null_lock_zone(struct nullb_device *dev,
>>                     struct nullb_zone *zone)
>> +    __no_context_analysis /* conditional locking */
> 
> I see we already have __context_unsafe(comment) macro defined
> which may be used instead for inline comment.

context_unsafe() suppresses warnings triggered by __guarded_by() and
__guarded_by_ptr() but does not suppress analysis of functions in the
function body annotated with __releases() and __acquired(). If I remove
the __no_context_analysis annotation and surround the function body with
context_unsafe(), the following appears:

drivers/block/null_blk/zoned.c:37:2: error: spinlock 'zone->.spinlock' 
is not held on every path through here [-Werror,-Wthread-safety-analysis]
    37 |         context_unsafe(
       |         ^
./include/linux/compiler-context-analysis.h:201:30: note: expanded from 
macro 'context_unsafe'
   201 | #define context_unsafe(...)             \
       |                                         ^
drivers/block/null_blk/zoned.c:39:3: note: spinlock acquired here
    39 |                 spin_lock_irq(&zone->spinlock);
       |                 ^

drivers/block/null_blk/zoned.c:37:2: error: mutex 'zone->.mutex' is not 
held on every path through here [-Werror,-Wthread-safety-analysis]
    37 |         context_unsafe(
       |         ^
./include/linux/compiler-context-analysis.h:201:30: note: expanded from 
macro 'context_unsafe'
   201 | #define context_unsafe(...)             \
       |                                         ^
drivers/block/null_blk/zoned.c:41:3: note: mutex acquired here
    41 |                 mutex_lock(&zone->mutex);
       |                 ^
2 errors generated.

Thanks,

Bart.