[PATCH] [ndctl PATCH v2] ndctl/list: display region caps for any of BTT, PFN, DAX

[PATCH] [ndctl PATCH v2] ndctl/list: display region caps for any of BTT, PFN, DAX

[Donet Tom]

If any one of BTT, PFN, or DAX is not present, but the other two
are, then the region capabilities are not displayed in the
ndctl list -R -C command.

This is because util_region_capabilities_to_json() returns NULL
if any one of BTT, PFN, or DAX is not present.

In this patch, we have changed the logic to display all the region
capabilities that are present.

Test Results with CONFIG_BTT disabled
=====================================
Without this patch
------------------
 # ./ndctl list -R -C
 [
  {
    "dev":"region1",
    "size":12884901888,
    "align":16777216,
    "available_size":11257511936,
    "max_available_extent":9630121984,
    "type":"pmem",
    "iset_id":14748366918514061582,
    "persistence_domain":"unknown"
  },

With this patch
---------------
 # ./ndctl list -R -C
 [
  {
    "dev":"region1",
    "size":12884901888,
    "align":16777216,
    "available_size":11257511936,
    "max_available_extent":9630121984,
    "type":"pmem",
    "iset_id":14748366918514061582,
    "capabilities":[
      {
        "mode":"fsdax",
        "alignments":[
          65536,
          2097152,
          1073741824
        ]
      },
      {
        "mode":"devdax",
        "alignments":[
          65536,
          2097152,
          1073741824
        ]
      }
    ],
    "persistence_domain":"unknown"
  },

v1 -> v2:
Addressed the review comments from Jeff and Alison.

v1:
https://lore.kernel.org/all/20250219094049.5156-1-donettom@linux.ibm.com/

Fixes: 965fa02e372f ("util: Distribute 'filter' and 'json' helpers to per-tool objects")
Signed-off-by: Donet Tom <donettom@linux.vnet.ibm.com>
---
 ndctl/json.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ndctl/json.c b/ndctl/json.c
index 23bad7f..7646882 100644
--- a/ndctl/json.c
+++ b/ndctl/json.c
@@ -381,7 +381,7 @@ struct json_object *util_region_capabilities_to_json(struct ndctl_region *region
 	struct ndctl_pfn *pfn = ndctl_region_get_pfn_seed(region);
 	struct ndctl_dax *dax = ndctl_region_get_dax_seed(region);
 
-	if (!btt || !pfn || !dax)
+	if (!btt && !pfn && !dax)
 		return NULL;
 
 	jcaps = json_object_new_array();
-- 
2.47.1

Re: [PATCH] [ndctl PATCH v2] ndctl/list: display region caps for any of BTT, PFN, DAX

[Jeff Moyer]

> diff --git a/ndctl/json.c b/ndctl/json.c
> index 23bad7f..7646882 100644
> --- a/ndctl/json.c
> +++ b/ndctl/json.c
> @@ -381,7 +381,7 @@ struct json_object *util_region_capabilities_to_json(struct ndctl_region *region
>  	struct ndctl_pfn *pfn = ndctl_region_get_pfn_seed(region);
>  	struct ndctl_dax *dax = ndctl_region_get_dax_seed(region);
>  
> -	if (!btt || !pfn || !dax)
> +	if (!btt && !pfn && !dax)
>  		return NULL;
>  
>  	jcaps = json_object_new_array();

Reviewed-by: Jeff Moyer <jmoyer@redhat.com>

Re: [PATCH] [ndctl PATCH v2] ndctl/list: display region caps for any of BTT, PFN, DAX

[Donet Tom]

On 2/20/25 19:05, Jeff Moyer wrote:
>> diff --git a/ndctl/json.c b/ndctl/json.c
>> index 23bad7f..7646882 100644
>> --- a/ndctl/json.c
>> +++ b/ndctl/json.c
>> @@ -381,7 +381,7 @@ struct json_object *util_region_capabilities_to_json(struct ndctl_region *region
>>   	struct ndctl_pfn *pfn = ndctl_region_get_pfn_seed(region);
>>   	struct ndctl_dax *dax = ndctl_region_get_dax_seed(region);
>>   
>> -	if (!btt || !pfn || !dax)
>> +	if (!btt && !pfn && !dax)
>>   		return NULL;
>>   
>>   	jcaps = json_object_new_array();
> Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
>
Thanks Jeff


Hi Alison

Should I send a v3 with Reviewed-by tag or will you take the patch with 
the tag?

Thanks
Donet

>

Re: [PATCH] [ndctl PATCH v2] ndctl/list: display region caps for any of BTT, PFN, DAX

[Zhijian Li (Fujitsu)]

On 26/02/2025 14:02, Donet Tom wrote:
> 
> On 2/20/25 19:05, Jeff Moyer wrote:
>>> diff --git a/ndctl/json.c b/ndctl/json.c
>>> index 23bad7f..7646882 100644
>>> --- a/ndctl/json.c
>>> +++ b/ndctl/json.c
>>> @@ -381,7 +381,7 @@ struct json_object *util_region_capabilities_to_json(struct ndctl_region *region
>>>       struct ndctl_pfn *pfn = ndctl_region_get_pfn_seed(region);
>>>       struct ndctl_dax *dax = ndctl_region_get_dax_seed(region);
>>> -    if (!btt || !pfn || !dax)
>>> +    if (!btt && !pfn && !dax)
>>>           return NULL;
>>>       jcaps = json_object_new_array();
>> Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
>>
> Thanks Jeff
> 
> 
> Hi Alison
> 
> Should I send a v3 with Reviewed-by tag 

Generally speaking, there is no need for you to do this.

A small tip might be helpful if you send a patch next time.

>> 
>> v1 -> v2:
>> Addressed the review comments from Jeff and Alison.
>> 
>> v1:
>> https://lore.kernel.org/all/20250219094049.5156-1-donettom@linux.ibm.com/

Commit messages that are not intended to appear in the upstream git tree
should be placed after the '---' marker. [0]

[0] https://docs.kernel.org/process/submitting-patches.html#commentary

Thanks
Zhijian

> or will you take the patch with the tag?
> 
> Thanks
> Donet
> 
>>
> 

Re: [PATCH] [ndctl PATCH v2] ndctl/list: display region caps for any of BTT, PFN, DAX

[Zhijian Li (Fujitsu)]

On 20/02/2025 14:20, Donet Tom wrote:
> If any one of BTT, PFN, or DAX is not present, but the other two
> are, then the region capabilities are not displayed in the
> ndctl list -R -C command.
> 
> This is because util_region_capabilities_to_json() returns NULL
> if any one of BTT, PFN, or DAX is not present.
> 
> In this patch, we have changed the logic to display all the region
> capabilities that are present.
> 
> Test Results with CONFIG_BTT disabled
> =====================================
> Without this patch
> ------------------
>   # ./ndctl list -R -C
>   [
>    {
>      "dev":"region1",
>      "size":12884901888,
>      "align":16777216,
>      "available_size":11257511936,
>      "max_available_extent":9630121984,
>      "type":"pmem",
>      "iset_id":14748366918514061582,
>      "persistence_domain":"unknown"
>    },
> 
> With this patch
> ---------------
>   # ./ndctl list -R -C
>   [
>    {
>      "dev":"region1",
>      "size":12884901888,
>      "align":16777216,
>      "available_size":11257511936,
>      "max_available_extent":9630121984,
>      "type":"pmem",
>      "iset_id":14748366918514061582,
>      "capabilities":[
>        {
>          "mode":"fsdax",
>          "alignments":[
>            65536,
>            2097152,
>            1073741824
>          ]
>        },
>        {
>          "mode":"devdax",
>          "alignments":[
>            65536,
>            2097152,
>            1073741824
>          ]
>        }
>      ],
>      "persistence_domain":"unknown"
>    },
> 
> v1 -> v2:
> Addressed the review comments from Jeff and Alison.
> 
> v1:
> https://lore.kernel.org/all/20250219094049.5156-1-donettom@linux.ibm.com/
> 
> Fixes: 965fa02e372f ("util: Distribute 'filter' and 'json' helpers to per-tool objects")
> Signed-off-by: Donet Tom <donettom@linux.vnet.ibm.com>


Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
Tested-by: Li Zhijian <lizhijian@fujitsu.com>


> ---
>   ndctl/json.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ndctl/json.c b/ndctl/json.c
> index 23bad7f..7646882 100644
> --- a/ndctl/json.c
> +++ b/ndctl/json.c
> @@ -381,7 +381,7 @@ struct json_object *util_region_capabilities_to_json(struct ndctl_region *region
>   	struct ndctl_pfn *pfn = ndctl_region_get_pfn_seed(region);
>   	struct ndctl_dax *dax = ndctl_region_get_dax_seed(region);
>   
> -	if (!btt || !pfn || !dax)
> +	if (!btt && !pfn && !dax)
>   		return NULL;
>   
>   	jcaps = json_object_new_array();

Re: [PATCH] [ndctl PATCH v2] ndctl/list: display region caps for any of BTT, PFN, DAX

[Alison Schofield]

On Wed, Feb 26, 2025 at 11:32:30AM +0530, Donet Tom wrote:
> 
> On 2/20/25 19:05, Jeff Moyer wrote:
> > > diff --git a/ndctl/json.c b/ndctl/json.c
> > > index 23bad7f..7646882 100644
> > > --- a/ndctl/json.c
> > > +++ b/ndctl/json.c
> > > @@ -381,7 +381,7 @@ struct json_object *util_region_capabilities_to_json(struct ndctl_region *region
> > >   	struct ndctl_pfn *pfn = ndctl_region_get_pfn_seed(region);
> > >   	struct ndctl_dax *dax = ndctl_region_get_dax_seed(region);
> > > -	if (!btt || !pfn || !dax)
> > > +	if (!btt && !pfn && !dax)
> > >   		return NULL;
> > >   	jcaps = json_object_new_array();
> > Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
> > 
> Thanks Jeff
> 
> 
> Hi Alison
> 
> Should I send a v3 with Reviewed-by tag or will you take the patch with the
> tag?

No need. The tags with gather automagically upon applying.
I'll also rm the text meant for below the --- like Zhijian noted.

Thanks for the patch!


> 
> Thanks
> Donet
> 
> > 

Re: [PATCH] [ndctl PATCH v2] ndctl/list: display region caps for any of BTT, PFN, DAX

[Donet Tom]

On 2/26/25 12:30, Zhijian Li (Fujitsu) wrote:
>
> On 26/02/2025 14:02, Donet Tom wrote:
>> On 2/20/25 19:05, Jeff Moyer wrote:
>>>> diff --git a/ndctl/json.c b/ndctl/json.c
>>>> index 23bad7f..7646882 100644
>>>> --- a/ndctl/json.c
>>>> +++ b/ndctl/json.c
>>>> @@ -381,7 +381,7 @@ struct json_object *util_region_capabilities_to_json(struct ndctl_region *region
>>>>        struct ndctl_pfn *pfn = ndctl_region_get_pfn_seed(region);
>>>>        struct ndctl_dax *dax = ndctl_region_get_dax_seed(region);
>>>> -    if (!btt || !pfn || !dax)
>>>> +    if (!btt && !pfn && !dax)
>>>>            return NULL;
>>>>        jcaps = json_object_new_array();
>>> Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
>>>
>> Thanks Jeff
>>
>>
>> Hi Alison
>>
>> Should I send a v3 with Reviewed-by tag
> Generally speaking, there is no need for you to do this.
>
> A small tip might be helpful if you send a patch next time.
>
>>> v1 -> v2:
>>> Addressed the review comments from Jeff and Alison.
>>>
>>> v1:
>>> https://lore.kernel.org/all/20250219094049.5156-1-donettom@linux.ibm.com/
> Commit messages that are not intended to appear in the upstream git tree
> should be placed after the '---' marker. [0]
>

Thanks Zhijian. I will take care of this from now on.

Thanks
Donet


>
> [0] https://docs.kernel.org/process/submitting-patches.html#commentary
>
> Thanks
> Zhijian
>
>> or will you take the patch with the tag?
>>
>> Thanks
>> Donet
>>

Re: [PATCH] [ndctl PATCH v2] ndctl/list: display region caps for any of BTT, PFN, DAX

[Donet Tom]

On 2/27/25 01:13, Alison Schofield wrote:
> On Wed, Feb 26, 2025 at 11:32:30AM +0530, Donet Tom wrote:
>> On 2/20/25 19:05, Jeff Moyer wrote:
>>>> diff --git a/ndctl/json.c b/ndctl/json.c
>>>> index 23bad7f..7646882 100644
>>>> --- a/ndctl/json.c
>>>> +++ b/ndctl/json.c
>>>> @@ -381,7 +381,7 @@ struct json_object *util_region_capabilities_to_json(struct ndctl_region *region
>>>>    	struct ndctl_pfn *pfn = ndctl_region_get_pfn_seed(region);
>>>>    	struct ndctl_dax *dax = ndctl_region_get_dax_seed(region);
>>>> -	if (!btt || !pfn || !dax)
>>>> +	if (!btt && !pfn && !dax)
>>>>    		return NULL;
>>>>    	jcaps = json_object_new_array();
>>> Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
>>>
>> Thanks Jeff
>>
>>
>> Hi Alison
>>
>> Should I send a v3 with Reviewed-by tag or will you take the patch with the
>> tag?
> No need. The tags with gather automagically upon applying.
> I'll also rm the text meant for below the --- like Zhijian noted.


Thank you.


>
> Thanks for the patch!
>
>
>> Thanks
>> Donet
>>