linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] Little fix that missed the merge window.
@ 2021-02-26 20:45 Konrad Rzeszutek Wilk
  2021-02-26 20:45 ` [PATCH] cxl: Make loop variable be 'i' instead of 'j' Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 8+ messages in thread
From: Konrad Rzeszutek Wilk @ 2021-02-26 20:45 UTC (permalink / raw)
  To: alison.schofield, vishal.l.verma, ira.weiny, ben.widawsky,
	dan.j.williams, linux-cxl
  Cc: linux-kernel

When the last set of patches were posted for review right before the
merge window this fix was deferred.

Please at your convience pick it up.

 drivers/cxl/mem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Konrad Rzeszutek Wilk (1):
      cxl: Make loop variable be 'i' instead of 'j'



^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH] cxl: Make loop variable be 'i' instead of 'j'
  2021-02-26 20:45 [PATCH v1] Little fix that missed the merge window Konrad Rzeszutek Wilk
@ 2021-02-26 20:45 ` Konrad Rzeszutek Wilk
  2021-02-26 20:55   ` Dan Williams
  2021-02-26 20:55   ` [PATCH] " Al Viro
  0 siblings, 2 replies; 8+ messages in thread
From: Konrad Rzeszutek Wilk @ 2021-02-26 20:45 UTC (permalink / raw)
  To: alison.schofield, vishal.l.verma, ira.weiny, ben.widawsky,
	dan.j.williams, linux-cxl
  Cc: linux-kernel, Konrad Rzeszutek Wilk

.. otherwise people spend extra cycles looking for the
inner loop and wondering 'why j'?

This was an over-sight when initial work was rebased
so lets fix it here.

Fixes: 583fa5e71cae ("cxl/mem: Add basic IOCTL interface")
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/cxl/mem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 244cb7d89678..2b8265b03b0d 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -698,7 +698,7 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd,
 	struct device *dev = &cxlmd->dev;
 	struct cxl_mem_command *cmd;
 	u32 n_commands;
-	int j = 0;
+	int i = 0;
 
 	dev_dbg(dev, "Query IOCTL\n");
 
@@ -716,10 +716,10 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd,
 	cxl_for_each_cmd(cmd) {
 		const struct cxl_command_info *info = &cmd->info;
 
-		if (copy_to_user(&q->commands[j++], info, sizeof(*info)))
+		if (copy_to_user(&q->commands[i++], info, sizeof(*info)))
 			return -EFAULT;
 
-		if (j == n_commands)
+		if (i == n_commands)
 			break;
 	}
 
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] cxl: Make loop variable be 'i' instead of 'j'
  2021-02-26 20:45 ` [PATCH] cxl: Make loop variable be 'i' instead of 'j' Konrad Rzeszutek Wilk
@ 2021-02-26 20:55   ` Dan Williams
  2021-02-26 21:16     ` [PATCH v2] " Konrad Rzeszutek Wilk
  2021-02-26 22:21     ` [PATCH v3] " Konrad Rzeszutek Wilk
  2021-02-26 20:55   ` [PATCH] " Al Viro
  1 sibling, 2 replies; 8+ messages in thread
From: Dan Williams @ 2021-02-26 20:55 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Schofield, Alison, Vishal L Verma, Weiny, Ira, Ben Widawsky,
	linux-cxl, Linux Kernel Mailing List

On Fri, Feb 26, 2021 at 12:46 PM Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
>

Hey, Konrad, thanks for fixing this up. Some more cosmetic changes to
fold in below...

> .. otherwise people spend extra cycles looking for the
> inner loop and wondering 'why j'?
>
> This was an over-sight when initial work was rebased

s/over-sight/oversight/

> so lets fix it here.

s/lets/let's/

> Fixes: 583fa5e71cae ("cxl/mem: Add basic IOCTL interface")

Since it's just cosmetic, I don't think it needs a "Fixes:". The
AUTOSEL-bot need not worry about this.

> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  drivers/cxl/mem.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 244cb7d89678..2b8265b03b0d 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -698,7 +698,7 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd,
>         struct device *dev = &cxlmd->dev;
>         struct cxl_mem_command *cmd;
>         u32 n_commands;
> -       int j = 0;
> +       int i = 0;

Let's move this initialization down to right before the loop. If this
ever gets refactored and @i gets reused this will break.

>
>         dev_dbg(dev, "Query IOCTL\n");
>
> @@ -716,10 +716,10 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd,
>         cxl_for_each_cmd(cmd) {
>                 const struct cxl_command_info *info = &cmd->info;
>
> -               if (copy_to_user(&q->commands[j++], info, sizeof(*info)))
> +               if (copy_to_user(&q->commands[i++], info, sizeof(*info)))
>                         return -EFAULT;
>
> -               if (j == n_commands)
> +               if (i == n_commands)
>                         break;
>         }
>
> --
> 2.13.6
>

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] cxl: Make loop variable be 'i' instead of 'j'
  2021-02-26 20:45 ` [PATCH] cxl: Make loop variable be 'i' instead of 'j' Konrad Rzeszutek Wilk
  2021-02-26 20:55   ` Dan Williams
@ 2021-02-26 20:55   ` Al Viro
  1 sibling, 0 replies; 8+ messages in thread
