[PATCH 12/16] nvme-auth: convert dhchap_auth_list to an array

[Sagi Grimberg]

On 11/9/22 19:40, Hannes Reinecke wrote:
> On 11/9/22 04:44, Sagi Grimberg wrote:
>> We know exactly how many dhchap contexts we will need, there is no need
>> to hold a list that we need to protect with a mutex. Convert to
>> a dynamically allocated array. And dhchap_context access state is
>> maintained by the chap itself.
>>
>> Make dhchap_auth_mutex protect only the ctrl host_key and ctrl_key
>> in a fine-graned lock such that there is no long lasting acuisition
>> of the lock.
>>
>> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
>> ---
>>   drivers/nvme/host/auth.c | 113 +++++++++++++++++++++------------------
>>   drivers/nvme/host/core.c |   4 ++
>>   drivers/nvme/host/nvme.h |   2 +-
>>   3 files changed, 65 insertions(+), 54 deletions(-)
>>
>> diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
>> index 23d486284da9..b78e0df54f6c 100644
>> --- a/drivers/nvme/host/auth.c
>> +++ b/drivers/nvme/host/auth.c
>> @@ -46,6 +46,14 @@ struct nvme_dhchap_queue_context {
>>       (qid == 0) ? 0 : BLK_MQ_REQ_NOWAIT | BLK_MQ_REQ_RESERVED
>>   #define nvme_auth_queue_from_qid(ctrl, qid) \
>>       (qid == 0) ? (ctrl)->fabrics_q : (ctrl)->connect_q
>> +#define nvme_auth_chap_from_qid(ctrl, qid) \
>> +    &((struct nvme_dhchap_queue_context*)(ctrl)->dhchap_ctxs)[qid]
>> +#define ctrl_max_dhchaps(ctrl) \
>> +    (ctrl)->opts->nr_io_queues + (ctrl)->opts->nr_write_queues + \
>> +            (ctrl)->opts->nr_poll_queues + 1
>> +#define nvme_foreach_dhchap(i, chap, ctrl) \
>> +    for (i = 0, chap = (ctrl)->dhchap_ctxs; \
>> +         i < ctrl_max_dhchaps(ctrl) && chap != NULL; i++, chap++)
>>   static int nvme_auth_submit(struct nvme_ctrl *ctrl, int qid,
>>                   void *data, size_t data_len, bool auth_send)
> 
> Please use an xarray (cf my patch doing the same).
> That leads to far better readable code, and avoid having to do weird casts.
> 
> Also, the claim 'we know exactly how many queues' isn't quite true, as 
> we only know the _maximum_ number of queues. The controller can (and 
> will) decrease this number based on the target configuration.

I don't want a dynamically allocated array, this pattern of first time
allocates a context and the rest of the time reuses is less readable and
causes interlocking.