From bb413489288e4e457353bac513fddb6330d245ca Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 12 Jun 2020 00:15:13 +0100 Subject: [PATCH 01/12] afs: Fix non-setting of mtime when writing into mmap The mtime on an inode needs to be updated when a write is made into an mmap'ed section. There are three ways in which this could be done: update it when page_mkwrite is called, update it when a page is changed from dirty to writeback or leave it to the server and fix the mtime up from the reply to the StoreData RPC. Found with the generic/215 xfstest. Fixes: 1cf7a1518aef ("afs: Implement shared-writeable mmap") Signed-off-by: David Howells --- fs/afs/write.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/afs/write.c b/fs/afs/write.c index 768497f82aee..9270bb01be67 100644 --- a/fs/afs/write.c +++ b/fs/afs/write.c @@ -844,6 +844,7 @@ vm_fault_t afs_page_mkwrite(struct vm_fault *vmf) vmf->page->index, priv); SetPagePrivate(vmf->page); set_page_private(vmf->page, priv); + file_update_time(file); sb_end_pagefault(inode->i_sb); return VM_FAULT_LOCKED; From 1f32ef79897052ef7d3d154610d8d6af95abde83 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 12 Jun 2020 23:58:51 +0100 Subject: [PATCH 02/12] afs: afs_write_end() should change i_size under the right lock Fix afs_write_end() to change i_size under vnode->cb_lock rather than ->wb_lock so that it doesn't race with afs_vnode_commit_status() and afs_getattr(). The ->wb_lock is only meant to guard access to ->wb_keys which isn't accessed by that piece of code. Fixes: 4343d00872e1 ("afs: Get rid of the afs_writeback record") Signed-off-by: David Howells --- fs/afs/write.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/afs/write.c b/fs/afs/write.c index 9270bb01be67..a55cb73e0449 100644 --- a/fs/afs/write.c +++ b/fs/afs/write.c @@ -194,11 +194,11 @@ int afs_write_end(struct file *file, struct address_space *mapping, i_size = i_size_read(&vnode->vfs_inode); if (maybe_i_size > i_size) { - spin_lock(&vnode->wb_lock); + write_seqlock(&vnode->cb_lock); i_size = i_size_read(&vnode->vfs_inode); if (maybe_i_size > i_size) i_size_write(&vnode->vfs_inode, maybe_i_size); - spin_unlock(&vnode->wb_lock); + write_sequnlock(&vnode->cb_lock); } if (!PageUptodate(page)) { From 3f4aa981816368fe6b1d13c2bfbe76df9687e787 Mon Sep 17 00:00:00 2001 From: David Howells Date: Sat, 13 Jun 2020 00:03:48 +0100 Subject: [PATCH 03/12] afs: Fix EOF corruption When doing a partial writeback, afs_write_back_from_locked_page() may generate an FS.StoreData RPC request that writes out part of a file when a file has been constructed from pieces by doing seek, write, seek, write, ... as is done by ld. The FS.StoreData RPC is given the current i_size as the file length, but the server basically ignores it unless the data length is 0 (in which case it's just a truncate operation). The revised file length returned in the result of the RPC may then not reflect what we suggested - and this leads to i_size getting moved backwards - which causes issues later. Fix the client to take account of this by ignoring the returned file size unless the data version number jumped unexpectedly - in which case we're going to have to clear the pagecache and reload anyway. This can be observed when doing a kernel build on an AFS mount. The following pair of commands produce the issue: ld -m elf_x86_64 -z max-page-size=0x200000 --emit-relocs \ -T arch/x86/realmode/rm/realmode.lds \ arch/x86/realmode/rm/header.o \ arch/x86/realmode/rm/trampoline_64.o \ arch/x86/realmode/rm/stack.o \ arch/x86/realmode/rm/reboot.o \ -o arch/x86/realmode/rm/realmode.elf arch/x86/tools/relocs --realmode \ arch/x86/realmode/rm/realmode.elf \ >arch/x86/realmode/rm/realmode.relocs This results in the latter giving: Cannot read ELF section headers 0/18: Success as the realmode.elf file got corrupted. The sequence of events can also be driven with: xfs_io -t -f \ -c "pwrite -S 0x58 0 0x58" \ -c "pwrite -S 0x59 10000 1000" \ -c "close" \ /afs/example.com/scratch/a Fixes: 31143d5d515e ("AFS: implement basic file write support") Signed-off-by: David Howells --- fs/afs/inode.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/fs/afs/inode.c b/fs/afs/inode.c index cd0a0060950b..8d10bfb392d1 100644 --- a/fs/afs/inode.c +++ b/fs/afs/inode.c @@ -168,6 +168,7 @@ static void afs_apply_status(struct afs_operation *op, struct timespec64 t; umode_t mode; bool data_changed = false; + bool change_size = false; _enter("{%llx:%llu.%u} %s", vp->fid.vid, vp->fid.vnode, vp->fid.unique, @@ -226,6 +227,7 @@ static void afs_apply_status(struct afs_operation *op, } else { set_bit(AFS_VNODE_ZAP_DATA, &vnode->flags); } + change_size = true; } else if (vnode->status.type == AFS_FTYPE_DIR) { /* Expected directory change is handled elsewhere so * that we can locally edit the directory and save on a @@ -233,11 +235,19 @@ static void afs_apply_status(struct afs_operation *op, */ if (test_bit(AFS_VNODE_DIR_VALID, &vnode->flags)) data_changed = false; + change_size = true; } if (data_changed) { inode_set_iversion_raw(&vnode->vfs_inode, status->data_version); - afs_set_i_size(vnode, status->size); + + /* Only update the size if the data version jumped. If the + * file is being modified locally, then we might have our own + * idea of what the size should be that's not the same as + * what's on the server. + */ + if (change_size) + afs_set_i_size(vnode, status->size); } } From da8d07551275abb3a38fae2d16e02bc9cc7396b2 Mon Sep 17 00:00:00 2001 From: David Howells Date: Sat, 13 Jun 2020 19:34:59 +0100 Subject: [PATCH 04/12] afs: Concoct ctimes The in-kernel afs filesystem ignores ctime because the AFS fileserver protocol doesn't support ctimes. This, however, causes various xfstests to fail. Work around this by: (1) Setting ctime to attr->ia_ctime in afs_setattr(). (2) Not ignoring ATTR_MTIME_SET, ATTR_TIMES_SET and ATTR_TOUCH settings. (3) Setting the ctime from the server mtime when on the target file when creating a hard link to it. (4) Setting the ctime on directories from their revised mtimes when renaming/moving a file. Found by the generic/221 and generic/309 xfstests. Signed-off-by: David Howells --- fs/afs/dir.c | 18 +++++++++++++++++- fs/afs/inode.c | 29 ++++++++++++++++++----------- fs/afs/internal.h | 2 ++ fs/afs/write.c | 1 + 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/fs/afs/dir.c b/fs/afs/dir.c index aa1d34141ea3..308a125e9de3 100644 --- a/fs/afs/dir.c +++ b/fs/afs/dir.c @@ -1268,6 +1268,7 @@ static void afs_vnode_new_inode(struct afs_operation *op) static void afs_create_success(struct afs_operation *op) { _enter("op=%08x", op->debug_id); + op->ctime = op->file[0].scb.status.mtime_client; afs_check_for_remote_deletion(op, op->file[0].vnode); afs_vnode_commit_status(op, &op->file[0]); afs_update_dentry_version(op, &op->file[0], op->dentry); @@ -1325,6 +1326,7 @@ static int afs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) afs_op_set_vnode(op, 0, dvnode); op->file[0].dv_delta = 1; + op->file[0].update_ctime = true; op->dentry = dentry; op->create.mode = S_IFDIR | mode; op->create.reason = afs_edit_dir_for_mkdir; @@ -1350,6 +1352,7 @@ static void afs_dir_remove_subdir(struct dentry *dentry) static void afs_rmdir_success(struct afs_operation *op) { _enter("op=%08x", op->debug_id); + op->ctime = op->file[0].scb.status.mtime_client; afs_check_for_remote_deletion(op, op->file[0].vnode); afs_vnode_commit_status(op, &op->file[0]); afs_update_dentry_version(op, &op->file[0], op->dentry); @@ -1404,6 +1407,7 @@ static int afs_rmdir(struct inode *dir, struct dentry *dentry) afs_op_set_vnode(op, 0, dvnode); op->file[0].dv_delta = 1; + op->file[0].update_ctime = true; op->dentry = dentry; op->ops = &afs_rmdir_operation; @@ -1479,6 +1483,7 @@ static void afs_dir_remove_link(struct afs_operation *op) static void afs_unlink_success(struct afs_operation *op) { _enter("op=%08x", op->debug_id); + op->ctime = op->file[0].scb.status.mtime_client; afs_check_for_remote_deletion(op, op->file[0].vnode); afs_vnode_commit_status(op, &op->file[0]); afs_vnode_commit_status(op, &op->file[1]); @@ -1537,6 +1542,7 @@ static int afs_unlink(struct inode *dir, struct dentry *dentry) afs_op_set_vnode(op, 0, dvnode); op->file[0].dv_delta = 1; + op->file[0].update_ctime = true; /* Try to make sure we have a callback promise on the victim. */ ret = afs_validate(vnode, op->key); @@ -1561,6 +1567,7 @@ static int afs_unlink(struct inode *dir, struct dentry *dentry) spin_unlock(&dentry->d_lock); op->file[1].vnode = vnode; + op->file[1].update_ctime = true; op->dentry = dentry; op->ops = &afs_unlink_operation; return afs_do_sync_operation(op); @@ -1601,6 +1608,7 @@ static int afs_create(struct inode *dir, struct dentry *dentry, umode_t mode, afs_op_set_vnode(op, 0, dvnode); op->file[0].dv_delta = 1; + op->file[0].update_ctime = true; op->dentry = dentry; op->create.mode = S_IFREG | mode; @@ -1620,6 +1628,7 @@ static void afs_link_success(struct afs_operation *op) struct afs_vnode_param *vp = &op->file[1]; _enter("op=%08x", op->debug_id); + op->ctime = dvp->scb.status.mtime_client; afs_vnode_commit_status(op, dvp); afs_vnode_commit_status(op, vp); afs_update_dentry_version(op, dvp, op->dentry); @@ -1672,6 +1681,8 @@ static int afs_link(struct dentry *from, struct inode *dir, afs_op_set_vnode(op, 0, dvnode); afs_op_set_vnode(op, 1, vnode); op->file[0].dv_delta = 1; + op->file[0].update_ctime = true; + op->file[1].update_ctime = true; op->dentry = dentry; op->dentry_2 = from; @@ -1740,9 +1751,12 @@ static void afs_rename_success(struct afs_operation *op) { _enter("op=%08x", op->debug_id); + op->ctime = op->file[0].scb.status.mtime_client; afs_vnode_commit_status(op, &op->file[0]); - if (op->file[1].vnode != op->file[0].vnode) + if (op->file[1].vnode != op->file[0].vnode) { + op->ctime = op->file[1].scb.status.mtime_client; afs_vnode_commit_status(op, &op->file[1]); + } } static void afs_rename_edit_dir(struct afs_operation *op) @@ -1860,6 +1874,8 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry, afs_op_set_vnode(op, 1, new_dvnode); /* May be same as orig_dvnode */ op->file[0].dv_delta = 1; op->file[1].dv_delta = 1; + op->file[0].update_ctime = true; + op->file[1].update_ctime = true; op->dentry = old_dentry; op->dentry_2 = new_dentry; diff --git a/fs/afs/inode.c b/fs/afs/inode.c index 8d10bfb392d1..e99705474dd1 100644 --- a/fs/afs/inode.c +++ b/fs/afs/inode.c @@ -165,6 +165,7 @@ static void afs_apply_status(struct afs_operation *op, { struct afs_file_status *status = &vp->scb.status; struct afs_vnode *vnode = vp->vnode; + struct inode *inode = &vnode->vfs_inode; struct timespec64 t; umode_t mode; bool data_changed = false; @@ -187,25 +188,25 @@ static void afs_apply_status(struct afs_operation *op, } if (status->nlink != vnode->status.nlink) - set_nlink(&vnode->vfs_inode, status->nlink); + set_nlink(inode, status->nlink); if (status->owner != vnode->status.owner) - vnode->vfs_inode.i_uid = make_kuid(&init_user_ns, status->owner); + inode->i_uid = make_kuid(&init_user_ns, status->owner); if (status->group != vnode->status.group) - vnode->vfs_inode.i_gid = make_kgid(&init_user_ns, status->group); + inode->i_gid = make_kgid(&init_user_ns, status->group); if (status->mode != vnode->status.mode) { - mode = vnode->vfs_inode.i_mode; + mode = inode->i_mode; mode &= ~S_IALLUGO; mode |= status->mode; - WRITE_ONCE(vnode->vfs_inode.i_mode, mode); + WRITE_ONCE(inode->i_mode, mode); } t = status->mtime_client; - vnode->vfs_inode.i_ctime = t; - vnode->vfs_inode.i_mtime = t; - vnode->vfs_inode.i_atime = t; + inode->i_mtime = t; + if (vp->update_ctime) + inode->i_ctime = op->ctime; if (vnode->status.data_version != status->data_version) data_changed = true; @@ -239,15 +240,18 @@ static void afs_apply_status(struct afs_operation *op, } if (data_changed) { - inode_set_iversion_raw(&vnode->vfs_inode, status->data_version); + inode_set_iversion_raw(inode, status->data_version); /* Only update the size if the data version jumped. If the * file is being modified locally, then we might have our own * idea of what the size should be that's not the same as * what's on the server. */ - if (change_size) + if (change_size) { afs_set_i_size(vnode, status->size); + inode->i_ctime = t; + inode->i_atime = t; + } } } @@ -817,7 +821,8 @@ int afs_setattr(struct dentry *dentry, struct iattr *attr) attr->ia_valid); if (!(attr->ia_valid & (ATTR_SIZE | ATTR_MODE | ATTR_UID | ATTR_GID | - ATTR_MTIME))) { + ATTR_MTIME | ATTR_MTIME_SET | ATTR_TIMES_SET | + ATTR_TOUCH))) { _leave(" = 0 [unsupported]"); return 0; } @@ -837,6 +842,8 @@ int afs_setattr(struct dentry *dentry, struct iattr *attr) if (attr->ia_valid & ATTR_SIZE) op->file[0].dv_delta = 1; + op->ctime = attr->ia_ctime; + op->file[0].update_ctime = 1; op->ops = &afs_setattr_operation; return afs_do_sync_operation(op); diff --git a/fs/afs/internal.h b/fs/afs/internal.h index 0c9806ef2a19..92cd6b8cc01f 100644 --- a/fs/afs/internal.h +++ b/fs/afs/internal.h @@ -746,6 +746,7 @@ struct afs_vnode_param { u8 dv_delta; /* Expected change in data version */ bool put_vnode; /* T if we have a ref on the vnode */ bool need_io_lock; /* T if we need the I/O lock on this */ + bool update_ctime; /* Need to update the ctime */ }; /* @@ -766,6 +767,7 @@ struct afs_operation { struct dentry *dentry; /* Dentry to be altered */ struct dentry *dentry_2; /* Second dentry to be altered */ struct timespec64 mtime; /* Modification time to record */ + struct timespec64 ctime; /* Change time to set */ short nr_files; /* Number of entries in file[], more_files */ short error; unsigned int abort_code; diff --git a/fs/afs/write.c b/fs/afs/write.c index a55cb73e0449..2003d7ee9e43 100644 --- a/fs/afs/write.c +++ b/fs/afs/write.c @@ -393,6 +393,7 @@ static void afs_store_data_success(struct afs_operation *op) { struct afs_vnode *vnode = op->file[0].vnode; + op->ctime = op->file[0].scb.status.mtime_client; afs_vnode_commit_status(op, &op->file[0]); if (op->error == 0) { afs_pages_written_back(vnode, op->store.first, op->store.last); From 793fe82ee33aab1023cf023cd7d744af19a3dff9 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 12 Jun 2020 16:13:52 +0100 Subject: [PATCH 05/12] afs: Fix truncation issues and mmap writeback size Fix the following issues: (1) Fix writeback to reduce the size of a store operation to i_size, effectively discarding the extra data. The problem comes when afs_page_mkwrite() records that a page is about to be modified by mmap(). It doesn't know what bits of the page are going to be modified, so it records the whole page as being dirty (this is stored in page->private as start and end offsets). Without this, the marshalling for the store to the server extends the size of the file to the end of the page (in afs_fs_store_data() and yfs_fs_store_data()). (2) Fix setattr to actually truncate the pagecache, thereby clearing the discarded part of a file. (3) Fix setattr to check that the new size is okay and to disable ATTR_SIZE if i_size wouldn't change. (4) Force i_size to be updated as the result of a truncate. (5) Don't truncate if ATTR_SIZE is not set. (6) Call pagecache_isize_extended() if the file was enlarged. Note that truncate_set_size() isn't used because the setting of i_size is done inside afs_vnode_commit_status() under the vnode->cb_lock. Found with the generic/029 and generic/393 xfstests. Fixes: 31143d5d515e ("AFS: implement basic file write support") Fixes: 4343d00872e1 ("afs: Get rid of the afs_writeback record") Signed-off-by: David Howells --- fs/afs/inode.c | 27 +++++++++++++++++++++++++-- fs/afs/internal.h | 7 ++++--- fs/afs/write.c | 6 ++++++ 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/fs/afs/inode.c b/fs/afs/inode.c index e99705474dd1..70c925978d10 100644 --- a/fs/afs/inode.c +++ b/fs/afs/inode.c @@ -169,7 +169,7 @@ static void afs_apply_status(struct afs_operation *op, struct timespec64 t; umode_t mode; bool data_changed = false; - bool change_size = false; + bool change_size = vp->set_size; _enter("{%llx:%llu.%u} %s", vp->fid.vid, vp->fid.vnode, vp->fid.unique, @@ -799,7 +799,15 @@ void afs_evict_inode(struct inode *inode) static void afs_setattr_success(struct afs_operation *op) { + struct inode *inode = &op->file[0].vnode->vfs_inode; + afs_vnode_commit_status(op, &op->file[0]); + if (op->setattr.attr->ia_valid & ATTR_SIZE) { + loff_t i_size = inode->i_size, size = op->setattr.attr->ia_size; + if (size > i_size) + pagecache_isize_extended(inode, i_size, size); + truncate_pagecache(inode, size); + } } static const struct afs_operation_ops afs_setattr_operation = { @@ -815,6 +823,7 @@ int afs_setattr(struct dentry *dentry, struct iattr *attr) { struct afs_operation *op; struct afs_vnode *vnode = AFS_FS_I(d_inode(dentry)); + int ret; _enter("{%llx:%llu},{n=%pd},%x", vnode->fid.vid, vnode->fid.vnode, dentry, @@ -827,6 +836,18 @@ int afs_setattr(struct dentry *dentry, struct iattr *attr) return 0; } + if (attr->ia_valid & ATTR_SIZE) { + if (!S_ISREG(vnode->vfs_inode.i_mode)) + return -EISDIR; + + ret = inode_newsize_ok(&vnode->vfs_inode, attr->ia_size); + if (ret) + return ret; + + if (attr->ia_size == i_size_read(&vnode->vfs_inode)) + attr->ia_valid &= ~ATTR_SIZE; + } + /* flush any dirty data outstanding on a regular file */ if (S_ISREG(vnode->vfs_inode.i_mode)) filemap_write_and_wait(vnode->vfs_inode.i_mapping); @@ -840,8 +861,10 @@ int afs_setattr(struct dentry *dentry, struct iattr *attr) afs_op_set_vnode(op, 0, vnode); op->setattr.attr = attr; - if (attr->ia_valid & ATTR_SIZE) + if (attr->ia_valid & ATTR_SIZE) { op->file[0].dv_delta = 1; + op->file[0].set_size = true; + } op->ctime = attr->ia_ctime; op->file[0].update_ctime = 1; diff --git a/fs/afs/internal.h b/fs/afs/internal.h index 92cd6b8cc01f..bdc1e5efebd4 100644 --- a/fs/afs/internal.h +++ b/fs/afs/internal.h @@ -744,9 +744,10 @@ struct afs_vnode_param { afs_dataversion_t dv_before; /* Data version before the call */ unsigned int cb_break_before; /* cb_break + cb_s_break before the call */ u8 dv_delta; /* Expected change in data version */ - bool put_vnode; /* T if we have a ref on the vnode */ - bool need_io_lock; /* T if we need the I/O lock on this */ - bool update_ctime; /* Need to update the ctime */ + bool put_vnode:1; /* T if we have a ref on the vnode */ + bool need_io_lock:1; /* T if we need the I/O lock on this */ + bool update_ctime:1; /* Need to update the ctime */ + bool set_size:1; /* Must update i_size */ }; /* diff --git a/fs/afs/write.c b/fs/afs/write.c index 2003d7ee9e43..7437806332d9 100644 --- a/fs/afs/write.c +++ b/fs/afs/write.c @@ -492,6 +492,7 @@ static int afs_write_back_from_locked_page(struct address_space *mapping, unsigned long count, priv; unsigned n, offset, to, f, t; pgoff_t start, first, last; + loff_t i_size, end; int loop, ret; _enter(",%lx", primary_page->index); @@ -592,7 +593,12 @@ static int afs_write_back_from_locked_page(struct address_space *mapping, first = primary_page->index; last = first + count - 1; + end = (loff_t)last * PAGE_SIZE + to; + i_size = i_size_read(&vnode->vfs_inode); + _debug("write back %lx[%u..] to %lx[..%u]", first, offset, last, to); + if (end > i_size) + to = i_size & ~PAGE_MASK; ret = afs_store_data(mapping, first, last, offset, to); switch (ret) { From 4ec89596d06bd481ba827f3b409b938d63914157 Mon Sep 17 00:00:00 2001 From: David Howells Date: Sun, 14 Jun 2020 22:12:05 +0100 Subject: [PATCH 06/12] afs: Fix the mapping of the UAEOVERFLOW abort code Abort code UAEOVERFLOW is returned when we try and set a time that's out of range, but it's currently mapped to EREMOTEIO by the default case. Fix UAEOVERFLOW to map instead to EOVERFLOW. Found with the generic/258 xfstest. Note that the test is wrong as it assumes that the filesystem will support a pre-UNIX-epoch date. Fixes: 1eda8bab70ca ("afs: Add support for the UAE error table") Signed-off-by: David Howells --- fs/afs/misc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/afs/misc.c b/fs/afs/misc.c index 52b19e9c1535..5334f1bd2bca 100644 --- a/fs/afs/misc.c +++ b/fs/afs/misc.c @@ -83,6 +83,7 @@ int afs_abort_to_error(u32 abort_code) case UAENOLCK: return -ENOLCK; case UAENOTEMPTY: return -ENOTEMPTY; case UAELOOP: return -ELOOP; + case UAEOVERFLOW: return -EOVERFLOW; case UAENOMEDIUM: return -ENOMEDIUM; case UAEDQUOT: return -EDQUOT; From 6c85cacc8c096fc5cbdba61b6aa8fe675805e5d1 Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 16 Jun 2020 00:18:09 +0100 Subject: [PATCH 07/12] afs: Remove yfs_fs_fetch_file_status() as it's not used Remove yfs_fs_fetch_file_status() as it's no longer used. Signed-off-by: David Howells --- fs/afs/internal.h | 1 - fs/afs/yfsclient.c | 42 ------------------------------------------ 2 files changed, 43 deletions(-) diff --git a/fs/afs/internal.h b/fs/afs/internal.h index bdc1e5efebd4..d2207cb40740 100644 --- a/fs/afs/internal.h +++ b/fs/afs/internal.h @@ -1438,7 +1438,6 @@ extern ssize_t afs_listxattr(struct dentry *, char *, size_t); /* * yfsclient.c */ -extern void yfs_fs_fetch_file_status(struct afs_operation *); extern void yfs_fs_fetch_data(struct afs_operation *); extern void yfs_fs_create_file(struct afs_operation *); extern void yfs_fs_make_dir(struct afs_operation *); diff --git a/fs/afs/yfsclient.c b/fs/afs/yfsclient.c index 52d5af5fcd44..993591739240 100644 --- a/fs/afs/yfsclient.c +++ b/fs/afs/yfsclient.c @@ -374,48 +374,6 @@ static int yfs_deliver_status_and_volsync(struct afs_call *call) return 0; } -/* - * YFS.FetchStatus operation type - */ -static const struct afs_call_type yfs_RXYFSFetchStatus_vnode = { - .name = "YFS.FetchStatus(vnode)", - .op = yfs_FS_FetchStatus, - .deliver = yfs_deliver_fs_status_cb_and_volsync, - .destructor = afs_flat_call_destructor, -}; - -/* - * Fetch the status information for a file. - */ -void yfs_fs_fetch_file_status(struct afs_operation *op) -{ - struct afs_vnode_param *vp = &op->file[0]; - struct afs_call *call; - __be32 *bp; - - _enter(",%x,{%llx:%llu},,", - key_serial(op->key), vp->fid.vid, vp->fid.vnode); - - call = afs_alloc_flat_call(op->net, &yfs_RXYFSFetchStatus_vnode, - sizeof(__be32) * 2 + - sizeof(struct yfs_xdr_YFSFid), - sizeof(struct yfs_xdr_YFSFetchStatus) + - sizeof(struct yfs_xdr_YFSCallBack) + - sizeof(struct yfs_xdr_YFSVolSync)); - if (!call) - return afs_op_nomem(op); - - /* marshall the parameters */ - bp = call->request; - bp = xdr_encode_u32(bp, YFSFETCHSTATUS); - bp = xdr_encode_u32(bp, 0); /* RPC flags */ - bp = xdr_encode_YFSFid(bp, &vp->fid); - yfs_check_req(call, bp); - - trace_afs_make_fs_call(call, &vp->fid); - afs_make_op_call(op, call, GFP_NOFS); -} - /* * Deliver reply data to an YFS.FetchData64. */ From 9bd87ec631ba07285138eed9c85645a12294f6c6 Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 16 Jun 2020 00:23:12 +0100 Subject: [PATCH 08/12] afs: Fix yfs_fs_fetch_status() to honour vnode selector Fix yfs_fs_fetch_status() to honour the vnode selector in op->fetch_status.which as does afs_fs_fetch_status() that allows afs_do_lookup() to use this as an alternative to the InlineBulkStatus RPC call if not implemented by the server. This doesn't matter in the current code as YFS servers always implement InlineBulkStatus, but a subsequent will call it on YFS servers too in some circumstances. Fixes: e49c7b2f6de7 ("afs: Build an abstraction around an "operation" concept") Signed-off-by: David Howells --- fs/afs/yfsclient.c | 51 +++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/fs/afs/yfsclient.c b/fs/afs/yfsclient.c index 993591739240..8c24fdc899e3 100644 --- a/fs/afs/yfsclient.c +++ b/fs/afs/yfsclient.c @@ -329,29 +329,6 @@ static void xdr_decode_YFSFetchVolumeStatus(const __be32 **_bp, *_bp += sizeof(*x) / sizeof(__be32); } -/* - * Deliver a reply that's a status, callback and volsync. - */ -static int yfs_deliver_fs_status_cb_and_volsync(struct afs_call *call) -{ - struct afs_operation *op = call->op; - const __be32 *bp; - int ret; - - ret = afs_transfer_reply(call); - if (ret < 0) - return ret; - - /* unmarshall the reply once we've received all of it */ - bp = call->buffer; - xdr_decode_YFSFetchStatus(&bp, call, &op->file[0].scb); - xdr_decode_YFSCallBack(&bp, call, &op->file[0].scb); - xdr_decode_YFSVolSync(&bp, &op->volsync); - - _leave(" = 0 [done]"); - return 0; -} - /* * Deliver reply data to operations that just return a file status and a volume * sync record. @@ -1562,13 +1539,37 @@ void yfs_fs_release_lock(struct afs_operation *op) afs_make_op_call(op, call, GFP_NOFS); } +/* + * Deliver a reply to YFS.FetchStatus + */ +static int yfs_deliver_fs_fetch_status(struct afs_call *call) +{ + struct afs_operation *op = call->op; + struct afs_vnode_param *vp = &op->file[op->fetch_status.which]; + const __be32 *bp; + int ret; + + ret = afs_transfer_reply(call); + if (ret < 0) + return ret; + + /* unmarshall the reply once we've received all of it */ + bp = call->buffer; + xdr_decode_YFSFetchStatus(&bp, call, &vp->scb); + xdr_decode_YFSCallBack(&bp, call, &vp->scb); + xdr_decode_YFSVolSync(&bp, &op->volsync); + + _leave(" = 0 [done]"); + return 0; +} + /* * YFS.FetchStatus operation type */ static const struct afs_call_type yfs_RXYFSFetchStatus = { .name = "YFS.FetchStatus", .op = yfs_FS_FetchStatus, - .deliver = yfs_deliver_fs_status_cb_and_volsync, + .deliver = yfs_deliver_fs_fetch_status, .destructor = afs_flat_call_destructor, }; @@ -1577,7 +1578,7 @@ static const struct afs_call_type yfs_RXYFSFetchStatus = { */ void yfs_fs_fetch_status(struct afs_operation *op) { - struct afs_vnode_param *vp = &op->file[0]; + struct afs_vnode_param *vp = &op->file[op->fetch_status.which]; struct afs_call *call; __be32 *bp; From 44767c353127cfcbee49a89bab39a3680ecd2a45 Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 16 Jun 2020 00:25:56 +0100 Subject: [PATCH 09/12] afs: Remove afs_operation::abort_code Remove afs_operation::abort_code as it's read but never set. Use ac.abort_code instead. Signed-off-by: David Howells --- fs/afs/dir.c | 2 +- fs/afs/internal.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/afs/dir.c b/fs/afs/dir.c index 308a125e9de3..ca6b147963a9 100644 --- a/fs/afs/dir.c +++ b/fs/afs/dir.c @@ -648,7 +648,7 @@ static void afs_do_lookup_success(struct afs_operation *op) vp = &op->file[0]; abort_code = vp->scb.status.abort_code; if (abort_code != 0) { - op->abort_code = abort_code; + op->ac.abort_code = abort_code; op->error = afs_abort_to_error(abort_code); } break; diff --git a/fs/afs/internal.h b/fs/afs/internal.h index d2207cb40740..598934d923cc 100644 --- a/fs/afs/internal.h +++ b/fs/afs/internal.h @@ -771,7 +771,6 @@ struct afs_operation { struct timespec64 ctime; /* Change time to set */ short nr_files; /* Number of entries in file[], more_files */ short error; - unsigned int abort_code; unsigned int debug_id; unsigned int cb_v_break; /* Volume break counter before op */ From 728279a5a1fd9fa9fa268f807391c4d19ad2822c Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 16 Jun 2020 00:34:09 +0100 Subject: [PATCH 10/12] afs: Fix use of afs_check_for_remote_deletion() afs_check_for_remote_deletion() checks to see if error ENOENT is returned by the server in response to an operation and, if so, marks the primary vnode as having been deleted as the FID is no longer valid. However, it's being called from the operation success functions, where no abort has happened - and if an inline abort is recorded, it's handled by afs_vnode_commit_status(). Fix this by actually calling the operation aborted method if provided and having that point to afs_check_for_remote_deletion(). Fixes: e49c7b2f6de7 ("afs: Build an abstraction around an "operation" concept") Signed-off-by: David Howells --- fs/afs/dir.c | 21 ++++++++++++++++++--- fs/afs/dir_silly.c | 2 +- fs/afs/file.c | 2 +- fs/afs/flock.c | 4 +--- fs/afs/fs_operation.c | 10 +++++++++- fs/afs/inode.c | 1 + fs/afs/internal.h | 10 +--------- 7 files changed, 32 insertions(+), 18 deletions(-) diff --git a/fs/afs/dir.c b/fs/afs/dir.c index ca6b147963a9..cd74731112f4 100644 --- a/fs/afs/dir.c +++ b/fs/afs/dir.c @@ -700,6 +700,7 @@ static const struct afs_operation_ops afs_fetch_status_operation = { .issue_afs_rpc = afs_fs_fetch_status, .issue_yfs_rpc = yfs_fs_fetch_status, .success = afs_do_lookup_success, + .aborted = afs_check_for_remote_deletion, }; /* @@ -1236,6 +1237,17 @@ void afs_d_release(struct dentry *dentry) _enter("%pd", dentry); } +void afs_check_for_remote_deletion(struct afs_operation *op) +{ + struct afs_vnode *vnode = op->file[0].vnode; + + switch (op->ac.abort_code) { + case VNOVNODE: + set_bit(AFS_VNODE_DELETED, &vnode->flags); + afs_break_callback(vnode, afs_cb_break_for_deleted); + } +} + /* * Create a new inode for create/mkdir/symlink */ @@ -1269,7 +1281,6 @@ static void afs_create_success(struct afs_operation *op) { _enter("op=%08x", op->debug_id); op->ctime = op->file[0].scb.status.mtime_client; - afs_check_for_remote_deletion(op, op->file[0].vnode); afs_vnode_commit_status(op, &op->file[0]); afs_update_dentry_version(op, &op->file[0], op->dentry); afs_vnode_new_inode(op); @@ -1303,6 +1314,7 @@ static const struct afs_operation_ops afs_mkdir_operation = { .issue_afs_rpc = afs_fs_make_dir, .issue_yfs_rpc = yfs_fs_make_dir, .success = afs_create_success, + .aborted = afs_check_for_remote_deletion, .edit_dir = afs_create_edit_dir, .put = afs_create_put, }; @@ -1353,7 +1365,6 @@ static void afs_rmdir_success(struct afs_operation *op) { _enter("op=%08x", op->debug_id); op->ctime = op->file[0].scb.status.mtime_client; - afs_check_for_remote_deletion(op, op->file[0].vnode); afs_vnode_commit_status(op, &op->file[0]); afs_update_dentry_version(op, &op->file[0], op->dentry); } @@ -1385,6 +1396,7 @@ static const struct afs_operation_ops afs_rmdir_operation = { .issue_afs_rpc = afs_fs_remove_dir, .issue_yfs_rpc = yfs_fs_remove_dir, .success = afs_rmdir_success, + .aborted = afs_check_for_remote_deletion, .edit_dir = afs_rmdir_edit_dir, .put = afs_rmdir_put, }; @@ -1484,7 +1496,6 @@ static void afs_unlink_success(struct afs_operation *op) { _enter("op=%08x", op->debug_id); op->ctime = op->file[0].scb.status.mtime_client; - afs_check_for_remote_deletion(op, op->file[0].vnode); afs_vnode_commit_status(op, &op->file[0]); afs_vnode_commit_status(op, &op->file[1]); afs_update_dentry_version(op, &op->file[0], op->dentry); @@ -1516,6 +1527,7 @@ static const struct afs_operation_ops afs_unlink_operation = { .issue_afs_rpc = afs_fs_remove_file, .issue_yfs_rpc = yfs_fs_remove_file, .success = afs_unlink_success, + .aborted = afs_check_for_remote_deletion, .edit_dir = afs_unlink_edit_dir, .put = afs_unlink_put, }; @@ -1580,6 +1592,7 @@ static const struct afs_operation_ops afs_create_operation = { .issue_afs_rpc = afs_fs_create_file, .issue_yfs_rpc = yfs_fs_create_file, .success = afs_create_success, + .aborted = afs_check_for_remote_deletion, .edit_dir = afs_create_edit_dir, .put = afs_create_put, }; @@ -1649,6 +1662,7 @@ static const struct afs_operation_ops afs_link_operation = { .issue_afs_rpc = afs_fs_link, .issue_yfs_rpc = yfs_fs_link, .success = afs_link_success, + .aborted = afs_check_for_remote_deletion, .edit_dir = afs_create_edit_dir, .put = afs_link_put, }; @@ -1700,6 +1714,7 @@ static const struct afs_operation_ops afs_symlink_operation = { .issue_afs_rpc = afs_fs_symlink, .issue_yfs_rpc = yfs_fs_symlink, .success = afs_create_success, + .aborted = afs_check_for_remote_deletion, .edit_dir = afs_create_edit_dir, .put = afs_create_put, }; diff --git a/fs/afs/dir_silly.c b/fs/afs/dir_silly.c index b14e3d9a25e2..001adb87ff23 100644 --- a/fs/afs/dir_silly.c +++ b/fs/afs/dir_silly.c @@ -151,7 +151,6 @@ static void afs_silly_unlink_success(struct afs_operation *op) struct afs_vnode *vnode = op->file[1].vnode; _enter("op=%08x", op->debug_id); - afs_check_for_remote_deletion(op, op->file[0].vnode); afs_vnode_commit_status(op, &op->file[0]); afs_vnode_commit_status(op, &op->file[1]); afs_update_dentry_version(op, &op->file[0], op->dentry); @@ -181,6 +180,7 @@ static const struct afs_operation_ops afs_silly_unlink_operation = { .issue_afs_rpc = afs_fs_remove_file, .issue_yfs_rpc = yfs_fs_remove_file, .success = afs_silly_unlink_success, + .aborted = afs_check_for_remote_deletion, .edit_dir = afs_silly_unlink_edit_dir, }; diff --git a/fs/afs/file.c b/fs/afs/file.c index 506c47471b42..6f6ed1605cfe 100644 --- a/fs/afs/file.c +++ b/fs/afs/file.c @@ -225,7 +225,6 @@ static void afs_fetch_data_success(struct afs_operation *op) struct afs_vnode *vnode = op->file[0].vnode; _enter("op=%08x", op->debug_id); - afs_check_for_remote_deletion(op, vnode); afs_vnode_commit_status(op, &op->file[0]); afs_stat_v(vnode, n_fetches); atomic_long_add(op->fetch.req->actual_len, &op->net->n_fetch_bytes); @@ -240,6 +239,7 @@ static const struct afs_operation_ops afs_fetch_data_operation = { .issue_afs_rpc = afs_fs_fetch_data, .issue_yfs_rpc = yfs_fs_fetch_data, .success = afs_fetch_data_success, + .aborted = afs_check_for_remote_deletion, .put = afs_fetch_data_put, }; diff --git a/fs/afs/flock.c b/fs/afs/flock.c index 71eea2a908c7..ffb8575345ca 100644 --- a/fs/afs/flock.c +++ b/fs/afs/flock.c @@ -175,10 +175,7 @@ static void afs_kill_lockers_enoent(struct afs_vnode *vnode) static void afs_lock_success(struct afs_operation *op) { - struct afs_vnode *vnode = op->file[0].vnode; - _enter("op=%08x", op->debug_id); - afs_check_for_remote_deletion(op, vnode); afs_vnode_commit_status(op, &op->file[0]); } @@ -186,6 +183,7 @@ static const struct afs_operation_ops afs_set_lock_operation = { .issue_afs_rpc = afs_fs_set_lock, .issue_yfs_rpc = yfs_fs_set_lock, .success = afs_lock_success, + .aborted = afs_check_for_remote_deletion, }; /* diff --git a/fs/afs/fs_operation.c b/fs/afs/fs_operation.c index 2d2dff5688a4..c264839b2fd0 100644 --- a/fs/afs/fs_operation.c +++ b/fs/afs/fs_operation.c @@ -187,9 +187,17 @@ void afs_wait_for_operation(struct afs_operation *op) op->error = afs_wait_for_call_to_complete(op->call, &op->ac); } - if (op->error == 0) { + switch (op->error) { + case 0: _debug("success"); op->ops->success(op); + break; + case -ECONNABORTED: + if (op->ops->aborted) + op->ops->aborted(op); + break; + default: + break; } afs_end_vnode_operation(op); diff --git a/fs/afs/inode.c b/fs/afs/inode.c index 70c925978d10..56e60d561f37 100644 --- a/fs/afs/inode.c +++ b/fs/afs/inode.c @@ -324,6 +324,7 @@ static const struct afs_operation_ops afs_fetch_status_operation = { .issue_afs_rpc = afs_fs_fetch_status, .issue_yfs_rpc = yfs_fs_fetch_status, .success = afs_fetch_status_success, + .aborted = afs_check_for_remote_deletion, }; /* diff --git a/fs/afs/internal.h b/fs/afs/internal.h index 598934d923cc..9420890e3577 100644 --- a/fs/afs/internal.h +++ b/fs/afs/internal.h @@ -934,6 +934,7 @@ extern const struct address_space_operations afs_dir_aops; extern const struct dentry_operations afs_fs_dentry_operations; extern void afs_d_release(struct dentry *); +extern void afs_check_for_remote_deletion(struct afs_operation *); /* * dir_edit.c @@ -1482,15 +1483,6 @@ static inline struct inode *AFS_VNODE_TO_I(struct afs_vnode *vnode) return &vnode->vfs_inode; } -static inline void afs_check_for_remote_deletion(struct afs_operation *op, - struct afs_vnode *vnode) -{ - if (op->error == -ENOENT) { - set_bit(AFS_VNODE_DELETED, &vnode->flags); - afs_break_callback(vnode, afs_cb_break_for_deleted); - } -} - /* * Note that a dentry got changed. We need to set d_fsdata to the data version * number derived from the result of the operation. It doesn't matter if From 7c295eec1e351003a8ca06c34f9e79336fa5b244 Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 16 Jun 2020 00:52:30 +0100 Subject: [PATCH 11/12] afs: afs_vnode_commit_status() doesn't need to check the RPC error afs_vnode_commit_status() is only ever called if op->error is 0, so remove the op->error checks from the function. Fixes: e49c7b2f6de7 ("afs: Build an abstraction around an "operation" concept") Signed-off-by: David Howells --- fs/afs/inode.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/fs/afs/inode.c b/fs/afs/inode.c index 56e60d561f37..d5d0ae7b2b1e 100644 --- a/fs/afs/inode.c +++ b/fs/afs/inode.c @@ -281,8 +281,6 @@ void afs_vnode_commit_status(struct afs_operation *op, struct afs_vnode_param *v _enter(""); - ASSERTCMP(op->error, ==, 0); - write_seqlock(&vnode->cb_lock); if (vp->scb.have_error) { @@ -300,7 +298,7 @@ void afs_vnode_commit_status(struct afs_operation *op, struct afs_vnode_param *v write_sequnlock(&vnode->cb_lock); - if (op->error == 0 && vp->scb.have_status) + if (vp->scb.have_status) afs_cache_permit(vnode, op->key, vp->cb_break_before, &vp->scb); } From b6489a49f7b71964e37978d6f89bbdbdb263f6f5 Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 15 Jun 2020 17:36:58 +0100 Subject: [PATCH 12/12] afs: Fix silly rename Fix AFS's silly rename by the following means: (1) Set the destination directory in afs_do_silly_rename() so as to avoid misbehaviour and indicate that the directory data version will increment by 1 so as to avoid warnings about unexpected changes in the DV. Also indicate that the ctime should be updated to avoid xfstest grumbling. (2) Note when the server indicates that a directory changed more than we expected (AFS_OPERATION_DIR_CONFLICT), indicating a conflict with a third party change, checking on successful completion of unlink and rename. The problem is that the FS.RemoveFile RPC op doesn't report the status of the unlinked file, though YFS.RemoveFile2 does. This can be mitigated by the assumption that if the directory DV cranked by exactly 1, we can be sure we removed one link from the file; further, ordinarily in AFS, files cannot be hardlinked across directories, so if we reduce nlink to 0, the file is deleted. However, if the directory DV jumps by more than 1, we cannot know if a third party intervened by adding or removing a link on the file we just removed a link from. The same also goes for any vnode that is at the destination of the FS.Rename RPC op. (3) Make afs_vnode_commit_status() apply the nlink drop inside the cb_lock section along with the other attribute updates if ->op_unlinked is set on the descriptor for the appropriate vnode. (4) Issue a follow up status fetch to the unlinked file in the event of a third party conflict that makes it impossible for us to know if we actually deleted the file or not. (5) Provide a flag, AFS_VNODE_SILLY_DELETED, to make afs_getattr() lie to the user about the nlink of a silly deleted file so that it appears as 0, not 1. Found with the generic/035 and generic/084 xfstests. Fixes: e49c7b2f6de7 ("afs: Build an abstraction around an "operation" concept") Reported-by: Marc Dionne Signed-off-by: David Howells --- fs/afs/dir.c | 21 +++++++++++++++++++-- fs/afs/dir_silly.c | 36 +++++++++++++++++++++++++++--------- fs/afs/inode.c | 22 +++++++++++++++++----- fs/afs/internal.h | 17 +++++++++++++++++ 4 files changed, 80 insertions(+), 16 deletions(-) diff --git a/fs/afs/dir.c b/fs/afs/dir.c index cd74731112f4..3e3c2bf0a722 100644 --- a/fs/afs/dir.c +++ b/fs/afs/dir.c @@ -696,7 +696,7 @@ static const struct afs_operation_ops afs_inline_bulk_status_operation = { .success = afs_do_lookup_success, }; -static const struct afs_operation_ops afs_fetch_status_operation = { +static const struct afs_operation_ops afs_lookup_fetch_status_operation = { .issue_afs_rpc = afs_fs_fetch_status, .issue_yfs_rpc = yfs_fs_fetch_status, .success = afs_do_lookup_success, @@ -1496,6 +1496,7 @@ static void afs_unlink_success(struct afs_operation *op) { _enter("op=%08x", op->debug_id); op->ctime = op->file[0].scb.status.mtime_client; + afs_check_dir_conflict(op, &op->file[0]); afs_vnode_commit_status(op, &op->file[0]); afs_vnode_commit_status(op, &op->file[1]); afs_update_dentry_version(op, &op->file[0], op->dentry); @@ -1580,9 +1581,24 @@ static int afs_unlink(struct inode *dir, struct dentry *dentry) op->file[1].vnode = vnode; op->file[1].update_ctime = true; + op->file[1].op_unlinked = true; op->dentry = dentry; op->ops = &afs_unlink_operation; - return afs_do_sync_operation(op); + afs_begin_vnode_operation(op); + afs_wait_for_operation(op); + + /* If there was a conflict with a third party, check the status of the + * unlinked vnode. + */ + if (op->error == 0 && (op->flags & AFS_OPERATION_DIR_CONFLICT)) { + op->file[1].update_ctime = false; + op->fetch_status.which = 1; + op->ops = &afs_fetch_status_operation; + afs_begin_vnode_operation(op); + afs_wait_for_operation(op); + } + + return afs_put_operation(op); error: return afs_put_operation(op); @@ -1767,6 +1783,7 @@ static void afs_rename_success(struct afs_operation *op) _enter("op=%08x", op->debug_id); op->ctime = op->file[0].scb.status.mtime_client; + afs_check_dir_conflict(op, &op->file[1]); afs_vnode_commit_status(op, &op->file[0]); if (op->file[1].vnode != op->file[0].vnode) { op->ctime = op->file[1].scb.status.mtime_client; diff --git a/fs/afs/dir_silly.c b/fs/afs/dir_silly.c index 001adb87ff23..04f75a44f243 100644 --- a/fs/afs/dir_silly.c +++ b/fs/afs/dir_silly.c @@ -16,6 +16,7 @@ static void afs_silly_rename_success(struct afs_operation *op) { _enter("op=%08x", op->debug_id); + afs_check_dir_conflict(op, &op->file[0]); afs_vnode_commit_status(op, &op->file[0]); } @@ -69,6 +70,11 @@ static int afs_do_silly_rename(struct afs_vnode *dvnode, struct afs_vnode *vnode return PTR_ERR(op); afs_op_set_vnode(op, 0, dvnode); + afs_op_set_vnode(op, 1, dvnode); + op->file[0].dv_delta = 1; + op->file[1].dv_delta = 1; + op->file[0].update_ctime = true; + op->file[1].update_ctime = true; op->dentry = old; op->dentry_2 = new; @@ -129,6 +135,7 @@ int afs_sillyrename(struct afs_vnode *dvnode, struct afs_vnode *vnode, switch (ret) { case 0: /* The rename succeeded. */ + set_bit(AFS_VNODE_SILLY_DELETED, &vnode->flags); d_move(dentry, sdentry); break; case -ERESTARTSYS: @@ -148,18 +155,11 @@ int afs_sillyrename(struct afs_vnode *dvnode, struct afs_vnode *vnode, static void afs_silly_unlink_success(struct afs_operation *op) { - struct afs_vnode *vnode = op->file[1].vnode; - _enter("op=%08x", op->debug_id); + afs_check_dir_conflict(op, &op->file[0]); afs_vnode_commit_status(op, &op->file[0]); afs_vnode_commit_status(op, &op->file[1]); afs_update_dentry_version(op, &op->file[0], op->dentry); - - drop_nlink(&vnode->vfs_inode); - if (vnode->vfs_inode.i_nlink == 0) { - set_bit(AFS_VNODE_DELETED, &vnode->flags); - clear_bit(AFS_VNODE_CB_PROMISED, &vnode->flags); - } } static void afs_silly_unlink_edit_dir(struct afs_operation *op) @@ -200,12 +200,30 @@ static int afs_do_silly_unlink(struct afs_vnode *dvnode, struct afs_vnode *vnode afs_op_set_vnode(op, 0, dvnode); afs_op_set_vnode(op, 1, vnode); + op->file[0].dv_delta = 1; + op->file[0].update_ctime = true; + op->file[1].op_unlinked = true; + op->file[1].update_ctime = true; op->dentry = dentry; op->ops = &afs_silly_unlink_operation; trace_afs_silly_rename(vnode, true); - return afs_do_sync_operation(op); + afs_begin_vnode_operation(op); + afs_wait_for_operation(op); + + /* If there was a conflict with a third party, check the status of the + * unlinked vnode. + */ + if (op->error == 0 && (op->flags & AFS_OPERATION_DIR_CONFLICT)) { + op->file[1].update_ctime = false; + op->fetch_status.which = 1; + op->ops = &afs_fetch_status_operation; + afs_begin_vnode_operation(op); + afs_wait_for_operation(op); + } + + return afs_put_operation(op); } /* diff --git a/fs/afs/inode.c b/fs/afs/inode.c index d5d0ae7b2b1e..1d13d2e882ad 100644 --- a/fs/afs/inode.c +++ b/fs/afs/inode.c @@ -284,16 +284,25 @@ void afs_vnode_commit_status(struct afs_operation *op, struct afs_vnode_param *v write_seqlock(&vnode->cb_lock); if (vp->scb.have_error) { + /* A YFS server will return this from RemoveFile2 and AFS and + * YFS will return this from InlineBulkStatus. + */ if (vp->scb.status.abort_code == VNOVNODE) { set_bit(AFS_VNODE_DELETED, &vnode->flags); clear_nlink(&vnode->vfs_inode); __afs_break_callback(vnode, afs_cb_break_for_deleted); + op->flags &= ~AFS_OPERATION_DIR_CONFLICT; } - } else { - if (vp->scb.have_status) - afs_apply_status(op, vp); + } else if (vp->scb.have_status) { + afs_apply_status(op, vp); if (vp->scb.have_cb) afs_apply_callback(op, vp); + } else if (vp->op_unlinked && !(op->flags & AFS_OPERATION_DIR_CONFLICT)) { + drop_nlink(&vnode->vfs_inode); + if (vnode->vfs_inode.i_nlink == 0) { + set_bit(AFS_VNODE_DELETED, &vnode->flags); + __afs_break_callback(vnode, afs_cb_break_for_deleted); + } } write_sequnlock(&vnode->cb_lock); @@ -304,7 +313,7 @@ void afs_vnode_commit_status(struct afs_operation *op, struct afs_vnode_param *v static void afs_fetch_status_success(struct afs_operation *op) { - struct afs_vnode_param *vp = &op->file[0]; + struct afs_vnode_param *vp = &op->file[op->fetch_status.which]; struct afs_vnode *vnode = vp->vnode; int ret; @@ -318,7 +327,7 @@ static void afs_fetch_status_success(struct afs_operation *op) } } -static const struct afs_operation_ops afs_fetch_status_operation = { +const struct afs_operation_ops afs_fetch_status_operation = { .issue_afs_rpc = afs_fs_fetch_status, .issue_yfs_rpc = yfs_fs_fetch_status, .success = afs_fetch_status_success, @@ -729,6 +738,9 @@ int afs_getattr(const struct path *path, struct kstat *stat, do { read_seqbegin_or_lock(&vnode->cb_lock, &seq); generic_fillattr(inode, stat); + if (test_bit(AFS_VNODE_SILLY_DELETED, &vnode->flags) && + stat->nlink > 0) + stat->nlink -= 1; } while (need_seqretry(&vnode->cb_lock, seq)); done_seqretry(&vnode->cb_lock, seq); diff --git a/fs/afs/internal.h b/fs/afs/internal.h index 9420890e3577..573a5922c3bb 100644 --- a/fs/afs/internal.h +++ b/fs/afs/internal.h @@ -634,6 +634,7 @@ struct afs_vnode { #define AFS_VNODE_AUTOCELL 6 /* set if Vnode is an auto mount point */ #define AFS_VNODE_PSEUDODIR 7 /* set if Vnode is a pseudo directory */ #define AFS_VNODE_NEW_CONTENT 8 /* Set if file has new content (create/trunc-0) */ +#define AFS_VNODE_SILLY_DELETED 9 /* Set if file has been silly-deleted */ struct list_head wb_keys; /* List of keys available for writeback */ struct list_head pending_locks; /* locks waiting to be granted */ @@ -748,6 +749,7 @@ struct afs_vnode_param { bool need_io_lock:1; /* T if we need the I/O lock on this */ bool update_ctime:1; /* Need to update the ctime */ bool set_size:1; /* Must update i_size */ + bool op_unlinked:1; /* True if file was unlinked by op */ }; /* @@ -839,6 +841,7 @@ struct afs_operation { #define AFS_OPERATION_LOCK_1 0x0200 /* Set if have io_lock on file[1] */ #define AFS_OPERATION_TRIED_ALL 0x0400 /* Set if we've tried all the fileservers */ #define AFS_OPERATION_RETRY_SERVER 0x0800 /* Set if we should retry the current server */ +#define AFS_OPERATION_DIR_CONFLICT 0x1000 /* Set if we detected a 3rd-party dir change */ }; /* @@ -1066,6 +1069,8 @@ extern int afs_wait_for_one_fs_probe(struct afs_server *, bool); /* * inode.c */ +extern const struct afs_operation_ops afs_fetch_status_operation; + extern void afs_vnode_commit_status(struct afs_operation *, struct afs_vnode_param *); extern int afs_fetch_status(struct afs_vnode *, struct key *, bool, afs_access_t *); extern int afs_ilookup5_test_by_fid(struct inode *, void *); @@ -1497,6 +1502,18 @@ static inline void afs_update_dentry_version(struct afs_operation *op, (void *)(unsigned long)dir_vp->scb.status.data_version; } +/* + * Check for a conflicting operation on a directory that we just unlinked from. + * If someone managed to sneak a link or an unlink in on the file we just + * unlinked, we won't be able to trust nlink on an AFS file (but not YFS). + */ +static inline void afs_check_dir_conflict(struct afs_operation *op, + struct afs_vnode_param *dvp) +{ + if (dvp->dv_before + dvp->dv_delta != dvp->scb.status.data_version) + op->flags |= AFS_OPERATION_DIR_CONFLICT; +} + static inline int afs_io_error(struct afs_call *call, enum afs_io_error where) { trace_afs_io_error(call->debug_id, -EIO, where);