| 1 | | Okay, I have a promising theory, and I’m now testing the following patch on b-m, p-b, r-m, and w-e: |
| 2 | | |
| 3 | | {{{ |
| 4 | | From: Anders Kaseorg <andersk@mit.edu> |
| 5 | | Date: Fri, 8 Nov 2013 08:53:59 -0500 |
| 6 | | Subject: [PATCH] Linux: canonical_dentry: Don’t reverse the order if D_ALIAS_IS_HLIST |
| 7 | | |
| 8 | | It turns out commit 6bea047fb404bde828c6358ae06f7941aa2bc959 “Linux |
| 9 | | 3.6: d_alias and i_dentry are now hlists” had a second bug (the first |
| 10 | | being the one fixed by commit a71cc5511c115c1b6cb4a6a2997a846bab6e19e2 |
| 11 | | “Linux: osi_TryEvictVCache: Don’t skip the first dentry if |
| 12 | | D_ALIAS_IS_HLIST”). Since there is no corresponding hlist function |
| 13 | | for list_for_each_entry_reverse, the commit just used |
| 14 | | hlist_for_each_entry. But that changed the search order so that |
| 15 | | canonical_dentry now chose the newest dentry in the i_dentry list |
| 16 | | instead of the oldest one! |
| 17 | | |
| 18 | | Since the newest dentry may change at any time, this defeated the |
| 19 | | protections of commit dda3ea5f9ddda389955249e17a2e97b2e5ce7f1c “Linux: |
| 20 | | Make dir dentry aliases act like symlinks”. I believe this to be the |
| 21 | | cause of the symptoms on scripts.mit.edu servers where getcwd() |
| 22 | | mysteriously starts returning ENOENT. |
| 23 | | |
| 24 | | Fix the canonical_dentry algorithm to find the oldest dentry, like it |
| 25 | | did before d_alias became an hlist, but using forward iteration |
| 26 | | instead of reverse iteration. Clarify the variable name and comments |
| 27 | | to make clear that this is important. |
| 28 | | |
| 29 | | Change-Id: I9d31f90fd63e44dcde62d0257059c09527b4f93a |
| 30 | | Signed-off-by: Anders Kaseorg <andersk@mit.edu> |
| 31 | | --- |
| 32 | | src/afs/LINUX/osi_vnodeops.c | 15 ++++++--------- |
| 33 | | 1 file changed, 6 insertions(+), 9 deletions(-) |
| 34 | | |
| 35 | | diff --git a/src/afs/LINUX/osi_vnodeops.c b/src/afs/LINUX/osi_vnodeops.c |
| 36 | | index 2a29625..fabf575 100644 |
| 37 | | --- a/src/afs/LINUX/osi_vnodeops.c |
| 38 | | +++ b/src/afs/LINUX/osi_vnodeops.c |
| 39 | | @@ -839,14 +839,14 @@ static struct dentry * |
| 40 | | canonical_dentry(struct inode *ip) |
| 41 | | { |
| 42 | | struct vcache *vcp = VTOAFS(ip); |
| 43 | | - struct dentry *first = NULL, *ret = NULL, *cur; |
| 44 | | + struct dentry *oldest = NULL, *ret = NULL, *cur; |
| 45 | | #if defined(D_ALIAS_IS_HLIST) && !defined(HLIST_ITERATOR_NO_NODE) |
| 46 | | struct hlist_node *p; |
| 47 | | #endif |
| 48 | | |
| 49 | | /* general strategy: |
| 50 | | * if vcp->target_link is set, and can be found in ip->i_dentry, use that. |
| 51 | | - * otherwise, use the first dentry in ip->i_dentry. |
| 52 | | + * otherwise, use the oldest (tail) dentry in ip->i_dentry. |
| 53 | | * if ip->i_dentry is empty, use the 'dentry' argument we were given. |
| 54 | | */ |
| 55 | | /* note that vcp->target_link specifies which dentry to use, but we have |
| 56 | | @@ -869,20 +869,17 @@ canonical_dentry(struct inode *ip) |
| 57 | | hlist_for_each_entry(cur, p, &ip->i_dentry, d_alias) { |
| 58 | | # endif |
| 59 | | #else |
| 60 | | - list_for_each_entry_reverse(cur, &ip->i_dentry, d_alias) { |
| 61 | | + list_for_each_entry(cur, &ip->i_dentry, d_alias) { |
| 62 | | #endif |
| 63 | | |
| 64 | | if (!vcp->target_link || cur == vcp->target_link) { |
| 65 | | ret = cur; |
| 66 | | - break; |
| 67 | | } |
| 68 | | |
| 69 | | - if (!first) { |
| 70 | | - first = cur; |
| 71 | | - } |
| 72 | | + oldest = cur; |
| 73 | | } |
| 74 | | - if (!ret && first) { |
| 75 | | - ret = first; |
| 76 | | + if (!ret && oldest) { |
| 77 | | + ret = oldest; |
| 78 | | } |
| 79 | | |
| 80 | | vcp->target_link = ret; |
| 81 | | -- |
| 82 | | 1.8.5.rc0 |
| 83 | | }}} |
| | 1 | Okay, I have a promising theory, and I’m now testing [http://gerrit.openafs.org/10444 this patch] on b-m, p-b, r-m, and w-e. |
