Re: [PATCH -tip] x86: mm/hugetlbpage.c fix style problems

From: Ingo Molnar
Date: Sun Jun 21 2009 - 07:22:28 EST



* Jaswinder Singh Rajput <jaswinder@xxxxxxxxxx> wrote:

>
> Impact : cleanup
>
> Fix :
>
> WARNING: Use #include <linux/mman.h> instead of <asm/mman.h>
> WARNING: line over 80 characters
> ERROR: code indent should use tabs where possible X 10
> ERROR: do not use assignment in if condition
> ERROR: return is not a function, parentheses are not required
>
> total: 12 errors, 2 warnings
>
> Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@xxxxxxxxx>
> ---
> arch/x86/mm/hugetlbpage.c | 33 ++++++++++++++++++---------------
> 1 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
> index f46c340..978fc5f 100644
> --- a/arch/x86/mm/hugetlbpage.c
> +++ b/arch/x86/mm/hugetlbpage.c
> @@ -12,7 +12,7 @@
> #include <linux/slab.h>
> #include <linux/err.h>
> #include <linux/sysctl.h>
> -#include <asm/mman.h>
> +#include <linux/mman.h>
> #include <asm/tlb.h>
> #include <asm/tlbflush.h>
> #include <asm/pgalloc.h>
> @@ -93,7 +93,8 @@ static void huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
>
> spin_lock(&mm->page_table_lock);
> if (pud_none(*pud))
> - pud_populate(mm, pud, (pmd_t *)((unsigned long)spte & PAGE_MASK));
> + pud_populate(mm, pud,
> + (pmd_t *)((unsigned long)spte & PAGE_MASK));

this change is not an improvement.

> else
> put_page(virt_to_page(spte));
> spin_unlock(&mm->page_table_lock);
> @@ -269,11 +270,11 @@ static unsigned long hugetlb_get_unmapped_area_bottomup(struct file *file,
> struct vm_area_struct *vma;
> unsigned long start_addr;
>
> - if (len > mm->cached_hole_size) {
> - start_addr = mm->free_area_cache;
> - } else {
> - start_addr = TASK_UNMAPPED_BASE;
> - mm->cached_hole_size = 0;
> + if (len > mm->cached_hole_size)
> + start_addr = mm->free_area_cache;
> + else {
> + start_addr = TASK_UNMAPPED_BASE;
> + mm->cached_hole_size = 0;
> }
>

this change is not an improvement either.

Plus at a first glance that file has a couple of other problems as
well beyond trivial style issue:

- dead code which should be removed

- a 64-bit only boot option that should probably be extended to
32-bit as well

- insanely long function names with local scope, causing ugly
linebreaks

and more style issues as well:

- various inconsistent capitalizations in comment blocks

- some other small details as well

Lets try to do a genuinely big and comprehensive forward step that
addresses all these issues (and any other issues that are in that
file), ok?

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/