| Message ID | 20230810053958.14992-1-yangx.jy@fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Vishal Verma |
| Headers | show |
| Series | [NDCTL,1/2] daxctl: Don't check param.no_movable when param.no_online is set | expand |
Ping -----Original Message----- From: Xiao Yang <yangx.jy@fujitsu.com> Sent: 2023年8月10日 13:40 To: vishal.l.verma@intel.com; nvdimm@lists.linux.dev Cc: linux-cxl@vger.kernel.org; Yang, Xiao/杨 晓 <yangx.jy@fujitsu.com> Subject: [NDCTL PATCH 1/2] daxctl: Don't check param.no_movable when param.no_online is set param.no_movable is used to online memory in ZONE_NORMAL but param.no_online is used to not online memory. So it's unnecessary to check param.no_movable when param.no_online is set. Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> --- daxctl/device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daxctl/device.c b/daxctl/device.c index 360ae8b..ba31eb6 100644 --- a/daxctl/device.c +++ b/daxctl/device.c @@ -711,7 +711,7 @@ static int reconfig_mode_system_ram(struct daxctl_dev *dev) const char *devname = daxctl_dev_get_devname(dev); int rc, skip_enable = 0; - if (param.no_online || !param.no_movable) { + if (param.no_online) { if (!param.force && daxctl_dev_will_auto_online_memory(dev)) { fprintf(stderr, "%s: error: kernel policy will auto-online memory, aborting\n", -- 2.40.1
Hi All, Ping, is there any feedback on this patchset? Best Regards, Xiao Yang On 2023/10/9 17:40, Yang, Xiao/杨 晓 wrote: > Ping > > -----Original Message----- > From: Xiao Yang <yangx.jy@fujitsu.com> > Sent: 2023年8月10日 13:40 > To: vishal.l.verma@intel.com; nvdimm@lists.linux.dev > Cc: linux-cxl@vger.kernel.org; Yang, Xiao/杨 晓 <yangx.jy@fujitsu.com> > Subject: [NDCTL PATCH 1/2] daxctl: Don't check param.no_movable when param.no_online is set > > param.no_movable is used to online memory in ZONE_NORMAL but param.no_online is used to not online memory. So it's unnecessary to check param.no_movable when param.no_online is set. > > Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> > --- > daxctl/device.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/daxctl/device.c b/daxctl/device.c index 360ae8b..ba31eb6 100644 > --- a/daxctl/device.c > +++ b/daxctl/device.c > @@ -711,7 +711,7 @@ static int reconfig_mode_system_ram(struct daxctl_dev *dev) > const char *devname = daxctl_dev_get_devname(dev); > int rc, skip_enable = 0; > > - if (param.no_online || !param.no_movable) { > + if (param.no_online) { > if (!param.force && daxctl_dev_will_auto_online_memory(dev)) { > fprintf(stderr, > "%s: error: kernel policy will auto-online memory, aborting\n", > -- > 2.40.1 >
Hey, apologies for the delay here, and appreciate the pings. Xiao Yang wrote: > param.no_movable is used to online memory in ZONE_NORMAL but > param.no_online is used to not online memory. So it's unnecessary > to check param.no_movable when param.no_online is set. > > Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> > --- > daxctl/device.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/daxctl/device.c b/daxctl/device.c > index 360ae8b..ba31eb6 100644 > --- a/daxctl/device.c > +++ b/daxctl/device.c > @@ -711,7 +711,7 @@ static int reconfig_mode_system_ram(struct daxctl_dev *dev) > const char *devname = daxctl_dev_get_devname(dev); > int rc, skip_enable = 0; > > - if (param.no_online || !param.no_movable) { > + if (param.no_online) { I agree that if daxctl is not going to online the memory then it does not matter what no_movable is set to. However, part of what is happening here is to communicate when daxctl options collide with kernel policy. I.e. consider the case where no_movable is set and the kernel policy is set to online_movable, then daxctl should communicate: "error: kernel policy will auto-online memory, aborting" Maybe the fix here is to split the no-online vs "online or online_movable" kernel policy, and the no-movable vs "online_movable" kernel policy.
diff --git a/daxctl/device.c b/daxctl/device.c index 360ae8b..ba31eb6 100644 --- a/daxctl/device.c +++ b/daxctl/device.c @@ -711,7 +711,7 @@ static int reconfig_mode_system_ram(struct daxctl_dev *dev) const char *devname = daxctl_dev_get_devname(dev); int rc, skip_enable = 0; - if (param.no_online || !param.no_movable) { + if (param.no_online) { if (!param.force && daxctl_dev_will_auto_online_memory(dev)) { fprintf(stderr, "%s: error: kernel policy will auto-online memory, aborting\n",
param.no_movable is used to online memory in ZONE_NORMAL but param.no_online is used to not online memory. So it's unnecessary to check param.no_movable when param.no_online is set. Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> --- daxctl/device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)