cifs: flush data on any setattr
We already flush all the dirty pages for an inode before doing ATTR_SIZE and ATTR_MTIME changes. There's another problem though -- if we change the mode so that the file becomes read-only then we may not be able to write data to it after a reconnect. Fix this by just going back to flushing all the dirty data on any setattr call. There are probably some cases that can be optimized out, but I'm not sure they're worthwhile and we need to consider them more carefully to make sure that we don't cause regressions if we have to reconnect before writeback occurs. Signed-off-by: Jeff Layton <jlayton@redhat.com> Signed-off-by: Steve French <sfrench@us.ibm.com>
This commit is contained in:
parent
20d9207849
commit
0f4d634c59
|
@ -1792,20 +1792,21 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
if ((attrs->ia_valid & ATTR_MTIME) || (attrs->ia_valid & ATTR_SIZE)) {
|
/*
|
||||||
/*
|
* Attempt to flush data before changing attributes. We need to do
|
||||||
Flush data before changing file size or changing the last
|
* this for ATTR_SIZE and ATTR_MTIME for sure, and if we change the
|
||||||
write time of the file on the server. If the
|
* ownership or mode then we may also need to do this. Here, we take
|
||||||
flush returns error, store it to report later and continue.
|
* the safe way out and just do the flush on all setattr requests. If
|
||||||
BB: This should be smarter. Why bother flushing pages that
|
* the flush returns error, store it to report later and continue.
|
||||||
will be truncated anyway? Also, should we error out here if
|
*
|
||||||
the flush returns error?
|
* BB: This should be smarter. Why bother flushing pages that
|
||||||
*/
|
* will be truncated anyway? Also, should we error out here if
|
||||||
rc = filemap_write_and_wait(inode->i_mapping);
|
* the flush returns error?
|
||||||
if (rc != 0) {
|
*/
|
||||||
cifsInode->write_behind_rc = rc;
|
rc = filemap_write_and_wait(inode->i_mapping);
|
||||||
rc = 0;
|
if (rc != 0) {
|
||||||
}
|
cifsInode->write_behind_rc = rc;
|
||||||
|
rc = 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (attrs->ia_valid & ATTR_SIZE) {
|
if (attrs->ia_valid & ATTR_SIZE) {
|
||||||
|
@ -1903,20 +1904,21 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
}
|
}
|
||||||
|
|
||||||
if ((attrs->ia_valid & ATTR_MTIME) || (attrs->ia_valid & ATTR_SIZE)) {
|
/*
|
||||||
/*
|
* Attempt to flush data before changing attributes. We need to do
|
||||||
Flush data before changing file size or changing the last
|
* this for ATTR_SIZE and ATTR_MTIME for sure, and if we change the
|
||||||
write time of the file on the server. If the
|
* ownership or mode then we may also need to do this. Here, we take
|
||||||
flush returns error, store it to report later and continue.
|
* the safe way out and just do the flush on all setattr requests. If
|
||||||
BB: This should be smarter. Why bother flushing pages that
|
* the flush returns error, store it to report later and continue.
|
||||||
will be truncated anyway? Also, should we error out here if
|
*
|
||||||
the flush returns error?
|
* BB: This should be smarter. Why bother flushing pages that
|
||||||
*/
|
* will be truncated anyway? Also, should we error out here if
|
||||||
rc = filemap_write_and_wait(inode->i_mapping);
|
* the flush returns error?
|
||||||
if (rc != 0) {
|
*/
|
||||||
cifsInode->write_behind_rc = rc;
|
rc = filemap_write_and_wait(inode->i_mapping);
|
||||||
rc = 0;
|
if (rc != 0) {
|
||||||
}
|
cifsInode->write_behind_rc = rc;
|
||||||
|
rc = 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (attrs->ia_valid & ATTR_SIZE) {
|
if (attrs->ia_valid & ATTR_SIZE) {
|
||||||
|
|
Loading…
Reference in New Issue