Messages in this thread | | | From | David Howells <> | Subject | Re: [PATCH 14/14] FRV: Better mmap support in uClinux | Date | Tue, 02 Nov 2004 16:43:25 +0000 |
| |
> > 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/
| |