netfs, cifs: Fix mtime/ctime update for mmapped writes

Commit Message

David Howells Sept. 17, 2024, 7:54 a.m. UTC

The cifs flag CIFS_INO_MODIFIED_ATTR, which indicates that the mtime and
ctime need to be written back on close, got taken over by netfs as
NETFS_ICTX_MODIFIED_ATTR to avoid the need to call a function pointer to
set it.

The flag gets set correctly on buffered writes, but doesn't get set by
netfs_page_mkwrite(), leading to occasional failures in generic/080 and
generic/215.

Fix this by setting the flag in netfs_page_mkwrite().

Fixes: 73425800ac94 ("netfs, cifs: Move CIFS_INO_MODIFIED_ATTR to netfs_inode")
Reported-by: kernel test robot <[e-mail]>
Closes: https://lore.kernel.org/oe-lkp/[e-mail]
Signed-off-by: David Howells <[e-mail]>
cc: Jeff Layton <[e-mail]>
cc: Steve French <[e-mail]>
cc: Paulo Alcantara <[e-mail]>
cc: [e-mail]
cc: [e-mail]
cc: [e-mail]
---
 fs/netfs/buffered_write.c |    1 +
 1 file changed, 1 insertion(+)

Comments

Christian Brauner Sept. 17, 2024, 8:59 a.m. UTC | #1

On Tue, 17 Sep 2024 08:54:28 +0100, David Howells wrote:
> The cifs flag CIFS_INO_MODIFIED_ATTR, which indicates that the mtime and
> ctime need to be written back on close, got taken over by netfs as
> NETFS_ICTX_MODIFIED_ATTR to avoid the need to call a function pointer to
> set it.
> 
> The flag gets set correctly on buffered writes, but doesn't get set by
> netfs_page_mkwrite(), leading to occasional failures in generic/080 and
> generic/215.
> 
> [...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] netfs, cifs: Fix mtime/ctime update for mmapped writes
      https://git.kernel.org/vfs/vfs/c/edd297c2764c

Paulo Alcantara Sept. 17, 2024, 5:55 p.m. UTC | #2

David Howells <[e-mail]> writes:

> The cifs flag CIFS_INO_MODIFIED_ATTR, which indicates that the mtime and
> ctime need to be written back on close, got taken over by netfs as
> NETFS_ICTX_MODIFIED_ATTR to avoid the need to call a function pointer to
> set it.
>
> The flag gets set correctly on buffered writes, but doesn't get set by
> netfs_page_mkwrite(), leading to occasional failures in generic/080 and
> generic/215.
>
> Fix this by setting the flag in netfs_page_mkwrite().
>
> Fixes: 73425800ac94 ("netfs, cifs: Move CIFS_INO_MODIFIED_ATTR to netfs_inode")
> Reported-by: kernel test robot <[e-mail]>
> Closes: https://lore.kernel.org/oe-lkp/[e-mail]
> Signed-off-by: David Howells <[e-mail]>
> cc: Jeff Layton <[e-mail]>
> cc: Steve French <[e-mail]>
> cc: Paulo Alcantara <[e-mail]>
> cc: [e-mail]
> cc: [e-mail]
> cc: [e-mail]
> ---
>  fs/netfs/buffered_write.c |    1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Paulo Alcantara (Red Hat) <[e-mail]>

Steve French Sept. 17, 2024, 9:24 p.m. UTC | #3

You can also add "Tested-by: Steve French <[e-mail]>"

On Tue, Sep 17, 2024 at 1:01 PM Paulo Alcantara <[e-mail]> wrote:
>
> David Howells <[e-mail]> writes:
>
> > The cifs flag CIFS_INO_MODIFIED_ATTR, which indicates that the mtime and
> > ctime need to be written back on close, got taken over by netfs as
> > NETFS_ICTX_MODIFIED_ATTR to avoid the need to call a function pointer to
> > set it.
> >
> > The flag gets set correctly on buffered writes, but doesn't get set by
> > netfs_page_mkwrite(), leading to occasional failures in generic/080 and
> > generic/215.
> >
> > Fix this by setting the flag in netfs_page_mkwrite().
> >
> > Fixes: 73425800ac94 ("netfs, cifs: Move CIFS_INO_MODIFIED_ATTR to netfs_inode")
> > Reported-by: kernel test robot <[e-mail]>
> > Closes: https://lore.kernel.org/oe-lkp/[e-mail]
> > Signed-off-by: David Howells <[e-mail]>
> > cc: Jeff Layton <[e-mail]>
> > cc: Steve French <[e-mail]>
> > cc: Paulo Alcantara <[e-mail]>
> > cc: [e-mail]
> > cc: [e-mail]
> > cc: [e-mail]
> > ---
> >  fs/netfs/buffered_write.c |    1 +
> >  1 file changed, 1 insertion(+)
>
> Reviewed-by: Paulo Alcantara (Red Hat) <[e-mail]>
>

Patch

diff --git a/fs/netfs/buffered_write.c b/fs/netfs/buffered_write.c
index d7eae597e54d..b3910dfcb56d 100644
--- a/fs/netfs/buffered_write.c
+++ b/fs/netfs/buffered_write.c
@@ -552,6 +552,7 @@  vm_fault_t netfs_page_mkwrite(struct vm_fault *vmf, struct netfs_group *netfs_gr
 		trace_netfs_folio(folio, netfs_folio_trace_mkwrite);
 	netfs_set_group(folio, netfs_group);
 	file_update_time(file);
+	set_bit(NETFS_ICTX_MODIFIED_ATTR, &ictx->flags);
 	if (ictx->ops->post_modify)
 		ictx->ops->post_modify(inode);
 	ret = VM_FAULT_LOCKED;