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. |