Re: [PATCH] nvme: check PI size if metadata size or below

[Tokunori Ikegami <ikegami.t@gmail.com>]

The existing function nvme_configure_metadata() code checks as same as 
below but is this checking case only okay?

static void nvme_configure_metadata(struct nvme_ctrl *ctrl,
         struct nvme_ns_head *head, struct nvme_id_ns *id,
         struct nvme_id_ns_nvm *nvm, struct nvme_ns_info *info)
{

...

     if (head->pi_size && head->ms >= head->pi_size)
         head->pi_type = id->dps & NVME_NS_DPS_PI_MASK;

On 2024/10/23 1:54, Keith Busch wrote:
> On Wed, Oct 23, 2024 at 01:34:27AM +0900, Tokunori Ikegami wrote:
>> -	if ((io.control & NVME_RW_PRINFO_PRACT) &&
>> -	    (ns->head->ms == ns->head->pi_size)) {
>> +	if (io.control & NVME_RW_PRINFO_PRACT &&
>> +	    ns->head->ms >= ns->head->pi_size) {
>>   		/*
>>   		 * Protection information is stripped/inserted by the
>>   		 * controller.
>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>> index 093cb423f536..fbcb2243ba84 100644
>> --- a/drivers/nvme/host/nvme.h
>> +++ b/drivers/nvme/host/nvme.h
>> @@ -542,7 +542,7 @@ struct nvme_ns {
>>   /* NVMe ns supports metadata actions by the controller (generate/strip) */
>>   static inline bool nvme_ns_has_pi(struct nvme_ns_head *head)
>>   {
>> -	return head->pi_type && head->ms == head->pi_size;
>> +	return head->pi_type && head->ms >= head->pi_size;
>>   }
> It would have been nice if PRACT worked this way, but spec says it
> doesn't. The control bit is a no-op if Metadata Size does not equal the
> PI size, which is what these checks are looking for, so we can't take
> this.