On Thu, Dec 16, 2010 at 07:06:29AM -0500, Christoph Hellwig wrote: > > +STATIC ssize_t > > +xfs_file_dio_aio_write( > > + struct kiocb *iocb, > > + const struct iovec *iovp, > > + unsigned long nr_segs, > > + loff_t pos, > > + size_t ocount, > > + int *need_i_mutex, > > + int *iolock) > > need_i_mutex == 1 is equivalent to iolock == XFS_IOLOCK_EXCL, I think > it's a good idea to throw in a patch to remove the need_i_mutex variable > before this patch. > > Maybe that patch can even add xfs_rw_lock or similar helpers to > manipulate the i_mutex, the iolock, and the iolock variable together? That sounds like a fine idea. > Speaking of that, shouldn't xfs_file_aio_read also take the iolock > exclusive during the page invalidation and then demote it, just like > the write case? The above helpers would enforce that nicely. Probably, though it might be best to leave that to another cleanup series. I'll see how much perturbation of the read path it makes.... > > + if ((pos & target->bt_smask) || (count & target->bt_smask)) > > + return XFS_ERROR(-EINVAL); > > For the error traps to work this needs to be > > -XFS_ERROR(EINVAL); Ok, will fix. I just copied the existing code. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs