IronPort-SDR: WalicxANJ1g838oWa7ciEQQ+C7kLw7l9OU1viC7o5EqQPc1xPPOgnrB3NZsXqRHmXZGtBf4tWN
 ULDWa597osBw8H2aZcXCFXi4mSYta5o05ljogj7JY/Xj0Si50vqXMj9mZwYKF85Ol54n6KQSAq
 wIidEmExJhDUG0FdoLSZGy94lW6ykWeEbCLHfyDBT+e+C5GQ3r2tRIgMxREgW3vhpRxpHphN8S
 gfgbWix/7sq7Acigs2ysTpQzDYxqP2Eu2H8syeyML0A0R9kef9bLL0qeyNk+UgKCvotDKvNgN2
 9NsUzNBfVGvBpinieToY9kpk
IronPort-SDR: MMlmuTK1T+QJUOvPPqvCLA/YYOoxeha2DbB03vzArUZe6FaGbO6mJJAWeLuw5xNhueV5RvhmFQ
 TQUUBeGBnCPz/qqplZp7MRS/7PaCxKsYmuVAvAINpVri7vGJNqvgbd0mL8BxKO8Cskad6qVDDP
 DHOHxJ4bPBoj6M5mNtF7JbEAzMUU53pEJG/lA4vsIrU603zzXWRBLXkHMMPyUl1+d+1qeBjj6k
 5SZDuEk5sGnkUHYsjZsoKh3watOupwor4b0pJzP4tYODLbCZSIlq+cIQlZ7ivLTNClj1ZLP8mK
 uS0=
WDCIronportException: Internal
Message-ID: <ef1c39ab-7b56-6a37-0f4f-1ca111d5b48b@opensource.wdc.com>
Date:   Sat, 27 Nov 2021 10:08:28 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.3.2
Subject: Re: [PATCH v2] nvme: report write pointer for a full zone as zone
 start + zone len
Content-Language: en-US
To:     Niklas Cassel <Niklas.Cassel@wdc.com>,
        Keith Busch <kbusch@kernel.org>, Jens Axboe <axboe@fb.com>,
        Christoph Hellwig <hch@lst.de>,
        Sagi Grimberg <sagi@grimberg.me>
Cc:     "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <20211126104157.227400-1-Niklas.Cassel@wdc.com>
From:   Damien Le Moal <damien.lemoal@opensource.wdc.com>
Organization: Western Digital
In-Reply-To: <20211126104157.227400-1-Niklas.Cassel@wdc.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2021/11/26 19:42, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> The write pointer in NVMe ZNS is invalid for a zone in zone state full.
> The same also holds true for ZAC/ZBC.
> 
> The current behavior for NVMe is to simply propagate the wp reported by
> the drive, even for full zones. Since the wp is invalid for a full zone,
> the wp reported by the drive may be any value.
> 
> The way that the sd_zbc driver handles a full zone is to always report
> the wp as zone start + zone len, regardless of what the drive reported.
> null_blk also follows this convention.
> 
> Do the same for NVMe, so that a BLKREPORTZONE ioctl reports the write
> pointer for a full zone in a consistent way, regardless of the interface
> of the underlying zoned block device.
> 
> blkzone report before patch:
> start: 0x000040000, len 0x040000, cap 0x03e000, wptr 0xfffffffffffbfff8
> reset:0 non-seq:0, zcond:14(fu) [type: 2(SEQ_WRITE_REQUIRED)]
> 
> blkzone report after patch:
> start: 0x000040000, len 0x040000, cap 0x03e000, wptr 0x040000 reset:0
> non-seq:0, zcond:14(fu) [type: 2(SEQ_WRITE_REQUIRED)]
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
> Changes since v1:
> - Minor commit message rewording.
> - Use if/else instead of setting wp unconditionally and then
>   conditionally updating it.
> 
>  drivers/nvme/host/zns.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
> index bfc259e0d7b8..9f81beb4df4e 100644
> --- a/drivers/nvme/host/zns.c
> +++ b/drivers/nvme/host/zns.c
> @@ -166,7 +166,10 @@ static int nvme_zone_parse_entry(struct nvme_ns *ns,
>  	zone.len = ns->zsze;
>  	zone.capacity = nvme_lba_to_sect(ns, le64_to_cpu(entry->zcap));
>  	zone.start = nvme_lba_to_sect(ns, le64_to_cpu(entry->zslba));
> -	zone.wp = nvme_lba_to_sect(ns, le64_to_cpu(entry->wp));
> +	if (zone.cond == BLK_ZONE_COND_FULL)
> +		zone.wp = zone.start + zone.len;
> +	else
> +		zone.wp = nvme_lba_to_sect(ns, le64_to_cpu(entry->wp));
>  
>  	return cb(&zone, idx, data);
>  }
> 

Looks good.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

Note: read-only zones also have an undefined wp. So I wonder if we should not
set the wp similarly to full zones, to match the fact that we cannot write to
these zones. Same for offline zones, but these are tricky since they cannot be
read either, meaning that wp should be set to the zone start for that case...


-- 
Damien Le Moal
Western Digital Research