From: Pavel Shilovsky <piastryyy@gmail.com>
To: Ronnie Sahlberg <lsahlber@redhat.com>
Cc: linux-cifs <linux-cifs@vger.kernel.org>,
Steve French <smfrench@gmail.com>
Subject: Re: [PATCH] cifs: flush before set-info if we have writeable handles
Date: Wed, 17 Jul 2019 11:44:04 -0700 [thread overview]
Message-ID: <CAKywueTNf7eTbbmcT=uO8JNrqZC2ZSLDFugw6TOwXWSJm_5zGw@mail.gmail.com> (raw)
In-Reply-To: <20190716032349.10711-1-lsahlber@redhat.com>
пн, 15 июл. 2019 г. в 20:24, Ronnie Sahlberg <lsahlber@redhat.com>:
>
> Servers can defer destaging any data and updating the mtime until close().
> This means that if we do a setinfo to modify the mtime while other handles
> are open for write the server may overwrite our setinfo timestamps when
> if flushes the file on close() of the writeable handle.
>
> To avoid this we add an explicit flush() before any attempts to use setinfo
> and update the mtime IF we have writeable handles open.
>
> This makes "cp -p" preserve the mtime when copying files to some smb servers.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
> fs/cifs/smb2inode.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
> index 278405d26c47..a3603ec3a086 100644
> --- a/fs/cifs/smb2inode.c
> +++ b/fs/cifs/smb2inode.c
> @@ -516,7 +516,11 @@ smb2_set_file_info(struct inode *inode, const char *full_path,
> {
> struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
> struct tcon_link *tlink;
> + struct cifs_tcon *tcon;
> + struct TCP_Server_Info *server;
> int rc;
> + struct cifsInodeInfo *cifsi;
> + struct cifsFileInfo *wrcfile;
>
> if ((buf->CreationTime == 0) && (buf->LastAccessTime == 0) &&
> (buf->LastWriteTime == 0) && (buf->ChangeTime == 0) &&
> @@ -527,7 +531,19 @@ smb2_set_file_info(struct inode *inode, const char *full_path,
> if (IS_ERR(tlink))
> return PTR_ERR(tlink);
>
> - rc = smb2_compound_op(xid, tlink_tcon(tlink), cifs_sb, full_path,
> + tcon = tlink_tcon(tlink);
> + server = tcon->ses->server;
> +
> + if (buf->LastWriteTime) {
> + cifsi = CIFS_I(inode);
> + wrcfile = find_writable_file(cifsi, false);
use get_writable_file instead - this will allow us to get a real RC.
if -EBADF is returned then no writable handle and should proceed
normally, otherwise let's fail setattr.
> + if (wrcfile) {
> + filemap_write_and_wait(inode->i_mapping);
If the above function returns error, the same problem that you are
trying to fix appears. If an error happen, we should fail setattr to
avoid corruption of timestamps.
> + server->ops->flush(xid, tcon, &wrcfile->fid);
the same logic applies to flush call - need to fail setattr if an error occured.
> + cifsFileInfo_put(wrcfile);
> + }
> + }
> + rc = smb2_compound_op(xid, tcon, cifs_sb, full_path,
> FILE_WRITE_ATTRIBUTES, FILE_OPEN, 0, buf,
> SMB2_OP_SET_INFO);
> cifs_put_tlink(tlink);
> --
> 2.13.6
>
--
Best regards,
Pavel Shilovsky
next prev parent reply other threads:[~2019-07-17 18:44 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-16 3:23 [PATCH] cifs: flush before set-info if we have writeable handles Ronnie Sahlberg
2019-07-17 18:44 ` Pavel Shilovsky [this message]
-- strict thread matches above, loose matches on Subject: below --
2019-07-18 22:12 Ronnie Sahlberg
2019-07-16 3:16 Ronnie Sahlberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKywueTNf7eTbbmcT=uO8JNrqZC2ZSLDFugw6TOwXWSJm_5zGw@mail.gmail.com' \
--to=piastryyy@gmail.com \
--cc=linux-cifs@vger.kernel.org \
--cc=lsahlber@redhat.com \
--cc=smfrench@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).