Received-SPF: pass (google.com: domain of linux-kernel+bounces-44546-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 30 Jan 2024 12:46:08 +0100 (CET)
From: Mikulas Patocka <mpatocka@redhat.com>
To: Yu Kuai <yukuai1@huaweicloud.com>
cc: heinzm@redhat.com, xni@redhat.com, agk@redhat.com, snitzer@kernel.org, 
    dm-devel@lists.linux.dev, song@kernel.org, yukuai3@huawei.com, 
    jbrassow@f14.redhat.com, neilb@suse.de, shli@fb.com, akpm@osdl.org, 
    linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org, 
    yi.zhang@huawei.com, yangerkun@huawei.com
Subject: Re: [PATCH RFC v4 13/14] dm: wait for IO completion before removing
 dm device
In-Reply-To: <20240130021843.3608859-14-yukuai1@huaweicloud.com>
Message-ID: <fa4cd2f8-d0e8-5b6d-2ac6-1c5f1710a5ee@redhat.com>
References: <20240130021843.3608859-1-yukuai1@huaweicloud.com> <20240130021843.3608859-14-yukuai1@huaweicloud.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII


On Tue, 30 Jan 2024, Yu Kuai wrote:

> From: Yu Kuai <yukuai3@huawei.com>
> 
> __dm_destroy() guarantee that device openers is zero, and then
> only call 'presuspend' and 'postsuspend' for the target. For
> request-based dm, 'md->holders' will be grabbed for each rq and
> __dm_destroy() will wait for 'md->holders' to be zero. However, for
> bio-based device, __dm_destroy() doesn't wait for all bios to be done.
> 
> Fix this problem by calling dm_wait_for_completion() to wail for all
> inflight IO to be done, like what dm_suspend() does.

If the number of openers is zero, it is guaranteed that there are no bios 
in flight. Therefore, we don't have to wait for them.

If there are bios in flight, it is a bug in the code that issues the bios. 
You can put WARN_ON(dm_in_flight_bios(md)) there.

Mikulas

> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>  drivers/md/dm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 8dcabf84d866..2c0eae67d0f1 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -58,6 +58,7 @@ static DEFINE_IDR(_minor_idr);
>  static DEFINE_SPINLOCK(_minor_lock);
>  
>  static void do_deferred_remove(struct work_struct *w);
> +static int dm_wait_for_completion(struct mapped_device *md, unsigned int task_state);
>  
>  static DECLARE_WORK(deferred_remove_work, do_deferred_remove);
>  
> @@ -2495,6 +2496,8 @@ static void __dm_destroy(struct mapped_device *md, bool wait)
>  	if (!dm_suspended_md(md)) {
>  		dm_table_presuspend_targets(map);
>  		set_bit(DMF_SUSPENDED, &md->flags);
> +		if (wait)
> +			dm_wait_for_completion(md, TASK_UNINTERRUPTIBLE);
>  		set_bit(DMF_POST_SUSPENDING, &md->flags);
>  		dm_table_postsuspend_targets(map);
>  	}
> -- 
> 2.39.2
>