On Fri, 2022-01-07 at 21:52 +0000, David Howells wrote:
> This patch isn't quite right and needs a fix. The attached patch fixes it.
> I'll fold that in and post a v5 of this patch.
>
> David
> ---
> cifs: Fix the fscache cookie management
>
> Fix the fscache cookie management in cifs in the following ways:
>
> (1) The cookie should be released in cifs_evict_inode() after it has been
> unused from being pinned by cifs_set_page_dirty().
>
> (2) The cookie shouldn't be released in cifsFileInfo_put_final(). That's
> dealing with closing a file, not getting rid of an inode. The cookie
> needs to persist beyond the last file close because writepages may
> happen after closure.
>
> (3) The cookie needs to be used in cifs_atomic_open() to match
> cifs_open(). As yet unknown files being opened for writing seem to go
> by the former route rather than the latter.
>
> This can be triggered by something like:
>
> # systemctl start cachefilesd
> # mount //carina/test /xfstest.test -o user=shares,pass=xxxx.fsc
> # bash 5</xfstest.test/bar
> # echo hello >/xfstest.test/bar
>
> The key is to open the file R/O and then open it R/W and write to it whilst
> it's still open for R/O. A cookie isn't acquired if it's just opened R/W
> as it goes through the atomic_open method rather than the open method.
>
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> ---
> fs/cifs/cifsfs.c | 8 ++++++++
> fs/cifs/dir.c | 4 ++++
> fs/cifs/file.c | 2 --
> 3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index d3f3acf340f1..26cf2193c9a2 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -398,6 +398,7 @@ cifs_evict_inode(struct inode *inode)
> truncate_inode_pages_final(&inode->i_data);
> if (inode->i_state & I_PINNING_FSCACHE_WB)
> cifs_fscache_unuse_inode_cookie(inode, true);
> + cifs_fscache_release_inode_cookie(inode);
> clear_inode(inode);
> }
>
> @@ -722,6 +723,12 @@ static int cifs_show_stats(struct seq_file *s, struct dentry *root)
> }
> #endif
>
> +static int cifs_write_inode(struct inode *inode, struct writeback_control *wbc)
> +{
> + fscache_unpin_writeback(wbc, cifs_inode_cookie(inode));
> + return 0;
> +}
> +
> static int cifs_drop_inode(struct inode *inode)
> {
> struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
> @@ -734,6 +741,7 @@ static int cifs_drop_inode(struct inode *inode)
> static const struct super_operations cifs_super_ops = {
> .statfs = cifs_statfs,
> .alloc_inode = cifs_alloc_inode,
> + .write_inode = cifs_write_inode,
> .free_inode = cifs_free_inode,
> .drop_inode = cifs_drop_inode,
> .evict_inode = cifs_evict_inode,
> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> index 6e8e7cc26ae2..6186824b366e 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -22,6 +22,7 @@
> #include "cifs_unicode.h"
> #include "fs_context.h"
> #include "cifs_ioctl.h"
> +#include "fscache.h"
>
> static void
> renew_parental_timestamps(struct dentry *direntry)
> @@ -509,6 +510,9 @@ cifs_atomic_open(struct inode *inode, struct dentry *direntry,
> rc = -ENOMEM;
> }
>
> + fscache_use_cookie(cifs_inode_cookie(file_inode(file)),
> + file->f_mode & FMODE_WRITE);
> +
> out:
> cifs_put_tlink(tlink);
> out_free_xid:
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index d872f6fe8e7d..44da7646f789 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -376,8 +376,6 @@ static void cifsFileInfo_put_final(struct cifsFileInfo *cifs_file)
> struct cifsLockInfo *li, *tmp;
> struct super_block *sb = inode->i_sb;
>
> - cifs_fscache_release_inode_cookie(inode);
> -
> /*
> * Delete any outstanding lock records. We'll lose them when the file
> * is closed anyway.
>
Looks good.
Acked-by: Jeff Layton <jlayton@xxxxxxxxxx>
