* [PATCH] mm/hugepages: fix "orig_pud" set but not used
@ 2019-03-01 0:49 Qian Cai
2019-03-01 21:09 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: Qian Cai @ 2019-03-01 0:49 UTC (permalink / raw)
To: akpm; +Cc: willy, linux-mm, linux-kernel, Qian Cai
The commit a00cc7d9dd93 ("mm, x86: add support for PUD-sized transparent
hugepages") introduced pudp_huge_get_and_clear_full() but no one uses
its return code, so just make it void.
mm/huge_memory.c: In function 'zap_huge_pud':
mm/huge_memory.c:1982:8: warning: variable 'orig_pud' set but not used
[-Wunused-but-set-variable]
pud_t orig_pud;
^~~~~~~~
Signed-off-by: Qian Cai <cai@lca.pw>
---
include/asm-generic/pgtable.h | 8 ++++----
mm/huge_memory.c | 4 +---
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index 05e61e6c843f..17b789557afe 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -167,11 +167,11 @@ static inline pmd_t pmdp_huge_get_and_clear_full(struct mm_struct *mm,
#endif
#ifndef __HAVE_ARCH_PUDP_HUGE_GET_AND_CLEAR_FULL
-static inline pud_t pudp_huge_get_and_clear_full(struct mm_struct *mm,
- unsigned long address, pud_t *pudp,
- int full)
+static inline void pudp_huge_get_and_clear_full(struct mm_struct *mm,
+ unsigned long address,
+ pud_t *pudp, int full)
{
- return pudp_huge_get_and_clear(mm, address, pudp);
+ pudp_huge_get_and_clear(mm, address, pudp);
}
#endif
#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index faf357eaf0ce..9f57a1173e6a 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1979,7 +1979,6 @@ spinlock_t *__pud_trans_huge_lock(pud_t *pud, struct vm_area_struct *vma)
int zap_huge_pud(struct mmu_gather *tlb, struct vm_area_struct *vma,
pud_t *pud, unsigned long addr)
{
- pud_t orig_pud;
spinlock_t *ptl;
ptl = __pud_trans_huge_lock(pud, vma);
@@ -1991,8 +1990,7 @@ int zap_huge_pud(struct mmu_gather *tlb, struct vm_area_struct *vma,
* pgtable_trans_huge_withdraw after finishing pudp related
* operations.
*/
- orig_pud = pudp_huge_get_and_clear_full(tlb->mm, addr, pud,
- tlb->fullmm);
+ pudp_huge_get_and_clear_full(tlb->mm, addr, pud, tlb->fullmm);
tlb_remove_pud_tlb_entry(tlb, pud, addr);
if (vma_is_dax(vma)) {
spin_unlock(ptl);
--
2.17.2 (Apple Git-113)
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mm/hugepages: fix "orig_pud" set but not used
2019-03-01 0:49 [PATCH] mm/hugepages: fix "orig_pud" set but not used Qian Cai
@ 2019-03-01 21:09 ` Andrew Morton
2019-03-01 21:44 ` Matthew Wilcox
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2019-03-01 21:09 UTC (permalink / raw)
To: Qian Cai; +Cc: willy, linux-mm, linux-kernel
On Thu, 28 Feb 2019 19:49:03 -0500 Qian Cai <cai@lca.pw> wrote:
> The commit a00cc7d9dd93 ("mm, x86: add support for PUD-sized transparent
> hugepages") introduced pudp_huge_get_and_clear_full() but no one uses
> its return code, so just make it void.
>
> mm/huge_memory.c: In function 'zap_huge_pud':
> mm/huge_memory.c:1982:8: warning: variable 'orig_pud' set but not used
> [-Wunused-but-set-variable]
> pud_t orig_pud;
> ^~~~~~~~
>
> ...
>
> --- a/include/asm-generic/pgtable.h
> +++ b/include/asm-generic/pgtable.h
> @@ -167,11 +167,11 @@ static inline pmd_t pmdp_huge_get_and_clear_full(struct mm_struct *mm,
> #endif
>
> #ifndef __HAVE_ARCH_PUDP_HUGE_GET_AND_CLEAR_FULL
> -static inline pud_t pudp_huge_get_and_clear_full(struct mm_struct *mm,
> - unsigned long address, pud_t *pudp,
> - int full)
> +static inline void pudp_huge_get_and_clear_full(struct mm_struct *mm,
> + unsigned long address,
> + pud_t *pudp, int full)
> {
> - return pudp_huge_get_and_clear(mm, address, pudp);
> + pudp_huge_get_and_clear(mm, address, pudp);
> }
Not sure this is a good change. Future callers might want that return
value.
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1979,7 +1979,6 @@ spinlock_t *__pud_trans_huge_lock(pud_t *pud, struct vm_area_struct *vma)
> int zap_huge_pud(struct mmu_gather *tlb, struct vm_area_struct *vma,
> pud_t *pud, unsigned long addr)
> {
> - pud_t orig_pud;
> spinlock_t *ptl;
>
> ptl = __pud_trans_huge_lock(pud, vma);
> @@ -1991,8 +1990,7 @@ int zap_huge_pud(struct mmu_gather *tlb, struct vm_area_struct *vma,
> * pgtable_trans_huge_withdraw after finishing pudp related
> * operations.
> */
> - orig_pud = pudp_huge_get_and_clear_full(tlb->mm, addr, pud,
> - tlb->fullmm);
> + pudp_huge_get_and_clear_full(tlb->mm, addr, pud, tlb->fullmm);
In fact this code perhaps should be passing orig_pud into
pudp_huge_get_and_clear_full(). That could depend on what future
per-arch implementations of pudp_huge_get_and_clear_full() choose to
do.
Anyway, I'll await Matthew's feedback.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mm/hugepages: fix "orig_pud" set but not used
2019-03-01 21:09 ` Andrew Morton
@ 2019-03-01 21:44 ` Matthew Wilcox
0 siblings, 0 replies; 3+ messages in thread
From: Matthew Wilcox @ 2019-03-01 21:44 UTC (permalink / raw)
To: Andrew Morton; +Cc: Qian Cai, linux-mm, linux-kernel
On Fri, Mar 01, 2019 at 01:09:51PM -0800, Andrew Morton wrote:
> On Thu, 28 Feb 2019 19:49:03 -0500 Qian Cai <cai@lca.pw> wrote:
>
> > The commit a00cc7d9dd93 ("mm, x86: add support for PUD-sized transparent
> > hugepages") introduced pudp_huge_get_and_clear_full() but no one uses
> > its return code, so just make it void.
> >
> > mm/huge_memory.c: In function 'zap_huge_pud':
> > mm/huge_memory.c:1982:8: warning: variable 'orig_pud' set but not used
> > [-Wunused-but-set-variable]
> > pud_t orig_pud;
> > ^~~~~~~~
> >
> > ...
> >
> > --- a/include/asm-generic/pgtable.h
> > +++ b/include/asm-generic/pgtable.h
> > @@ -167,11 +167,11 @@ static inline pmd_t pmdp_huge_get_and_clear_full(struct mm_struct *mm,
> > #endif
> >
> > #ifndef __HAVE_ARCH_PUDP_HUGE_GET_AND_CLEAR_FULL
> > -static inline pud_t pudp_huge_get_and_clear_full(struct mm_struct *mm,
> > - unsigned long address, pud_t *pudp,
> > - int full)
> > +static inline void pudp_huge_get_and_clear_full(struct mm_struct *mm,
> > + unsigned long address,
> > + pud_t *pudp, int full)
> > {
> > - return pudp_huge_get_and_clear(mm, address, pudp);
> > + pudp_huge_get_and_clear(mm, address, pudp);
> > }
>
> Not sure this is a good change. Future callers might want that return
> value.
>
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -1979,7 +1979,6 @@ spinlock_t *__pud_trans_huge_lock(pud_t *pud, struct vm_area_struct *vma)
> > int zap_huge_pud(struct mmu_gather *tlb, struct vm_area_struct *vma,
> > pud_t *pud, unsigned long addr)
> > {
> > - pud_t orig_pud;
> > spinlock_t *ptl;
> >
> > ptl = __pud_trans_huge_lock(pud, vma);
> > @@ -1991,8 +1990,7 @@ int zap_huge_pud(struct mmu_gather *tlb, struct vm_area_struct *vma,
> > * pgtable_trans_huge_withdraw after finishing pudp related
> > * operations.
> > */
> > - orig_pud = pudp_huge_get_and_clear_full(tlb->mm, addr, pud,
> > - tlb->fullmm);
> > + pudp_huge_get_and_clear_full(tlb->mm, addr, pud, tlb->fullmm);
>
> In fact this code perhaps should be passing orig_pud into
> pudp_huge_get_and_clear_full(). That could depend on what future
> per-arch implementations of pudp_huge_get_and_clear_full() choose to
> do.
>
> Anyway, I'll await Matthew's feedback.
I'm not sure it's wise to diverge from pmdp_huge_get_and_clear_full()
which does return the orig_pud. I agree we don't currently use it,
so maybe we should just change zap_huge_pud() to not assign the return
value from pudp_huge_get_and_clear_full()?
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-01 21:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-01 0:49 [PATCH] mm/hugepages: fix "orig_pud" set but not used Qian Cai
2019-03-01 21:09 ` Andrew Morton
2019-03-01 21:44 ` Matthew Wilcox
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).