From: Al Viro @ 2021-02-26 20:55 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: alison.schofield, vishal.l.verma, ira.weiny, ben.widawsky,
	dan.j.williams, linux-cxl, linux-kernel

On Fri, Feb 26, 2021 at 03:45:52PM -0500, Konrad Rzeszutek Wilk wrote:
> .. otherwise people spend extra cycles looking for the
> inner loop and wondering 'why j'?
> 
> This was an over-sight when initial work was rebased
> so lets fix it here.
> 
> Fixes: 583fa5e71cae ("cxl/mem: Add basic IOCTL interface")
  ^^^^^

34 days too early; otherwise, that's a damn good contender...

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH v2] cxl: Make loop variable be 'i' instead of 'j'
  2021-02-26 20:55   ` Dan Williams
@ 2021-02-26 21:16     ` Konrad Rzeszutek Wilk
  2021-03-01 16:55       ` David Laight
  2021-02-26 22:21     ` [PATCH v3] " Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 8+ messages in thread
From: Konrad Rzeszutek Wilk @ 2021-02-26 21:16 UTC (permalink / raw)
  To: alison.schofield, vishal.l.verma, ira.weiny, ben.widawsky,
	dan.j.williams, linux-cxl
  Cc: linux-kernel, Konrad Rzeszutek Wilk

.. otherwise people spend extra cycles looking for the
inner loop and wondering 'why j'?

This was an oversight when initial work was rebased
so let's fix it here.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
v1: Initial posting
v2: Fix per Dan's request
---
 drivers/cxl/mem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 244cb7d89678..d43197a193ce 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -698,7 +698,6 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd,
 	struct device *dev = &cxlmd->dev;
 	struct cxl_mem_command *cmd;
 	u32 n_commands;
-	int j = 0;
 
 	dev_dbg(dev, "Query IOCTL\n");
 
@@ -715,11 +714,12 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd,
 	 */
 	cxl_for_each_cmd(cmd) {
 		const struct cxl_command_info *info = &cmd->info;
+		int i = 0;
 
-		if (copy_to_user(&q->commands[j++], info, sizeof(*info)))
+		if (copy_to_user(&q->commands[i++], info, sizeof(*info)))
 			return -EFAULT;
 
-		if (j == n_commands)
+		if (i == n_commands)
 			break;
 	}
 
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH v3] cxl: Make loop variable be 'i' instead of 'j'
  2021-02-26 20:55   ` Dan Williams
  2021-02-26 21:16     ` [PATCH v2] " Konrad Rzeszutek Wilk
@ 2021-02-26 22:21     ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 8+ messages in thread
From: Konrad Rzeszutek Wilk @ 2021-02-26 22:21 UTC (permalink / raw)
  To: alison.schofield, vishal.l.verma, ira.weiny, ben.widawsky,
	dan.j.williams, linux-cxl
  Cc: linux-kernel, Konrad Rzeszutek Wilk

.. otherwise people spend extra cycles looking for the
inner loop and wondering 'why j'?

This was an oversight when initial work was rebased
so let's fix it here.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
v1: Initial posting
v2: Fix per Dan's request
v3: Duh, don't initialize i in the loop, but do it outside of it.
---
 drivers/cxl/mem.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 244cb7d89678..e7246e585e62 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -698,7 +698,7 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd,
 	struct device *dev = &cxlmd->dev;
 	struct cxl_mem_command *cmd;
 	u32 n_commands;
