[PATCH] rdma: siw: remove unused variable

[PATCH] rdma: siw: remove unused variable

[Anders Roxell]

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 <anders.roxell@linaro.org>
---
 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

Re: [PATCH] rdma: siw: remove unused variable

[Bernard Metzler]

-----"Anders Roxell" <anders.roxell@linaro.org> wrote: -----

>To: bmt@zurich.ibm.com, dledford@redhat.com, jgg@ziepe.ca
>From: "Anders Roxell" <anders.roxell@linaro.org>
>Date: 07/26/2019 11:26AM
>Cc: linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org, "Anders
>Roxell" <anders.roxell@linaro.org>
>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 <anders.roxell@linaro.org>
>---
> 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 <bmt@zurich.ibm.com>

Re: [PATCH] rdma: siw: remove unused variable

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 872 bytes --]

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 <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] rdma: siw: remove unused variable

[Jason Gunthorpe]

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

Re: [PATCH] rdma: siw: remove unused variable

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1652 bytes --]

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 <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] rdma: siw: remove unused variable

[John Hubbard]

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/20190729074306.10368-1-jhubbard@nvidia.com

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