[Devel] [PATCH vz10 10/24] dm-ploop: annotate nested md_lock in ploop_check_delta_before_flip()

[Pavel Tikhomirov]

On 7/6/26 12:59, Konstantin Khorenko wrote:
> ploop_check_delta_before_flip() locks the live device's md->md_lock and
> then a second md_lock from the delta being read into a separate md_root
> tree (d_md). They are distinct objects of the same lock class, so lockdep
> reports a false "possible recursive locking" deadlock. The two are only
> ever taken here and always in this (md, then d_md) order, so annotate the
> inner acquire with SINGLE_DEPTH_NESTING.
> 
> Fixes: a9cd5e6dc646 ("dm-ploop: Add ploop target driver")
> Feature: dm-ploop: ploop target driver
> https://virtuozzo.atlassian.net/browse/VSTOR-137234
> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> ---
>  drivers/md/dm-ploop-cmd.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/md/dm-ploop-cmd.c b/drivers/md/dm-ploop-cmd.c
> index 5fa78e6cff9a..73c5613c82f5 100644
> --- a/drivers/md/dm-ploop-cmd.c
> +++ b/drivers/md/dm-ploop-cmd.c
> @@ -1101,7 +1101,13 @@ static int ploop_check_delta_before_flip(struct ploop *ploop, struct file *file)
>  		d_bat_entries = d_md->kmpage;
>  
>  		spin_lock_irq(&md->md_lock); /* read */
> -		spin_lock(&d_md->md_lock);
> +		/*
> +		 * md (live device tree) and d_md (delta being read into a
> +		 * separate md_root tree) are distinct md_page objects of the
> +		 * same lock class; they are only ever locked here in this
> +		 * order, so annotate the nesting for lockdep.
> +		 */
> +		spin_lock_nested(&d_md->md_lock, SINGLE_DEPTH_NESTING);

This does not sound good, nested lock semantic is for when we lock one lock twice
for some reason. But here lock is different. So it's more of a workaround than
a clear solution here.

We should either:

a) Have d_md->md_lock moved to separate lockdep class (e.g. via
lockdep_set_class()).

or

b) Even better, just drop d_md->md_lock completely. AI suggests that this lock
is not needed at all as d_md is a local object, there is no point to sync it with
anyone else.

Also note that there is other call site where we have the same lockdep problem, see
notify_delta_merged().

>  		for (; i <= end; i++) {
>  			if (ploop_md_page_cluster_is_in_top_delta(ploop, md, i) &&
>  			    d_bat_entries[i] != BAT_ENTRY_NONE) {

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.