* [PATCH] dax/kmem: Pass valid argument to memory_group_register_static @ 2023-06-20 14:03 Tarun Sahu 2023-06-20 23:18 ` Alison Schofield 0 siblings, 1 reply; 5+ messages in thread From: Tarun Sahu @ 2023-06-20 14:03 UTC (permalink / raw) To: nvdimm, linux-cxl Cc: linux-kernel, dave.jiang, dan.j.williams, vishal.l.verma, aneesh.kumar, jaypatel, tsahu memory_group_register_static takes maximum number of pages as the argument while dev_dax_kmem_probe passes total_len (in bytes) as the argument. Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com> --- drivers/dax/kmem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c index 7b36db6f1cbd..898ca9505754 100644 --- a/drivers/dax/kmem.c +++ b/drivers/dax/kmem.c @@ -99,7 +99,7 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax) if (!data->res_name) goto err_res_name; - rc = memory_group_register_static(numa_node, total_len); + rc = memory_group_register_static(numa_node, PFN_UP(total_len)); if (rc < 0) goto err_reg_mgid; data->mgid = rc; -- 2.31.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] dax/kmem: Pass valid argument to memory_group_register_static 2023-06-20 14:03 [PATCH] dax/kmem: Pass valid argument to memory_group_register_static Tarun Sahu @ 2023-06-20 23:18 ` Alison Schofield 2023-06-21 6:06 ` Tarun Sahu 0 siblings, 1 reply; 5+ messages in thread From: Alison Schofield @ 2023-06-20 23:18 UTC (permalink / raw) To: Tarun Sahu Cc: nvdimm, linux-cxl, linux-kernel, dave.jiang, dan.j.williams, vishal.l.verma, aneesh.kumar, jaypatel On Tue, Jun 20, 2023 at 07:33:32PM +0530, Tarun Sahu wrote: > memory_group_register_static takes maximum number of pages as the argument > while dev_dax_kmem_probe passes total_len (in bytes) as the argument. This sounds like a fix. An explanation of the impact and a fixes tag may be needed. Also, wondering how you found it. Alison > > Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com> > --- > drivers/dax/kmem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c > index 7b36db6f1cbd..898ca9505754 100644 > --- a/drivers/dax/kmem.c > +++ b/drivers/dax/kmem.c > @@ -99,7 +99,7 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax) > if (!data->res_name) > goto err_res_name; > > - rc = memory_group_register_static(numa_node, total_len); > + rc = memory_group_register_static(numa_node, PFN_UP(total_len)); > if (rc < 0) > goto err_reg_mgid; > data->mgid = rc; > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dax/kmem: Pass valid argument to memory_group_register_static 2023-06-20 23:18 ` Alison Schofield @ 2023-06-21 6:06 ` Tarun Sahu 2023-06-21 6:42 ` Verma, Vishal L 0 siblings, 1 reply; 5+ messages in thread From: Tarun Sahu @ 2023-06-21 6:06 UTC (permalink / raw) To: Alison Schofield Cc: nvdimm, linux-cxl, linux-kernel, dave.jiang, dan.j.williams, vishal.l.verma, aneesh.kumar, jaypatel Hi Alison, Alison Schofield <alison.schofield@intel.com> writes: > On Tue, Jun 20, 2023 at 07:33:32PM +0530, Tarun Sahu wrote: >> memory_group_register_static takes maximum number of pages as the argument >> while dev_dax_kmem_probe passes total_len (in bytes) as the argument. > > This sounds like a fix. An explanation of the impact and a fixes tag > may be needed. Also, wondering how you found it. > Yes, it is a fix, I found it during dry code walk-through. There is not any impact as such. As, memory_group_register_static just set the max_pages limit which is used in auto_movable_zone_for_pfn to determine the zone. which might cause these condition to behave differently, This will be true always so jump will happen to kernel_zone if (!auto_movable_can_online_movable(NUMA_NO_NODE, group, nr_pages)) goto kernel_zone; --- kernel_zone: return default_kernel_zone_for_pfn(nid, pfn, nr_pages); --- Here, In below, zone_intersects compare range will be larger as nr_pages will be higher (derived from total_len passed in dev_dax_kmem_probe). static struct zone *default_kernel_zone_for_pfn(int nid, unsigned long start_pfn, unsigned long nr_pages) { struct pglist_data *pgdat = NODE_DATA(nid); int zid; for (zid = 0; zid < ZONE_NORMAL; zid++) { struct zone *zone = &pgdat->node_zones[zid]; if (zone_intersects(zone, start_pfn, nr_pages)) return zone; } return &pgdat->node_zones[ZONE_NORMAL]; } In Mostly cases, ZONE_NORMAL will be returned. But there is no crash/panic issues involved here, only decision making on selecting zone is affected. > Alison > >> >> Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com> >> --- >> drivers/dax/kmem.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c >> index 7b36db6f1cbd..898ca9505754 100644 >> --- a/drivers/dax/kmem.c >> +++ b/drivers/dax/kmem.c >> @@ -99,7 +99,7 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax) >> if (!data->res_name) >> goto err_res_name; >> >> - rc = memory_group_register_static(numa_node, total_len); >> + rc = memory_group_register_static(numa_node, PFN_UP(total_len)); >> if (rc < 0) >> goto err_reg_mgid; >> data->mgid = rc; >> -- >> 2.31.1 >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dax/kmem: Pass valid argument to memory_group_register_static 2023-06-21 6:06 ` Tarun Sahu @ 2023-06-21 6:42 ` Verma, Vishal L 2023-06-22 7:15 ` Tarun Sahu 0 siblings, 1 reply; 5+ messages in thread From: Verma, Vishal L @ 2023-06-21 6:42 UTC (permalink / raw) To: Schofield, Alison, tsahu Cc: Williams, Dan J, Jiang, Dave, linux-cxl, nvdimm, linux-kernel, aneesh.kumar, jaypatel On Wed, 2023-06-21 at 11:36 +0530, Tarun Sahu wrote: > Hi Alison, > > Alison Schofield <alison.schofield@intel.com> writes: > > > On Tue, Jun 20, 2023 at 07:33:32PM +0530, Tarun Sahu wrote: > > > memory_group_register_static takes maximum number of pages as the argument > > > while dev_dax_kmem_probe passes total_len (in bytes) as the argument. > > > > This sounds like a fix. An explanation of the impact and a fixes tag > > may be needed. Also, wondering how you found it. > > > Yes, it is a fix, I found it during dry code walk-through. > There is not any impact as such. As, > memory_group_register_static just set the max_pages limit which > is used in auto_movable_zone_for_pfn to determine the zone. > > which might cause these condition to behave differently, > > This will be true always so jump will happen to kernel_zone > if (!auto_movable_can_online_movable(NUMA_NO_NODE, group, nr_pages)) > goto kernel_zone; > --- > kernel_zone: > return default_kernel_zone_for_pfn(nid, pfn, nr_pages); > > --- > > Here, In below, zone_intersects compare range will be larger as nr_pages > will be higher (derived from total_len passed in dev_dax_kmem_probe). > > static struct zone *default_kernel_zone_for_pfn(int nid, unsigned long start_pfn, > unsigned long nr_pages) > { > struct pglist_data *pgdat = NODE_DATA(nid); > int zid; > > for (zid = 0; zid < ZONE_NORMAL; zid++) { > struct zone *zone = &pgdat->node_zones[zid]; > > if (zone_intersects(zone, start_pfn, nr_pages)) > return zone; > } > > return &pgdat->node_zones[ZONE_NORMAL]; > } > > In Mostly cases, ZONE_NORMAL will be returned. But there is no > crash/panic issues involved here, only decision making on selecting zone > is affected. > Hi Tarun, Good find! With a Fixes tag, and perhaps inclusion of a bit more of this detail described in the commit message, feel free to add: Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dax/kmem: Pass valid argument to memory_group_register_static 2023-06-21 6:42 ` Verma, Vishal L @ 2023-06-22 7:15 ` Tarun Sahu 0 siblings, 0 replies; 5+ messages in thread From: Tarun Sahu @ 2023-06-22 7:15 UTC (permalink / raw) To: Verma, Vishal L, Schofield, Alison Cc: Williams, Dan J, Jiang, Dave, linux-cxl, nvdimm, linux-kernel, aneesh.kumar, jaypatel Hi Vishal, "Verma, Vishal L" <vishal.l.verma@intel.com> writes: > On Wed, 2023-06-21 at 11:36 +0530, Tarun Sahu wrote: >> Hi Alison, >> >> Alison Schofield <alison.schofield@intel.com> writes: >> >> > On Tue, Jun 20, 2023 at 07:33:32PM +0530, Tarun Sahu wrote: >> > > memory_group_register_static takes maximum number of pages as the argument >> > > while dev_dax_kmem_probe passes total_len (in bytes) as the argument. >> > >> > This sounds like a fix. An explanation of the impact and a fixes tag >> > may be needed. Also, wondering how you found it. >> > >> Yes, it is a fix, I found it during dry code walk-through. >> There is not any impact as such. As, >> memory_group_register_static just set the max_pages limit which >> is used in auto_movable_zone_for_pfn to determine the zone. >> >> which might cause these condition to behave differently, >> >> This will be true always so jump will happen to kernel_zone >> if (!auto_movable_can_online_movable(NUMA_NO_NODE, group, nr_pages)) >> goto kernel_zone; >> --- >> kernel_zone: >> return default_kernel_zone_for_pfn(nid, pfn, nr_pages); >> >> --- >> >> Here, In below, zone_intersects compare range will be larger as nr_pages >> will be higher (derived from total_len passed in dev_dax_kmem_probe). >> >> static struct zone *default_kernel_zone_for_pfn(int nid, unsigned long start_pfn, >> unsigned long nr_pages) >> { >> struct pglist_data *pgdat = NODE_DATA(nid); >> int zid; >> >> for (zid = 0; zid < ZONE_NORMAL; zid++) { >> struct zone *zone = &pgdat->node_zones[zid]; >> >> if (zone_intersects(zone, start_pfn, nr_pages)) >> return zone; >> } >> >> return &pgdat->node_zones[ZONE_NORMAL]; >> } >> >> In Mostly cases, ZONE_NORMAL will be returned. But there is no >> crash/panic issues involved here, only decision making on selecting zone >> is affected. >> > > Hi Tarun, > > Good find! With a Fixes tag, and perhaps inclusion of a bit more of > this detail described in the commit message, feel free to add: > Thanks for reviewing, sent the updated version. > Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-22 7:16 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-06-20 14:03 [PATCH] dax/kmem: Pass valid argument to memory_group_register_static Tarun Sahu 2023-06-20 23:18 ` Alison Schofield 2023-06-21 6:06 ` Tarun Sahu 2023-06-21 6:42 ` Verma, Vishal L 2023-06-22 7:15 ` Tarun Sahu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).