Messages in this thread | | | Date | Sun, 21 Jun 2009 13:22:01 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH -tip] x86: mm/hugetlbpage.c fix style problems |
| |
* Jaswinder Singh Rajput <jaswinder@kernel.org> 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@gmail.com> > --- > 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
| |