2010/11/24 Paul Mundt <lethal@xxxxxxxxxxxx>: > On Sat, Nov 20, 2010 at 10:58:40AM +0100, Marco Stornelli wrote: >> diff -Nurp linux-2.6.36-orig/fs/pramfs/file.c linux-2.6.36/fs/pramfs/file.c >> --- linux-2.6.36-orig/fs/pramfs/file.c 1970-01-01 01:00:00.000000000 +0100 >> +++ linux-2.6.36/fs/pramfs/file.c 2010-09-24 18:34:03.000000000 +0200 >> @@ -0,0 +1,166 @@ >> +/* >> + * FILE NAME fs/pramfs/file.c >> + * >> + * BRIEF DESCRIPTION >> + * >> + * File operations for files. >> + * > This FILE NAME / BRIEF DESCRIPTION thing should probably die, it's also > not used consistently throughout the series. Maybe yes, it's more a problem then a advantage :) > >> +static int pram_open_file(struct inode *inode, struct file *filp) >> +{ >> +#ifndef CONFIG_PRAMFS_XIP >> + /* Without XIP we force to use Direct IO */ >> + filp->f_flags |= O_DIRECT; >> +#endif >> + return generic_file_open(inode, filp); >> +} >> + > Relying on a config option for this is in violently poor taste. Rely on > the config option to enable/disable XIP support as you like, but whether > it's actually mounted XIP or not should really depend be determined by a > mount option. Presently you have no way of dealing with the file system > being mounted multiple times with mixed XIP and non-XIP, which doesn't > seem like a particularly exotic configuration. As you seem to have copied Maybe on embedded, it could be. > most of this from ext2, I'm curious why you opted to hardcode this > instead of maintaining the flexibility that ext2 XIP has over this. > First of all because it was simpler :) In addition there was some design problem to use it in combination with the memory protection. The difference with ext2 is that we aren't talking about a general purpose fs used (mainly) on normal desktop/server systems, but a specific fs for embedded world, so I think some little constraints are ok. Marco -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html