Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <8d8501e8-9e39-8b02-d248-48f778a95d96@samsung.com>
Date:   Thu, 16 Jun 2022 14:24:33 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
        Thunderbird/91.9.1
Subject: Re: [dm-devel] [PATCH v7 12/13] dm: call dm_zone_endio after the
 target endio callback for zoned devices
Content-Language: en-US
To:     Damien Le Moal <damien.lemoal@opensource.wdc.com>, <hch@lst.de>,
        <snitzer@redhat.com>, <axboe@kernel.dk>
CC:     <bvanassche@acm.org>, <pankydev8@gmail.com>,
        <gost.dev@samsung.com>, <jiangbo.365@bytedance.com>,
        <linux-nvme@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
        <linux-block@vger.kernel.org>, <dm-devel@redhat.com>,
        <jonathan.derrick@linux.dev>, <Johannes.Thumshirn@wdc.com>,
        <dsterba@suse.com>, <jaegeuk@kernel.org>
From:   Pankaj Raghav <p.raghav@samsung.com>
In-Reply-To: <f7b586a3-5370-f3b9-72dc-f9bea0b63f1f@opensource.wdc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
CMS-TYPE: 201P
References: <20220615101920.329421-1-p.raghav@samsung.com>
        <CGME20220615102007eucas1p1106f9520e2a86beb3792107dffd8071b@eucas1p1.samsung.com>
        <20220615101920.329421-13-p.raghav@samsung.com>
        <f7b586a3-5370-f3b9-72dc-f9bea0b63f1f@opensource.wdc.com>
Precedence: bulk

On 2022-06-15 13:01, Damien Le Moal wrote:
> On 6/15/22 19:19, Pankaj Raghav wrote:
>> dm_zone_endio() updates the bi_sector of orig bio for zoned devices that
>> uses either native append or append emulation and it is called before the
>> endio of the target. But target endio can still update the clone bio
>> after dm_zone_endio is called, thereby, the orig bio does not contain
>> the updated information anymore. Call dm_zone_endio for zoned devices
>> after calling the target's endio function
>>
>> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
>> ---
>> @Damien and @Hannes: I couldn't come up with a testcase that uses endio callback and
>> zone append or append emulation for zoned devices to test for
>> regression in this change. It would be great if you can suggest
>> something. This change is required for the npo2 target as we update the
>> clone bio sector in the endio callback function and the orig bio should
>> be updated only after the endio callback for zone appends.
> 
> Running zonefs tests on top of dm-crypt will exercise DM zone append
> emulation.
> 
Thanks. However, I am facing issues creating a dm-crypt target with a
zoned device. Steps:
- cryptsetup luksFormat <zns-device>

is throwing a bunch of IO errors with the following error message:
Device wipe error, offset 32768.
Cannot wipe header on device <zns-device>.

I can observe the same behavior in both v5.18 and next-20220615 with
cryptsetup 2.4.3.The same step is working correctly on a normal NVMe device.
Am I doing something wrong?
ZNS info: zsze 128M and zcap 128M with 50 zones
>>
>>  drivers/md/dm.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
>> index 3f17fe1de..3a74e1038 100644
>> --- a/drivers/md/dm.c
>> +++ b/drivers/md/dm.c
>> @@ -1025,10 +1025,6 @@ static void clone_endio(struct bio *bio)
>>  			disable_write_zeroes(md);
>>  	}
>>  
>> -	if (static_branch_unlikely(&zoned_enabled) &&
>> -	    unlikely(blk_queue_is_zoned(bdev_get_queue(bio->bi_bdev))))
>> -		dm_zone_endio(io, bio);
>> -
>>  	if (endio) {
>>  		int r = endio(ti, bio, &error);
>>  		switch (r) {
>> @@ -1057,6 +1053,10 @@ static void clone_endio(struct bio *bio)
>>  		}
>>  	}
>>  
>> +	if (static_branch_unlikely(&zoned_enabled) &&
>> +	    unlikely(blk_queue_is_zoned(bdev_get_queue(bio->bi_bdev))))
> 
> blk_queue_is_zoned(bdev_get_queue(bio->bi_bdev))) ->
> bdev_is_zoned(bio->bi_bdev)
> 
Ok. Even though I just moved the statements, I think this is trivial
enough to take it along.
>> +		dm_zone_endio(io, bio);
>> +
>>  	if (static_branch_unlikely(&swap_bios_enabled) &&
>>  	    unlikely(swap_bios_limit(ti, bio)))
>>  		up(&md->swap_bios_semaphore);
> 
>