Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > - there's a bunch of gfp_t-related sparse warnings. I'll send a patch > to fix them as a followup to this mail. Added. > - the Kconfig hackery is rather ugly. I'd rather express the depencies > with selects in this case - the filesystem fscache options select > FSCACHE, as does the cachefiles module. Patch will be sent later. I don't much like that either; some people want to build FS-Cache in whilst retaining AFS and NFS as modules, and you've removed that option. I also think "select" should go away as it causes problems, though that's a weakness of the config programs. > - include/linux/fscache.h is an utter mess :) > Please reorder the file so there's just a single big > #if defined(CONFIG_FSCACHE) || defined(CONFIG_FSCACHE_MODULE) > #else > #endif > block. Doing that will also allow to drop a lot of the __fscache > function and directly stub out the non-__ versions. Sigh. Another #ifdef-hater. > - please remove the #ifdef CONFIG_DEBUG_SLAB BUG_ONs from cookie.c, > driver should never deal with slab or slab debugging internals. They're useful debugging checks, but okay. > - various statically initialize structs in main.c set members to > NULL, which is not required and should be removed Ok. > - IIRC k* release callbacks must not be NULL. Please check which > Greg on your kset usage Eh? The only one I've got is not NULL. It points to fscache_ktype_release(). > - please adjust your comment style to normal linux style. kernel-doc > headers for functions please, kernel-doc is broken. It encourages people to be lazy about writing proper documentation. I have provided proper documentation. > - maybe fscache should provide a generic ->mkwrite operation or > even a set of otherwise filemap-based vm_operations for it's client > filesystems? Possibly. David - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
