On Thu, Nov 15, 2012 at 11:28:07AM -0500, Christoph Hellwig wrote:
> > - if ((iocb->ki_pos & target->bt_smask) ||
> > - (size & target->bt_smask)) {
> > - if (iocb->ki_pos == i_size_read(inode))
> > + if ((pos & target->bt_smask) || (size & target->bt_smask)) {
> > + if (pos == i_size_read(inode))
> > return 0;
> > return -XFS_ERROR(EINVAL);
> > }
> > }
> >
> > - n = mp->m_super->s_maxbytes - iocb->ki_pos;
> > + n = mp->m_super->s_maxbytes - pos;
>
> What does this have to do with the recent of the patch?
Left over from an original version of the patch that also changed
the ranges of the flushes.
> Not that is diapprove, but I don't think it fits here.
>
> > if (inode->i_mapping->nrpages) {
> > - ret = -xfs_flushinval_pages(ip,
> > - (iocb->ki_pos & PAGE_CACHE_MASK),
> > - -1, FI_REMAPF_LOCKED);
> > + ret = -filemap_write_and_wait_range(
> > + VFS_I(ip)->i_mapping,
> > + pos, -1);
> > if (ret) {
> > xfs_rw_iunlock(ip, XFS_IOLOCK_EXCL);
> > return ret;
> > }
> > + truncate_pagecache_range(VFS_I(ip), pos, -1);
>
> We already have a local "inode" variable that can be used in these two
> places.
Ah, copy-n-waste problem.
> Also the -1 end might be a 1:1 translation of what was there, but is not what
> we really want. At very least it needs an XXX comment that the range should
> be revisited.
Yes, I know, but the original patch I had that changed the ranges to
something sensible was causing fsx and other failures all over the
place. It appears that setting the ranges appropriately here exposes
other (worse) bugs, so I decided to leave doing that until I have
time to go on a wild goose chase....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
