Messages in this thread | | | From | David Howells <> | Subject | Re: [PATCH 7/7] FS-Cache: CacheFiles: A cache that backs onto a mounted filesystem | Date | Fri, 21 Apr 2006 15:49:57 +0100 |
| |
Andrew Morton <akpm@osdl.org> 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-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/
| |