[PATCH 2/3] nvme-core: move gencounter check into nvme_cid()

[Sagi Grimberg]

>> External email: Use caution opening links or attachments
>>
>>
>> On Fri, Dec 10, 2021 at 03:21:15AM -0800, Chaitanya Kulkarni wrote:
>>> -     if (!(ctrl->quirks & NVME_QUIRK_SKIP_CID_GEN))
>>> -             nvme_req(req)->genctr++;
>>>         cmd->common.command_id = nvme_cid(req);
>>>         trace_nvme_setup_cmd(req, cmd);
>>>         return ret;
>>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>>> index 98d7627cfdce..2be0191e1a1f 100644
>>> --- a/drivers/nvme/host/nvme.h
>>> +++ b/drivers/nvme/host/nvme.h
>>> @@ -511,6 +511,8 @@ struct nvme_ctrl_ops {
>>>
>>>    static inline u16 nvme_cid(struct request *rq)
>>>    {
>>> +     if (!(nvme_req(rq)->ctrl->quirks & NVME_QUIRK_SKIP_CID_GEN))
>>> +             nvme_req(rq)->genctr++;
>>>         return nvme_cid_install_genctr(nvme_req(rq)->genctr) | rq->tag;
>>>    }
>>
>> No, this will incrememnt the counter every time someone queries the
>> command id for this request, which happens in multiple places. The
>> counter can't be modified for the lifetime of the request.
>>
> 
> Yes, the right thing to do here is to create a new stub for
> CONFIG_NVME_DEBUG_USE_GENCTR and !CONFIG_NVME_DEBUG_USE_GENCTR
> case and move the quick check in there, will send out V2.

Don't understand the global config option at all, distributions
will probably enable it anyways (as the default needs to be set
to Y anyways). However, this set was obviously not tested with nvme-tcp
because it breaks it completely. And it is also in general a bad
practice IMO to increment the genctr on every install. It needs to
be in a different helper.

Please make sure to test nvme-tcp on your v2.