diff mbox series

[NDCTL,1/2] daxctl: Don't check param.no_movable when param.no_online is set

Message ID 20230810053958.14992-1-yangx.jy@fujitsu.com
State New, archived
Headers show
Series [NDCTL,1/2] daxctl: Don't check param.no_movable when param.no_online is set | expand

Commit Message

Xiao Yang Aug. 10, 2023, 5:39 a.m. UTC
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(-)

Comments

Xiao Yang Oct. 9, 2023, 9:40 a.m. UTC | #1
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
Xiao Yang Jan. 15, 2024, 12:12 a.m. UTC | #2
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
>
Dan Williams Jan. 17, 2024, 12:16 a.m. UTC | #3
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 mbox series

Patch

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",