[dm-devel] [PATCH 1/2] multipath.conf(5): remove io-affinity information

[dm-devel] [PATCH 1/2] multipath.conf(5): remove io-affinity information

[Benjamin Marzinski]

The multpath-tools do not support the io-affinity path selector.  We
always add a repeat count as the path argument. The io-affinity selector
doesn't take one. Instead it takes a bitmap of CPUs that a path can run
on. This isn't something that lends itself to the kind of
auto-assembling that multipathd does. But even if we did want to try to
support this path-selector, until we do, we shouldn't be listing it in
the multipath.conf documentation.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipath/multipath.conf.5 | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 1fea9d5a..3a45ac89 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -205,10 +205,6 @@ of outstanding I/O to the path and its relative throughput.
 estimation of future service time based on the history of previous I/O submitted
 to each path.
 .TP
-.I "io-affinity 0"
-(Since 5.11 kernel) Choose the path for the next bunch of I/O based on a CPU to
-path mapping the user passes in and what CPU we are executing on.
-.TP
 The default is: \fBservice-time 0\fR
 .RE
 .
-- 
2.17.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 2/2] libmpathpersist: fix command keyword ordering

[Benjamin Marzinski]

When libmpathpersist was communicating with multipathd, it wasn't using
the correct keyword order for the commands, as specified in the CLI
commands reference. Since commit f812466f, multipathd requires commands
to be ordered correctly. Fix the ordering.

Fixes: f812466f ("multipathd: more robust command parsing")
Reported-by: miaoguanqin <miaoguanqin@huawei.com>
Cc: lixiaokeng <lixiaokeng@huawei.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmpathpersist/mpath_updatepr.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/libmpathpersist/mpath_updatepr.c b/libmpathpersist/mpath_updatepr.c
index 5824c169..4529a82b 100644
--- a/libmpathpersist/mpath_updatepr.c
+++ b/libmpathpersist/mpath_updatepr.c
@@ -18,7 +18,7 @@
 #include "mpathpr.h"
 
 
-static int do_update_pr(char *alias, char *arg)
+static int do_update_pr(char *alias, char *cmd, char *key)
 {
 	int fd;
 	char str[256];
@@ -31,7 +31,10 @@ static int do_update_pr(char *alias, char *arg)
 		return -1;
 	}
 
-	snprintf(str,sizeof(str),"map %s %s", alias, arg);
+	if (key)
+		snprintf(str,sizeof(str),"%s map %s key %s", cmd, alias, key);
+	else
+		snprintf(str,sizeof(str),"%s map %s", cmd, alias);
 	condlog (2, "%s: pr message=%s", alias, str);
 	if (send_packet(fd, str) != 0) {
 		condlog(2, "%s: message=%s send error=%d", alias, str, errno);
@@ -56,18 +59,16 @@ static int do_update_pr(char *alias, char *arg)
 }
 
 int update_prflag(char *mapname, int set) {
-	return do_update_pr(mapname, (set)? "setprstatus" : "unsetprstatus");
+	return do_update_pr(mapname, (set)? "setprstatus" : "unsetprstatus",
+			    NULL);
 }
 
 int update_prkey_flags(char *mapname, uint64_t prkey, uint8_t sa_flags) {
 	char str[256];
-	char *flagstr = "";
 
-	if (sa_flags & MPATH_F_APTPL_MASK)
-		flagstr = ":aptpl";
-	if (prkey)
-		sprintf(str, "setprkey key %" PRIx64 "%s", prkey, flagstr);
-	else
-		sprintf(str, "unsetprkey");
-	return do_update_pr(mapname, str);
+	if (!prkey)
+		return do_update_pr(mapname, "unsetprkey", NULL);
+	sprintf(str, "%" PRIx64 "%s", prkey,
+		(sa_flags & MPATH_F_APTPL_MASK) ? ":aptpl" : "");
+	return do_update_pr(mapname, "setprkey", str);
 }
-- 
2.17.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/2] multipath.conf(5): remove io-affinity information

[Martin Wilck]

On Tue, 2022-11-29 at 22:56 -0600, Benjamin Marzinski wrote:
> The multpath-tools do not support the io-affinity path selector.  We
> always add a repeat count as the path argument. The io-affinity
> selector
> doesn't take one. Instead it takes a bitmap of CPUs that a path can
> run
> on. This isn't something that lends itself to the kind of
> auto-assembling that multipathd does. But even if we did want to try
> to
> support this path-selector, until we do, we shouldn't be listing it
> in
> the multipath.conf documentation.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 2/2] libmpathpersist: fix command keyword ordering

[Martin Wilck]

On Tue, 2022-11-29 at 22:56 -0600, Benjamin Marzinski wrote:
> When libmpathpersist was communicating with multipathd, it wasn't
> using
> the correct keyword order for the commands, as specified in the CLI
> commands reference. Since commit f812466f, multipathd requires
> commands
> to be ordered correctly. Fix the ordering.
> 
> Fixes: f812466f ("multipathd: more robust command parsing")
> Reported-by: miaoguanqin <miaoguanqin@huawei.com>
> Cc: lixiaokeng <lixiaokeng@huawei.com>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>

