Message ID | 20190726092540.22467-1-anders.roxell@linaro.org |
---|---|
State | New |
Headers | show |
Series | rdma: siw: remove unused variable | expand |
-----"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>
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
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
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
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
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); }
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(-) -- 2.20.1