On Wed, 10 Nov 2010 16:15:33 +0300
Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
> 2010/11/10 Jeff Layton <jlayton@xxxxxxxxxx>:
> >
> > Hmm...setting the dirty bit before the data has actually been copied to
> > the page? I'm not sure that's a good idea. If you want to do that you
> > may want to run it by linux-fsdevel first.
> >
> > Maybe you could do something instead with the fsdata pointer in
> > write_begin and write_end?
> >
>
> Jeff, what's about simply invalidating inode range if we don't have
> exlusive oplock after cifs_user_write? I don't think that there is a
> chance to write a data to the server and keep oplock level II in the
> same time - so no need for storing the data in the cache at all. And
> if we don't store it in the cache we don't need to modify
> cifs_write_end - no races.
>
> so, I mean the following:
>
> + if (CIFS_I(inode)->clientCanCacheAll)
> + return generic_file_aio_write(iocb, iov, nr_segs, pos);
> +
> + cifs_sb = CIFS_SB(iocb->ki_filp->f_path.dentry->d_sb);
> +
> + if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO) == 0) {
> + int rc;
> +
> + written = generic_file_aio_write(iocb, iov, nr_segs, pos);
> +
> + rc = filemap_fdatawrite(inode->i_mapping);
> + if (rc)
> + cFYI(1, "cifs_file_aio_write: %d rc on %p inode",
> + rc, inode);
> + return written;
> + }
> +
> + written = cifs_user_write(iocb->ki_filp, iov->iov_base,
> + iov->iov_len, &pos);
> +
> + if (written > 0)
> + invalidate_mapping_pages(inode->i_mapping,
> + (pos-written) >> PAGE_CACHE_SHIFT,
> + (pos-1) >> PAGE_CACHE_SHIFT);
> + iocb->ki_pos = pos;
>
That might work, and it gets around the ugly double write thing that
the earlier patches did.
You're probably right that most servers won't allow you to keep a
level II oplock and write to the file. If one does, then the pages will
have to be refaulted in, of course which will suck for performance, but
I'm not terribly concerned about that.
Also, please take some time and sprinkle some comments in there too. In
a year or two we'll be struggling to recall this conversation to figure
out why it was done this way...
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