> ---
>  libmpathpersist/mpath_updatepr.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/libmpathpersist/mpath_updatepr.c
> b/libmpathpersist/mpath_updatepr.c
> index 5824c169..4529a82b 100644
> --- a/libmpathpersist/mpath_updatepr.c
> +++ b/libmpathpersist/mpath_updatepr.c
> @@ -18,7 +18,7 @@
>  #include "mpathpr.h"
>  
>  
> -static int do_update_pr(char *alias, char *arg)
> +static int do_update_pr(char *alias, char *cmd, char *key)
>  {
>         int fd;
>         char str[256];
> @@ -31,7 +31,10 @@ static int do_update_pr(char *alias, char *arg)
>                 return -1;
>         }
>  
> -       snprintf(str,sizeof(str),"map %s %s", alias, arg);
> +       if (key)
> +               snprintf(str,sizeof(str),"%s map %s key %s", cmd,
> alias, key);
> +       else
> +               snprintf(str,sizeof(str),"%s map %s", cmd, alias);
>         condlog (2, "%s: pr message=%s", alias, str);
>         if (send_packet(fd, str) != 0) {
>                 condlog(2, "%s: message=%s send error=%d", alias,
> str, errno);
> @@ -56,18 +59,16 @@ static int do_update_pr(char *alias, char *arg)
>  }
>  
>  int update_prflag(char *mapname, int set) {
> -       return do_update_pr(mapname, (set)? "setprstatus" :
> "unsetprstatus");
> +       return do_update_pr(mapname, (set)? "setprstatus" :
> "unsetprstatus",
> +                           NULL);
>  }
>  
>  int update_prkey_flags(char *mapname, uint64_t prkey, uint8_t
> sa_flags) {
>         char str[256];
> -       char *flagstr = "";
>  
> -       if (sa_flags & MPATH_F_APTPL_MASK)
> -               flagstr = ":aptpl";
> -       if (prkey)
> -               sprintf(str, "setprkey key %" PRIx64 "%s", prkey,
> flagstr);
> -       else
> -               sprintf(str, "unsetprkey");
> -       return do_update_pr(mapname, str);
> +       if (!prkey)
> +               return do_update_pr(mapname, "unsetprkey", NULL);
> +       sprintf(str, "%" PRIx64 "%s", prkey,
> +               (sa_flags & MPATH_F_APTPL_MASK) ? ":aptpl" : "");
> +       return do_update_pr(mapname, "setprkey", str);
>  }

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/2] multipath.conf(5): remove io-affinity information

[Xose Vazquez Perez]

On 11/30/22 05:56, bmarzins at redhat.com (Benjamin Marzinski) wrote:

> The multpath-tools do not support the io-affinity path selector.  We
> always add a repeat count as the path argument. The io-affinity selector
> doesn't take one. Instead it takes a bitmap of CPUs that a path can run
> on. This isn't something that lends itself to the kind of
> auto-assembling that multipathd does. But even if we did want to try to
> support this path-selector, until we do, we shouldn't be listing it in
> the multipath.conf documentation.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>

> ---
>   multipath/multipath.conf.5 | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
> index 1fea9d5a..3a45ac89 100644
> --- a/multipath/multipath.conf.5
> +++ b/multipath/multipath.conf.5
> @@ -205,10 +205,6 @@ of outstanding I/O to the path and its relative throughput.
>   estimation of future service time based on the history of previous I/O submitted
>   to each path.
>   .TP
> -.I "io-affinity 0"
> -(Since 5.11 kernel) Choose the path for the next bunch of I/O based on a CPU to
> -path mapping the user passes in and what CPU we are executing on.
> -.TP
>   The default is: \fBservice-time 0\fR
>   .RE
>   .

I think the main, and only?, consumer for this path selector is Exadata.

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/2] multipath.conf(5): remove io-affinity information

[Mike Christie]

On 12/3/22 9:10 AM, Xose Vazquez Perez wrote:
> On 11/30/22 05:56, bmarzins at redhat.com (Benjamin Marzinski) wrote:
> 
>> The multpath-tools do not support the io-affinity path selector.  We
>> always add a repeat count as the path argument. The io-affinity selector
>> doesn't take one. Instead it takes a bitmap of CPUs that a path can run
>> on. This isn't something that lends itself to the kind of
>> auto-assembling that multipathd does. But even if we did want to try to
>> support this path-selector, until we do, we shouldn't be listing it in
>> the multipath.conf documentation.
>>
>> Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>
> 
>> ---
>>   multipath/multipath.conf.5 | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
>> index 1fea9d5a..3a45ac89 100644
>> --- a/multipath/multipath.conf.5
>> +++ b/multipath/multipath.conf.5
>> @@ -205,10 +205,6 @@ of outstanding I/O to the path and its relative throughput.
>>   estimation of future service time based on the history of previous I/O submitted
>>   to each path.
>>   .TP
>> -.I "io-affinity 0"
>> -(Since 5.11 kernel) Choose the path for the next bunch of I/O based on a CPU to
>> -path mapping the user passes in and what CPU we are executing on.
>> -.TP
>>   The default is: \fBservice-time 0\fR
>>   .RE
>>   .
> 
> I think the main, and only?, consumer for this path selector is Exadata.

Not exadata but another user in oracle.

The patch is ok with me. We've been setting it up manually. When I get time
to do multipathd support I'll update the docs as well.


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel