Re: [PATCH 2/3] CIFS: New read cache mechanism

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 27 Sep 2010 22:31:54 +0400
Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:

> Add cifs_sync_read call to provide reading from the cache if we have at least
> Level II oplock and otherwise - reading from the server.
> 
> Signed-off-by: Pavel Shilovsky <piastryyy@xxxxxxxxx>
> ---
>  fs/cifs/cifsfs.c |   26 ++++++++++++++++++++++++--
>  1 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 898d2a5..a7361ee 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -549,6 +549,28 @@ cifs_get_sb(struct file_system_type *fs_type,
>  	return 0;
>  }
> 
> +static ssize_t cifs_sync_read(struct file *filp, char __user *buf,
> +			      size_t len, loff_t *ppos)
> +{
> +	ssize_t read;
> +	struct cifsInodeInfo *cinode;
> +	struct cifs_sb_info *cifs_sb;
> +
> +	cifs_sb = CIFS_SB(filp->f_path.dentry->d_inode->i_sb);
> +
> +	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO))
> +		return do_sync_read(filp, buf, len, ppos);
> +
> +	cinode = CIFS_I(filp->f_path.dentry->d_inode);
> +
> +	if (cinode->clientCanCacheRead)
> +		read = do_sync_read(filp, buf, len, ppos);
> +	else
> +		read = cifs_user_read(filp, buf, len, ppos);
> +
> +	return read;
> +}
> +
>  static ssize_t cifs_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
>  				   unsigned long nr_segs, loff_t pos)
>  {
> @@ -668,7 +690,7 @@ const struct inode_operations cifs_symlink_inode_ops = {
>  };
> 
>  const struct file_operations cifs_file_ops = {
> -	.read = do_sync_read,
> +	.read = cifs_sync_read,
>  	.write = do_sync_write,
>  	.aio_read = generic_file_aio_read,
>  	.aio_write = cifs_file_aio_write,
> @@ -705,7 +727,7 @@ const struct file_operations cifs_file_direct_ops = {
>  	.setlease = cifs_setlease,
>  };
>  const struct file_operations cifs_file_nobrl_ops = {
> -	.read = do_sync_read,
> +	.read = cifs_sync_read,
>  	.write = do_sync_write,
>  	.aio_read = generic_file_aio_read,
>  	.aio_write = cifs_file_aio_write,

Ok, so suppose you make this change and don't change
cifs_file_aio_write at all, and at some point in the future, we make
cifs do async writes...

I go and do a buffered write to a file, cifs_file_aio_write then calls
filemap_fdatawrite to flush out the data, but doesn't necessarily wait
for it to complete before returning.

Now, I do I read from that file descriptor, which ends up being
satisfied before the write I just did got flushed out. I see old
data in the read.

According to the write(2) manpage:

POSIX  requires  that  a  read(2)  which can be proved to occur after a
write() has returned returns the new data.  Note that not all file sysâ
tems are POSIX conforming.

It's arguable that CIFS isn't POSIX conforming, but we do our best with
it. I think we ought to conform to POSIX in this regard. If you want
to do this, you probably also need to ensure that the dirty data gets
flushed to the server prior to returning from a write call.

-- 
Jeff Layton <jlayton@xxxxxxxxx>
--
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


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux