lkml.org 
[lkml]   [2004]   [Nov]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 14/14] FRV: Better mmap support in uClinux
Date

> > diff -uNr /warthog/kernels/linux-2.6.10-rc1-bk10/fs/proc/base.c linux-2.6.10-rc1-bk10-frv/fs/proc/base.c
> ...
>
> please split this up into a mmu and a no-mmu function, in a mmu and nommu
> file respectively.

What would you say to this function being moved into the mm/ directory?
Perhaps in mm/proc.c.

> > +/* list of shareable VMAs */
> > +LIST_HEAD(shareable_vma_list);
> > +DECLARE_RWSEM(shareable_vma_sem);
>
> the should both be static

I'm intending to do a /proc file to allow this list to be viewed.

> > + if (flags & MAP_FIXED || addr) {
> > +// printk("can't do fixed-address/overlay mmap of RAM\n");
>
> please avoid C++-style comments in core kernel code.

I should make this #ifdef'd on a CONFIG_DEBUG_ option.

> > +#ifdef CONFIG_MMU
> > vma->vm_ops = &generic_file_vm_ops;
> > +#endif
>
> please find a nice way to abstract this out without the ifdefs here.

Why? The #ifdef tells you exactly what you need to know. This _is_ the right
way to do it. Doing otherwise just makes the code less obvious. However, if
you can come up with a better way, I'll review the patch for you.

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

\
 
 \ /
  Last update: 2005-03-22 14:07    [W:0.030 / U:2.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site