On Sat, 26 Mar 2011 11:14:45 +0300
Pavel Shilovsky <piastry@xxxxxxxxxxx> wrote:
> Base of approach for splitting all calls that uses cifs_invalidate_mapping
> (or cifs_revalidate_dentry, cifs_revalidate_file) into two groups:
> 1) aware about -EBUSY error code and report it back (cifs_d_revalidate,
> cifs_strict_fsync, cifs_file_strict_mmap);
> 2) don't do it (cifs_getattrs, cifs_lseek).
>
> Signed-off-by: Pavel Shilovsky <piastry@xxxxxxxxxxx>
> ---
> fs/cifs/cifsfs.c | 8 +++++++-
> fs/cifs/cifsfs.h | 2 +-
> fs/cifs/file.c | 18 ++++++++++++++----
> fs/cifs/inode.c | 23 ++++++++++++++++-------
> 4 files changed, 38 insertions(+), 13 deletions(-)
>
-----------------[snip]---------------------
> diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
> index bb64313..f4391ff 100644
> --- a/fs/cifs/cifsfs.h
> +++ b/fs/cifs/cifsfs.h
> @@ -61,7 +61,7 @@ extern int cifs_rename(struct inode *, struct dentry *, struct inode *,
> struct dentry *);
> extern int cifs_revalidate_file(struct file *filp);
> extern int cifs_revalidate_dentry(struct dentry *);
> -extern void cifs_invalidate_mapping(struct inode *inode);
> +extern int cifs_invalidate_mapping(struct inode *inode);
> extern int cifs_getattr(struct vfsmount *, struct dentry *, struct kstat *);
> extern int cifs_setattr(struct dentry *, struct iattr *);
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index b9731c9..d99cf48 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1448,8 +1448,13 @@ int cifs_strict_fsync(struct file *file, int datasync)
> cFYI(1, "Sync file - name: %s datasync: 0x%x",
> file->f_path.dentry->d_name.name, datasync);
>
> - if (!CIFS_I(inode)->clientCanCacheRead)
> - cifs_invalidate_mapping(inode);
> + if (!CIFS_I(inode)->clientCanCacheRead) {
> + rc = cifs_invalidate_mapping(inode);
> + if (rc) {
> + FreeXid(xid);
> + return rc;
> + }
> + }
>
Now that I've had some more time to think on this patch, I don't think
this is what we want at all. The problem in a nutshell is that an -EBUSY
return makes no sense here.
Suppose I run fsync() on a file, and everything is written out
successfully, but some page is redirtied in the meantime and can't be
invalidated. This then returns -EBUSY, but why should the task doing the
fsync care that the pages can't be invalidated? It shouldn't, AFAICT.
The thread getting the -EBUSY ought to be the one that next attempts to
access the page.
--
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
