On Wed, 22 Jan 2014 00:24:14 -0800 Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > On Tue, Jan 21, 2014 at 02:21:59PM -0500, Jeff Layton wrote: > > In any case, this helps but it's a little odd. With this patch, you add > > an invalidate_inode_pages2 call prior to doing the DIO. But, you've > > also left in the call to nfs_zap_mapping in the completion codepath. > > > > So now, we shoot down the mapping prior to doing a DIO write, and then > > mark the mapping for invalidation again when the write completes. Was > > that intentional? > > > > It seems a little excessive and might hurt performance in some cases. > > OTOH, if you mix buffered and DIO you're asking for trouble anyway and > > this approach seems to give better cache coherency. > > Thile follows the model implemented and documented in > generic_file_direct_write(). > Ok, thanks. That makes sense, and the problem described in those comments is almost exactly the one I've seen in practice. I'm still not 100% thrilled with the way that the NFS_INO_INVALID_DATA flag is handled, but that really has nothing to do with this patchset. You can add my Tested-by to the set if you like... Cheers, -- Jeff Layton <jlayton@xxxxxxxxxx> -- 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