* [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
* [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
* 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
* [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] 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
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).