Andrew Morton <akpm@xxxxxxxx> wrote: > > +unsigned long cachefiles_debug = 0; > > Unneeded initialisation. Yep. > > +static int cachefiles_init(void) > > __init? Yep. > removeable? Yep. > hm, what's going on here? It's strange for a callee to undo an i_mutex > which some caller took. It happens occasionally. The problem here is that I want to call this from three different places, but if I drop the mutex before calling the burial function, I have to get the mutex again to do the unlink; but as it is, I have to drop it before I can do the rename:-/ It's not nice, but... You have to note also that the directory's i_mutex is quite important for interacting with the daemon also. The wonders of working through an existing filesystem, and the the wonders of co-operating with userspace. > > +int > > +generic_file_buffered_write_one_kernel_page(struct file *file, > > + pgoff_t index, > > + struct page *src) > > Some covering comments would be nice. I copied those of generic_file_buffered_write() and rearranged them a bit:-) I'll add a comment to my function. > If the hosts's i_mutex is held (it should be, but there are no comments) > then we can read inode->i_size directly. Minor thing. Ah. Do we though? I just copied generic_file_buffered_write() and cut it down. The same is done there. The comments at the top of that function weren't exactly forthcoming on the preconditions for calling that function. > that's copy_highpage(). Good point. > Sigh. It's all a huge pile of new code. And it's only used by AFS, the > number of users of which can be counted on the fingers of one foot. An NFS > implementation would make a testing phase much more useful. Yes... Whilst I have it working with NFS, the NFS anti-aliasing problems are still there and still need to be sorted. I thought I'd got them nailed, but then Trond changed his mind:-( But that does not preclude putting what I can release up for review. 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