On Fri, 15 Nov 2013 10:33:24 -0500 Jeff Layton <jlayton@xxxxxxxxxx> wrote: > On Fri, 15 Nov 2013 07:02:04 -0800 > Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > > > On Fri, Nov 15, 2013 at 09:52:41AM -0500, Jeff Layton wrote: > > > Do you have these patches in a git tree someplace? If so, I wouldn't > > > mind running this reproducer against it to see if it helps. It's a bit > > > of a longshot, but what the heck... > > > > While I do have a local git tree I don't really have anywhere to push > > it to. But applying these patches shouldn't be all that hard. > > > > It's not -- I'm just lazy... > > FWIW, I tried this set and it didn't make any difference on the bug, so > I'll just keep soldiering on to track it down... > I just tried this set again, and it *did* seem to help that bug. I think the reason it didn't before was that I had applied this set on top of a tree that held a different patch that introduced a race in the nfs_revalidate_mapping() code. 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. -- 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
