2015-07-02 07:26:31

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH] mm:Make the function zap_huge_pmd bool

On Wed 01-07-15 14:27:57, Nicholas Krause wrote:
> This makes the function zap_huge_pmd have a return type of bool
> now due to this particular function always returning one or zero
> as its return value.

How does this help anything? IMO this just generates a pointless churn
in the code without a good reason.

> Signed-off-by: Nicholas Krause <[email protected]>
> ---
> include/linux/huge_mm.h | 2 +-
> mm/huge_memory.c | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index f10b20f..e83174e 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -19,7 +19,7 @@ extern struct page *follow_trans_huge_pmd(struct vm_area_struct *vma,
> unsigned long addr,
> pmd_t *pmd,
> unsigned int flags);
> -extern int zap_huge_pmd(struct mmu_gather *tlb,
> +extern bool zap_huge_pmd(struct mmu_gather *tlb,
> struct vm_area_struct *vma,
> pmd_t *pmd, unsigned long addr);
> extern int mincore_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index c107094..32b1993 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1384,11 +1384,11 @@ out:
> return 0;
> }
>
> -int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> +bool zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> pmd_t *pmd, unsigned long addr)
> {
> spinlock_t *ptl;
> - int ret = 0;
> + int ret = false;
>
> if (__pmd_trans_huge_lock(pmd, vma, &ptl) == 1) {
> struct page *page;
> @@ -1419,7 +1419,7 @@ int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> tlb_remove_page(tlb, page);
> }
> pte_free(tlb->mm, pgtable);
> - ret = 1;
> + ret = true;
> }
> return ret;
> }
> --
> 2.1.4
>

--
Michal Hocko
SUSE Labs


2015-07-02 16:04:44

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] mm:Make the function zap_huge_pmd bool

On Thu, Jul 02, 2015 at 09:26:21AM +0200, Michal Hocko wrote:
> On Wed 01-07-15 14:27:57, Nicholas Krause wrote:
> > This makes the function zap_huge_pmd have a return type of bool
> > now due to this particular function always returning one or zero
> > as its return value.
>
> How does this help anything? IMO this just generates a pointless churn
> in the code without a good reason.

Hi Michal,

My recommendation is to ignore patches sent by Nick. In my experience
he doesn't understand code before trying to make mechanical changes,
and very few of his patches add any new value, and at least one that
he tried to send me just 2 weeks or so ago (cherry-picked to try to
"prove" why he had turned over a new leaf, so that I would support the
removal of his e-mail address from being blacklisted on
vger.kernel.org) was buggy, and when I asked him some basic questions
about what the code was doing, it was clear he had no clue how the
seq_file abstraction worked. This didn't stop him from trying to
patch the code, and if he had tested it, it would have crashed and
burned instantly.

Of course, do whatevery you want, but IMHO it's not really not worth
your time to deal with his patches, and if you reply, most people
won't see his original e-mail since the vger.kernel.org blacklist is
still in effect.

Regards,

- Ted