| Message ID | 20250220062029.9789-1-donettom@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Queued |
| Headers | show |
| Series | [ndctl,v2] ndctl/list: display region caps for any of BTT, PFN, DAX | expand |
> 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>
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 >
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 > >> >
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();
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 > > >
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 >>
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 >>
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();
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(-)