Messages in this thread | | | Subject | Re: [PATCH v3 2/2] ceph: wait the first reply of inflight async unlink | From | Xiubo Li <> | Date | Wed, 18 May 2022 09:03:00 +0800 |
| |
On 5/17/22 10:36 PM, Matthew Wilcox wrote: > On Tue, May 17, 2022 at 08:55:49PM +0800, Xiubo Li wrote: >> +int ceph_wait_on_conflict_unlink(struct dentry *dentry) >> +{ >> + struct ceph_fs_client *fsc = ceph_sb_to_client(dentry->d_sb); >> + struct dentry *pdentry = dentry->d_parent; >> + struct dentry *udentry, *found = NULL; >> + struct ceph_dentry_info *di; >> + struct qstr dname; >> + u32 hash = dentry->d_name.hash; >> + int err; >> + >> + dname.name = dentry->d_name.name; >> + dname.len = dentry->d_name.len; >> + >> + rcu_read_lock(); >> + hash_for_each_possible_rcu(fsc->async_unlink_conflict, di, >> + hnode, hash) { >> + udentry = di->dentry; >> + >> + spin_lock(&udentry->d_lock); >> + if (udentry->d_name.hash != hash) >> + goto next; >> + if (unlikely(udentry->d_parent != pdentry)) >> + goto next; >> + if (!hash_hashed(&di->hnode)) >> + goto next; >> + >> + if (!test_bit(CEPH_DENTRY_ASYNC_UNLINK_BIT, &di->flags)) >> + pr_warn("%s dentry %p:%pd async unlink bit is not set\n", >> + __func__, dentry, dentry); >> + >> + if (d_compare(pdentry, udentry, &dname)) >> + goto next; >> + >> + spin_unlock(&udentry->d_lock); >> + found = dget(udentry); >> + break; >> +next: >> + spin_unlock(&udentry->d_lock); >> + } >> + rcu_read_unlock(); >> + >> + if (likely(!found)) >> + return 0; >> + >> + dout("%s dentry %p:%pd conflict with old %p:%pd\n", __func__, >> + dentry, dentry, found, found); >> + >> + err = wait_on_bit(&di->flags, CEPH_DENTRY_ASYNC_UNLINK_BIT, >> + TASK_INTERRUPTIBLE); > Do you really want to use TASK_INTERRUPTIBLE here? If the window is > resized and you get a SIGWINCH, or a timer goes off and you get a > SIGALRM, you want to return -EINTR? I would suggest that TASK_KILLABLE > is probably the semantics that you want. > Sounds reasonable. I will switch to use the TASK_KILLABLE.
@Jeff
I just copied this code from ceph_wait_on_async_create(). BTW, do we have any other consideration that we must use the TASK_INTERRUPTIBLE there ?
Thanks.
-- Xiubo
| |