Received-SPF: pass (google.com: domain of linux-kernel+bounces-44671-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Subject: Re: [PATCH RFC v4 13/14] dm: wait for IO completion before removing
 dm device
To: Mikulas Patocka <mpatocka@redhat.com>, 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, 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,
 "yukuai (C)" <yukuai3@huawei.com>
References: <20240130021843.3608859-1-yukuai1@huaweicloud.com>
 <20240130021843.3608859-14-yukuai1@huaweicloud.com>
 <fa4cd2f8-d0e8-5b6d-2ac6-1c5f1710a5ee@redhat.com>
From: Yu Kuai <yukuai1@huaweicloud.com>
Message-ID: <bdeac016-f57d-f917-d605-342fe9856ccf@huaweicloud.com>
Date: Tue, 30 Jan 2024 21:05:25 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <fa4cd2f8-d0e8-5b6d-2ac6-1c5f1710a5ee@redhat.com>
Content-Type: text/plain; charset=gbk; format=flowed
Content-Transfer-Encoding: 8bit

Hi,

?? 2024/01/30 19:46, Mikulas Patocka д??:
> 
> 
> 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.

I add this patch because while testing, there is a problem that is
hard to reporduce, as I mentioned in the other thread. I'll add BUG_ON()
and try if I can still reporduce this problem without triggering it.

Thanks,
Kuai

[12504.959682] BUG bio-296 (Not tainted): Object already free
[12504.960239] 
-----------------------------------------------------------------------------
[12504.960239]
[12504.961209] Allocated in mempool_alloc+0xe8/0x270 age=30 cpu=1 pid=203288
[12504.961905]  kmem_cache_alloc+0x36a/0x3b0
[12504.962324]  mempool_alloc+0xe8/0x270
[12504.962712]  bio_alloc_bioset+0x3b5/0x920
[12504.963129]  bio_alloc_clone+0x3e/0x160
[12504.963533]  alloc_io+0x3d/0x1f0
[12504.963876]  dm_submit_bio+0x12f/0xa30
[12504.964267]  __submit_bio+0x9c/0xe0
[12504.964639]  submit_bio_noacct_nocheck+0x25a/0x570
[12504.965136]  submit_bio_wait+0xc2/0x160
[12504.965535]  blkdev_issue_zeroout+0x19b/0x2e0
[12504.965991]  ext4_init_inode_table+0x246/0x560
[12504.966462]  ext4_lazyinit_thread+0x750/0xbe0
[12504.966922]  kthread+0x1b4/0x1f0
> 
> 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
>>
> 
> .
>