Re: [rfc][patch] mm: direct io less aggressive syncs and invalidates

[Nick Piggin <npiggin@suse.de>]

On Wed, Oct 29, 2008 at 09:12:24AM -0400, Jeff Moyer wrote:
> Nick Piggin <npiggin@suse.de> writes:
> 
> > On Tue, Oct 28, 2008 at 05:11:02PM -0400, Jeff Moyer wrote:
> >> Nick Piggin <npiggin@suse.de> writes:
> 
> >> > Index: linux-2.6/mm/filemap.c
> >> > ===================================================================
> >> > --- linux-2.6.orig/mm/filemap.c	2008-10-03 11:21:31.000000000 +1000
> >> > +++ linux-2.6/mm/filemap.c	2008-10-03 12:00:17.000000000 +1000
> >> > @@ -1304,11 +1304,8 @@ generic_file_aio_read(struct kiocb *iocb
> >> >  			goto out; /* skip atime */
> >> >  		size = i_size_read(inode);
> >> >  		if (pos < size) {
> >> > -			retval = filemap_write_and_wait(mapping);
> >> > -			if (!retval) {
> >> > -				retval = mapping->a_ops->direct_IO(READ, iocb,
> >> > +			retval = mapping->a_ops->direct_IO(READ, iocb,
> >> >  							iov, pos, nr_segs);
> >> > -			}
> >> 
> >> So why is it safe to get rid of this?  Can't this result in reading
> >> stale data from disk?
> >
> > AFAIKS, __blockdev_direct_IO is doing the same thing for us, when it
> > encounters a READ. I should have documented this change. This is one
> > thing I'm not *quite* sure of there  might be a path do the block device
> > that I haven't considered, and which does not do the sync...
> 
> Well, that's if dio_lock_type != DIO_NO_LOCKING.  cscope shows the
> following callers of blockdev_direct_IO_no_locking:
> 
> gfs2_direct_IO
> ocfs2_direct_IO
> xfs_vm_direct_IO
> 
> and of course
> 
> blkdev_direct_IO
> 
> I can't say whether all of these callers are safe.  They certainly don't
> appear to be safe to me.

Ah OK of course you're right. I'll need to take another look at that
and probably send any improvement as another patch.

My test SMP system just started getting memory errors for some reason
so I haven't been able to boot it :( Will try to resurrect it or find
another before resending...