-	int j = 0;
+	int i;
 
 	dev_dbg(dev, "Query IOCTL\n");
 
@@ -713,13 +713,14 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd,
 	 * otherwise, return max(n_commands, total commands) cxl_command_info
 	 * structures.
 	 */
+	i = 0;
 	cxl_for_each_cmd(cmd) {
 		const struct cxl_command_info *info = &cmd->info;
 
-		if (copy_to_user(&q->commands[j++], info, sizeof(*info)))
+		if (copy_to_user(&q->commands[i++], info, sizeof(*info)))
 			return -EFAULT;
 
-		if (j == n_commands)
+		if (i == n_commands)
 			break;
 	}
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* RE: [PATCH v2] cxl: Make loop variable be 'i' instead of 'j'
  2021-02-26 21:16     ` [PATCH v2] " Konrad Rzeszutek Wilk
@ 2021-03-01 16:55       ` David Laight
  2021-03-03 19:57         ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 8+ messages in thread
From: David Laight @ 2021-03-01 16:55 UTC (permalink / raw)
  To: 'Konrad Rzeszutek Wilk',
	alison.schofield, vishal.l.verma, ira.weiny, ben.widawsky,
	dan.j.williams, linux-cxl
  Cc: linux-kernel

From: Konrad Rzeszutek Wilk
> Sent: 26 February 2021 21:17
> 
> .. otherwise people spend extra cycles looking for the
> inner loop and wondering 'why j'?
> 
> This was an oversight when initial work was rebased
> so let's fix it here.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> v1: Initial posting
> v2: Fix per Dan's request
> ---
>  drivers/cxl/mem.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 244cb7d89678..d43197a193ce 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -698,7 +698,6 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd,
>  	struct device *dev = &cxlmd->dev;
>  	struct cxl_mem_command *cmd;
>  	u32 n_commands;
> -	int j = 0;
> 
>  	dev_dbg(dev, "Query IOCTL\n");
> 
> @@ -715,11 +714,12 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd,
>  	 */
>  	cxl_for_each_cmd(cmd) {
>  		const struct cxl_command_info *info = &cmd->info;
> +		int i = 0;
> 
> -		if (copy_to_user(&q->commands[j++], info, sizeof(*info)))
> +		if (copy_to_user(&q->commands[i++], info, sizeof(*info)))
>  			return -EFAULT;
> 
> -		if (j == n_commands)
> +		if (i == n_commands)
>  			break;


Did you test this?
Looks badly broken to me.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] cxl: Make loop variable be 'i' instead of 'j'
  2021-03-01 16:55       ` David Laight
@ 2021-03-03 19:57         ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 8+ messages in thread
From: Konrad Rzeszutek Wilk @ 2021-03-03 19:57 UTC (permalink / raw)
  To: David Laight
  Cc: alison.schofield, vishal.l.verma, ira.weiny, ben.widawsky,
	dan.j.williams, linux-cxl, linux-kernel

..snip..
> >  	cxl_for_each_cmd(cmd) {
> >  		const struct cxl_command_info *info = &cmd->info;
> > +		int i = 0;
> > 
> > -		if (copy_to_user(&q->commands[j++], info, sizeof(*info)))
> > +		if (copy_to_user(&q->commands[i++], info, sizeof(*info)))
> >  			return -EFAULT;
> > 
> > -		if (j == n_commands)
> > +		if (i == n_commands)
> >  			break;
> 
> 
> Did you test this?
> Looks badly broken to me.

I sent out the v3 which had that fixed. See
https://lore.kernel.org/linux-cxl/20210226222152.48467-1-konrad.wilk@oracle.com/T/#u

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-03-04  0:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-26 20:45 [PATCH v1] Little fix that missed the merge window Konrad Rzeszutek Wilk
2021-02-26 20:45 ` [PATCH] cxl: Make loop variable be 'i' instead of 'j' Konrad Rzeszutek Wilk
2021-02-26 20:55   ` Dan Williams
2021-02-26 21:16     ` [PATCH v2] " Konrad Rzeszutek Wilk
2021-03-01 16:55       ` David Laight
2021-03-03 19:57         ` Konrad Rzeszutek Wilk
2021-02-26 22:21     ` [PATCH v3] " Konrad Rzeszutek Wilk
2021-02-26 20:55   ` [PATCH] " Al Viro

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).