On Wed, 2011-01-05 at 17:26 +0000, Russell King - ARM Linux wrote: > On Wed, Jan 05, 2011 at 12:17:27PM -0500, Trond Myklebust wrote: > > We should already be flushing the kernel direct mapping after writing by > > means of the calls to flush_dcache_page() in xdr_partial_copy_from_skb() > > and all the helpers in net/sunrpc/xdr.c. > > Hmm, we're getting into the realms of what flush_dcache_page() is supposed > to do and what it's not supposed to do. > > Is this page an associated with a mapping (iow, page_mapping(page) is non- > NULL)? If not, flush_dcache_page() won't do anything, and from my > understanding, its flush_anon_page() which you want to be using there > instead. Actually, none of these pages are ever mapped into userspace, nor are they mapped into the page cache. They are allocated directly using alloc_page() by the thread that called the readdir() syscall, so afaics there should be no incoherent mappings other than the kernel direct mapping and the one created by vm_map_ram(). So, yes, you are right that we don't need the flush_dcache_page() here. > > The only new thing is the read access through the virtual address > > mapping. That mapping is created outside the loop in > > nfs_readdir_xdr_to_array(), which is why I'm thinking we do need the > > invalidate_kernel_vmap_range(): we're essentially doing a series of > > writes through the kernel direct mapping (i.e. readdir RPC calls), then > > reading the results through the virtual mapping. > > > > i.e. we're doing > > > > ptr = vm_map_ram(lowmem_pages); > > while (need_more_data) { > > > > for (i = 0; i < npages; i++) { > > addr = kmap_atomic(lowmem_page[i]); > > *addr = rpc_stuff; > > flush_dcache_page(lowmem_page[i]); > > kunmap_atomic(lowmem_page[i]); > > } > > > > invalidate_kernel_vmap_range(ptr); // Needed here? > > Yes, you're going to need some cache maintainence in there to make it work, > because accessing 'ptr' will load that data into the cache, and that won't > be updated by the writes via kmap_atomic(). > > Provided you don't write to ptr, then using invalidate_kernel_vmap_range() > will be safe. Thanks! That is what Marc's testing appears to confirm. -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@xxxxxxxxxx www.netapp.com -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html