Re: [PATCH v2] nvme: find numa distance only if controller has valid numa id

[Nilay Shroff <nilay@linux.ibm.com>]

On 4/16/24 08:06, Chaitanya Kulkarni wrote:
> On 4/15/24 07:22, Nilay Shroff wrote:
>> On system where native nvme multipath is configured and iopolicy
>> is set to numa but the nvme controller numa node id is undefined
>> or -1 (NUMA_NO_NODE) then avoid calculating node distance for
>> finding optimal io path. In such case we may access numa distance
>> table with invalid index and that may potentially refer to incorrect
>> memory. So this patch ensures that if the nvme controller numa node
>> id is -1 then instead of calculating node distance for finding optimal
>> io path, we set the numa node distance of such controller to default 10
>> (LOCAL_DISTANCE).
>>
>> Link: https://lore.kernel.org/all/20240413090614.678353-1-nilay@linux.ibm.com/
>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
>> ---
>> Changes from v1:
>> 	Formatting cleanups (Christoph)
>>
>>   drivers/nvme/host/multipath.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
>> index 5397fb428b24..08e598c87b08 100644
>> --- a/drivers/nvme/host/multipath.c
>> +++ b/drivers/nvme/host/multipath.c
>> @@ -247,7 +247,8 @@ static struct nvme_ns *__nvme_find_path(struct nvme_ns_head *head, int node)
>>   		if (nvme_path_is_disabled(ns))
>>   			continue;
>>   
>> -		if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_NUMA)
>> +		if (ns->ctrl->numa_node != NUMA_NO_NODE &&
>> +				READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_NUMA)
> 
> unless something is wrong when I apply this patch, above line should be
> kept under 80 char ?
> 
> something like following totally untested :-
> 
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 5397fb428b24..d16e976ae1a4 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -247,7 +247,8 @@ static struct nvme_ns *__nvme_find_path(struct 
> nvme_ns_head *head, int node)
>                  if (nvme_path_is_disabled(ns))
>                          continue;
> 
> -               if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_NUMA)
> +               if (ns->ctrl->numa_node != NUMA_NO_NODE &&
> +                   READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_NUMA)
>                          distance = node_distance(node, 
> ns->ctrl->numa_node);
>                  else
>                          distance = LOCAL_DISTANCE;
> 
> -ck
> 
> 
I don't know but checkpatch.pl didn't complain about my patch overshooting 
80 chars width. Anyways, I will reformat the patch and resend.

Thanks,
--Nilay