2019-07-26 09:26:52

by Anders Roxell

[permalink] [raw]
Subject: [PATCH] rdma: siw: remove unused variable

The variable 'p' si no longer used and the compiler rightly complains
that it should be removed.

../drivers/infiniband/sw/siw/siw_mem.c: In function ‘siw_free_plist’:
../drivers/infiniband/sw/siw/siw_mem.c:66:16: warning: unused variable
‘p’ [-Wunused-variable]
struct page **p = chunk->plist;
^

Rework to remove unused variable.

Fixes: 8288d030447f ("mm/gup: add make_dirty arg to put_user_pages_dirty_lock()")
Signed-off-by: Anders Roxell <[email protected]>
---
drivers/infiniband/sw/siw/siw_mem.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_mem.c b/drivers/infiniband/sw/siw/siw_mem.c
index 358d440efa11..ab83a9cec562 100644
--- a/drivers/infiniband/sw/siw/siw_mem.c
+++ b/drivers/infiniband/sw/siw/siw_mem.c
@@ -63,8 +63,6 @@ struct siw_mem *siw_mem_id2obj(struct siw_device *sdev, int stag_index)
static void siw_free_plist(struct siw_page_chunk *chunk, int num_pages,
bool dirty)
{
- struct page **p = chunk->plist;
-
put_user_pages_dirty_lock(chunk->plist, num_pages, dirty);
}

--
2.20.1



2019-07-26 19:21:20

by Bernard Metzler

[permalink] [raw]
Subject: Re: [PATCH] rdma: siw: remove unused variable

-----"Anders Roxell" <[email protected]> wrote: -----

>To: [email protected], [email protected], [email protected]
>From: "Anders Roxell" <[email protected]>
>Date: 07/26/2019 11:26AM
>Cc: [email protected], [email protected], "Anders
>Roxell" <[email protected]>
>Subject: [EXTERNAL] [PATCH] rdma: siw: remove unused variable
>
>The variable 'p' si no longer used and the compiler rightly complains
>that it should be removed.
>
>../drivers/infiniband/sw/siw/siw_mem.c: In function ‘siw_free_plist’:
>../drivers/infiniband/sw/siw/siw_mem.c:66:16: warning: unused
>variable
> ‘p’ [-Wunused-variable]
> struct page **p = chunk->plist;
> ^
>
>Rework to remove unused variable.
>
>Fixes: 8288d030447f ("mm/gup: add make_dirty arg to
>put_user_pages_dirty_lock()")
>Signed-off-by: Anders Roxell <[email protected]>
>---
> drivers/infiniband/sw/siw/siw_mem.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/infiniband/sw/siw/siw_mem.c
>b/drivers/infiniband/sw/siw/siw_mem.c
>index 358d440efa11..ab83a9cec562 100644
>--- a/drivers/infiniband/sw/siw/siw_mem.c
>+++ b/drivers/infiniband/sw/siw/siw_mem.c
>@@ -63,8 +63,6 @@ struct siw_mem *siw_mem_id2obj(struct siw_device
>*sdev, int stag_index)
> static void siw_free_plist(struct siw_page_chunk *chunk, int
>num_pages,
> bool dirty)
> {
>- struct page **p = chunk->plist;
>-
> put_user_pages_dirty_lock(chunk->plist, num_pages, dirty);
> }
>
>--
>2.20.1
>
>

If we can cut down siw_free_plist() to just calling
put_user_pages_dirty_lock(), we shall better call it directly
and not obfuscate that by another function.

Reviewed-by: Bernard Metzler <[email protected]>


2019-07-29 19:31:12

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH] rdma: siw: remove unused variable

