Re: [NDCTL PATCH] ndctl: cxl: Remove dependency for attributes derived from IDENTIFY command

[Dave Jiang <dave.jiang@intel.com>]

On 2/14/24 1:00 AM, Wonjae Lee wrote:
> On Fri, Feb 09, 2024 at 02:39:17PM -0700, Dave Jiang wrote:
>> A memdev may optionally not host a mailbox and therefore not able to execute
>> the IDENTIFY command. Currently the kernel emits empty strings for some of
>> the attributes instead of making them invisible in order to keep backward
>> compatibility for CXL CLI. Remove dependency of CXL CLI on the existance of
>> these attributes and only expose them if they exist. Without the dependency
>> the kernel will be able to make the non-existant attributes invisible.
>>
>> Link: https://lore.kernel.org/all/20230606121534.00003870@Huawei.com/
>> Suggested-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>  cxl/lib/libcxl.c | 48 ++++++++++++++++++++++++++----------------------
>>  cxl/memdev.c     | 15 ++++++++++-----
>>  2 files changed, 36 insertions(+), 27 deletions(-)
>>
> 
> [snip]
> 
>> diff --git a/cxl/memdev.c b/cxl/memdev.c
>> index 81f07991da06..feab7ea76e78 100644
>> --- a/cxl/memdev.c
>> +++ b/cxl/memdev.c
>> @@ -473,10 +473,13 @@ static int action_zero(struct cxl_memdev *memdev, struct action_context *actx)
>>   size_t size;
>>   int rc;
>>
>> - if (param.len)
>> + if (param.len) {
>>       size = param.len;
>> - else
>> + } else {
>>       size = cxl_memdev_get_label_size(memdev);
>> +     if (size == ULLONG_MAX)
>> +         return -EINVAL;
>> + }
> 
> Hello,
> 
> Doesn't action_write() also need to check the return value of
> cxl_memdev_get_label_size() like below?

Yes. I missed that one. Thank you!
> 
> diff --git a/cxl/memdev.c b/cxl/memdev.c
> index feab7ea..de46edc 100644
> --- a/cxl/memdev.c
> +++ b/cxl/memdev.c
> @@ -511,6 +511,8 @@ static int action_write(struct cxl_memdev *memdev, struct action_context *actx)
> 
>         if (!size) {
>                 size_t label_size = cxl_memdev_get_label_size(memdev);
> +               if (label_size == ULLONG_MAX)
> +                       return -EINVAL;
> 
>                 fseek(actx->f_in, 0L, SEEK_END);
>                 size = ftell(actx->f_in);
> 
> Thanks,
> Wonjae