Re: [PATCH v2 16/26] drbd: Make the lock context annotations compatible with Clang

[Christoph Hellwig <hch@lst.de>]

On Wed, Mar 25, 2026 at 02:44:57PM -0700, Bart Van Assche wrote:
> Clang performs more strict checking of lock context annotations than
> sparse. This patch makes the DRBD lock context annotations compatible
> with Clang and enables lock context analysis in the Makefile.
> 
> Reviewed-by: Christoph Böhmwalder <christoph.boehmwalder@linbit.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/block/drbd/Makefile        |  3 +
>  drivers/block/drbd/drbd_bitmap.c   | 26 ++++++---
>  drivers/block/drbd/drbd_int.h      | 88 ++++++++++++++++++------------
>  drivers/block/drbd/drbd_main.c     | 40 ++++++++++----
>  drivers/block/drbd/drbd_nl.c       |  5 +-
>  drivers/block/drbd/drbd_receiver.c | 31 ++++++++---
>  drivers/block/drbd/drbd_req.c      |  3 +
>  drivers/block/drbd/drbd_state.c    |  2 +
>  drivers/block/drbd/drbd_state.h    |  4 --
>  drivers/block/drbd/drbd_worker.c   |  6 +-
>  10 files changed, 138 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/block/drbd/Makefile b/drivers/block/drbd/Makefile
> index 67a8b352a1d5..8eaa83a7592b 100644
> --- a/drivers/block/drbd/Makefile
> +++ b/drivers/block/drbd/Makefile
> @@ -1,4 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> +
> +CONTEXT_ANALYSIS := y
> +
>  drbd-y := drbd_buildtag.o drbd_bitmap.o drbd_proc.o
>  drbd-y += drbd_worker.o drbd_receiver.o drbd_req.o drbd_actlog.o
>  drbd-y += drbd_main.o drbd_strings.o drbd_nl.o
> diff --git a/drivers/block/drbd/drbd_bitmap.c b/drivers/block/drbd/drbd_bitmap.c
> index 65ea6ec66bfd..3c521f0dc9ad 100644
> --- a/drivers/block/drbd/drbd_bitmap.c
> +++ b/drivers/block/drbd/drbd_bitmap.c
> @@ -122,12 +122,16 @@ static void __bm_print_lock_info(struct drbd_device *device, const char *func)
>  }
>  
>  void drbd_bm_lock(struct drbd_device *device, char *why, enum bm_flag flags)
> +	__acquires(&device->bitmap->bm_change)
>  {
>  	struct drbd_bitmap *b = device->bitmap;
>  	int trylock_failed;
>  
>  	if (!b) {
>  		drbd_err(device, "FIXME no bitmap in drbd_bm_lock!?\n");
> +		/* Fake __acquire() to keep the compiler happy. */
> +		__acquire(&b->bm_change);
> +		__acquire(drbd_bitmap_lock);
>  		return;

This looks like an impossible to hit case with cargo cult code.  I'd
suggest to remove it instead.

Similarly please kill the trylock_faiuled code, which has weird
debugging that is strictly inferior to what lockdep already provides.

>  void drbd_bm_unlock(struct drbd_device *device)
> +	__releases(&device->bitmap->bm_change)
>  {
>  	struct drbd_bitmap *b = device->bitmap;
>  	if (!b) {
>  		drbd_err(device, "FIXME no bitmap in drbd_bm_unlock!?\n");
> +		/* Fake __release() to keep the compiler happy. */
> +		__release(&b->bm_change);
> +		__release(drbd_bitmap_lock);
>  		return;
>  	}

Same comment as above.

> +extern void drbd_uuid_set(struct drbd_device *device, int idx, u64 val);
> +extern void _drbd_uuid_set(struct drbd_device *device, int idx, u64 val);
> +extern void drbd_uuid_new_current(struct drbd_device *device);
> +extern void drbd_uuid_set_bm(struct drbd_device *device, u64 val);
> +extern void drbd_uuid_move_history(struct drbd_device *device);
> +extern void __drbd_uuid_set(struct drbd_device *device, int idx, u64 val);
> +extern void drbd_md_set_flag(struct drbd_device *device, int flags);
> +extern void drbd_md_clear_flag(struct drbd_device *device, int flags);

All these externs should go away.

In general this still mixes up way too many things.  Address one
lock/issue at a time, and write proper commit logs explain why a
given solution is taken.