On Mon, Jul 29, 2019 at 02:19:35PM -0400, Doug Ledford wrote:
> On Fri, 2019-07-26 at 11:25 +0200, Anders Roxell wrote:
> > The variable 'p' si no longer used and the compiler rightly complains
> > that it should be removed.
> >
> > ../drivers/infiniband/sw/siw/siw_mem.c: In function ‘siw_free_plist’:
> > ../drivers/infiniband/sw/siw/siw_mem.c:66:16: warning: unused variable
> > ‘p’ [-Wunused-variable]
> > struct page **p = chunk->plist;
> > ^
> >
> > Rework to remove unused variable.
> >
> > Fixes: 8288d030447f ("mm/gup: add make_dirty arg to
> > put_user_pages_dirty_lock()")
>
> This commit hash and the commit subject does not exist in Linus' tree as
> of today. What tree is this being merged through, and is it slated to
> merge soon or is this a for-next item?

This is though -mm, maybe John knows what is what

Jason

2019-07-29 19:47:21

by Doug Ledford

[permalink] [raw]
Subject: Re: [PATCH] rdma: siw: remove unused variable

On Mon, 2019-07-29 at 16:03 -0300, Jason Gunthorpe wrote:
> On Mon, Jul 29, 2019 at 02:19:35PM -0400, Doug Ledford wrote:
> > On Fri, 2019-07-26 at 11:25 +0200, Anders Roxell wrote:
> > > The variable 'p' si no longer used and the compiler rightly
> > > complains
> > > that it should be removed.
> > >
> > > ../drivers/infiniband/sw/siw/siw_mem.c: In function
> > > ‘siw_free_plist’:
> > > ../drivers/infiniband/sw/siw/siw_mem.c:66:16: warning: unused
> > > variable
> > > ‘p’ [-Wunused-variable]
> > > struct page **p = chunk->plist;
> > > ^
> > >
> > > Rework to remove unused variable.
> > >
> > > Fixes: 8288d030447f ("mm/gup: add make_dirty arg to
> > > put_user_pages_dirty_lock()")
> >
> > This commit hash and the commit subject does not exist in Linus'
> > tree as
> > of today. What tree is this being merged through, and is it slated
> > to
> > merge soon or is this a for-next item?
>
> This is though -mm, maybe John knows what is what

Hmmm...if it's through -mm, doesn't that mean that we can't rely on the
hash because the next time Andrew's tree rebases (using quilt or
whatever it is he does) that the hash will change? It doesn't really
matter too much...we can't take the fix anyway, it should probably be
squashed into the patch that it's fixing, and if you follow Bernard's
advice, you fix the problem by eliminating this function and changing
the sole call site to just call put_user_pages_dirty_lock() directly.

--
Doug Ledford <[email protected]>
GPG KeyID: B826A3330E572FDD
Fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD


Attachments:
signature.asc (849.00 B)
This is a digitally signed message part

2019-07-29 22:42:13

by Doug Ledford

[permalink] [raw]
Subject: Re: [PATCH] rdma: siw: remove unused variable

On Fri, 2019-07-26 at 11:25 +0200, Anders Roxell wrote:
> The variable 'p' si no longer used and the compiler rightly complains
> that it should be removed.
>
> ../drivers/infiniband/sw/siw/siw_mem.c: In function ‘siw_free_plist’:
> ../drivers/infiniband/sw/siw/siw_mem.c:66:16: warning: unused variable
> ‘p’ [-Wunused-variable]
> struct page **p = chunk->plist;
> ^
>
> Rework to remove unused variable.
>
> Fixes: 8288d030447f ("mm/gup: add make_dirty arg to
> put_user_pages_dirty_lock()")

This commit hash and the commit subject does not exist in Linus' tree as
of today. What tree is this being merged through, and is it slated to
merge soon or is this a for-next item?

--
Doug Ledford <[email protected]>
GPG KeyID: B826A3330E572FDD
Fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD


Attachments:
signature.asc (849.00 B)
This is a digitally signed message part

2019-07-30 07:16:18

by John Hubbard

[permalink] [raw]
Subject: Re: [PATCH] rdma: siw: remove unused variable

On 7/29/19 12:45 PM, Doug Ledford wrote:
> On Mon, 2019-07-29 at 16:03 -0300, Jason Gunthorpe wrote:
>> On Mon, Jul 29, 2019 at 02:19:35PM -0400, Doug Ledford wrote:
>>> On Fri, 2019-07-26 at 11:25 +0200, Anders Roxell wrote:
>>>> The variable 'p' si no longer used and the compiler rightly
>>>> complains
>>>> that it should be removed.
>>>>
>>>> ../drivers/infiniband/sw/siw/siw_mem.c: In function
>>>> ‘siw_free_plist’:
>>>> ../drivers/infiniband/sw/siw/siw_mem.c:66:16: warning: unused
>>>> variable
>>>> ‘p’ [-Wunused-variable]
>>>> struct page **p = chunk->plist;
>>>> ^
>>>>
>>>> Rework to remove unused variable.
>>>>
>>>> Fixes: 8288d030447f ("mm/gup: add make_dirty arg to
>>>> put_user_pages_dirty_lock()")
>>>
>>> This commit hash and the commit subject does not exist in Linus'
>>> tree as
>>> of today. What tree is this being merged through, and is it slated
>>> to
>>> merge soon or is this a for-next item?
>>
>> This is though -mm, maybe John knows what is what
>
> Hmmm...if it's through -mm, doesn't that mean that we can't rely on the
> hash because the next time Andrew's tree rebases (using quilt or
> whatever it is he does) that the hash will change? It doesn't really
> matter too much...we can't take the fix anyway, it should probably be
> squashed into the patch that it's fixing, and if you follow Bernard's
> advice, you fix the problem by eliminating this function and changing
> the sole call site to just call put_user_pages_dirty_lock() directly.
>

Hi,

Although I don't know which tree has 8288d030447f, I did get a report
from linux-next last night with that report about the warning, and so
I believe that the patch flowed from Andrew's -mm tree (which has
speculatively added my patches), to linux-next

(+CC Andrew)

I also sent out a fix for it, as a reply-to the warning report:

https://lore.kernel.org/r/[email protected]

Pasting in my response (minus the trivial fix), to save you a click:

"This fixes the warning. Ideally this should be merged with the commit
that it fixes, if that's still possible.

"Andrew, would you also like a fixed version of this patch posted
as a new version of the 3-patch set that it came with?"

thanks,
--
John Hubbard
NVIDIA