Go to:
Gentoo Home
Documentation
Forums
Lists
Bugs
Planet
Store
Wiki
Get Gentoo!
Gentoo's Bugzilla – Attachment 363178 Details for
Bug 488492
[PATCH+] =sys-kernel/gentoo-sources-3.12 - Sandbox violations
Home
|
New
–
[Ex]
|
Browse
|
Search
|
Privacy Policy
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[PATCH] dcache: Revert pathname locking changes.
0001-dcache-Revert-pathname-locking-changes.patch (text/plain), 12.52 KB, created by
mike@marineau.org
on 2013-11-13 06:15:41 UTC
(
hide
)
Description:
[PATCH] dcache: Revert pathname locking changes.
Filename:
MIME Type:
Creator:
mike@marineau.org
Created:
2013-11-13 06:15:41 UTC
Size:
12.52 KB
patch
obsolete
>From 213127d7a3df9fde077484aca01f34f256ddaf62 Mon Sep 17 00:00:00 2001 >From: Michael Marineau <mike@marineau.org> >Date: Tue, 12 Nov 2013 22:02:07 -0800 >Subject: [PATCH] dcache: Revert pathname locking changes. > >This reverts the following commits: >232d2d60 Translating dentry into pathname without taking rename_lock >48f5ec21 split read_seqretry_or_unlock(), convert d_walk()... >18129977 get/release read lock in read_seqbegin_or_lock() & friend > >The first introduced a regression that causes sandbox to randomly fail, >the following two commits depend on the first. >--- > fs/dcache.c | 221 +++++++++++++++++++++--------------------------------------- > 1 file changed, 76 insertions(+), 145 deletions(-) > >diff --git a/fs/dcache.c b/fs/dcache.c >index ae6ebb8..d1686c7 100644 >--- a/fs/dcache.c >+++ b/fs/dcache.c >@@ -88,35 +88,6 @@ EXPORT_SYMBOL(rename_lock); > > static struct kmem_cache *dentry_cache __read_mostly; > >-/** >- * read_seqbegin_or_lock - begin a sequence number check or locking block >- * @lock: sequence lock >- * @seq : sequence number to be checked >- * >- * First try it once optimistically without taking the lock. If that fails, >- * take the lock. The sequence number is also used as a marker for deciding >- * whether to be a reader (even) or writer (odd). >- * N.B. seq must be initialized to an even number to begin with. >- */ >-static inline void read_seqbegin_or_lock(seqlock_t *lock, int *seq) >-{ >- if (!(*seq & 1)) /* Even */ >- *seq = read_seqbegin(lock); >- else /* Odd */ >- read_seqlock_excl(lock); >-} >- >-static inline int need_seqretry(seqlock_t *lock, int seq) >-{ >- return !(seq & 1) && read_seqretry(lock, seq); >-} >- >-static inline void done_seqretry(seqlock_t *lock, int seq) >-{ >- if (seq & 1) >- read_sequnlock_excl(lock); >-} >- > /* > * This is the single most critical data structure when it comes > * to the dcache: the hashtable for lookups. Somebody should try >@@ -1191,7 +1162,7 @@ void shrink_dcache_for_umount(struct super_block *sb) > * the parenthood after dropping the lock and check > * that the sequence number still matches. > */ >-static struct dentry *try_to_ascend(struct dentry *old, unsigned seq) >+static struct dentry *try_to_ascend(struct dentry *old, int locked, unsigned seq) > { > struct dentry *new = old->d_parent; > >@@ -1205,7 +1176,7 @@ static struct dentry *try_to_ascend(struct dentry *old, unsigned seq) > */ > if (new != old->d_parent || > (old->d_flags & DCACHE_DENTRY_KILLED) || >- need_seqretry(&rename_lock, seq)) { >+ (!locked && read_seqretry(&rename_lock, seq))) { > spin_unlock(&new->d_lock); > new = NULL; > } >@@ -1242,12 +1213,13 @@ static void d_walk(struct dentry *parent, void *data, > { > struct dentry *this_parent; > struct list_head *next; >- unsigned seq = 0; >+ unsigned seq; >+ int locked = 0; > enum d_walk_ret ret; > bool retry = true; > >+ seq = read_seqbegin(&rename_lock); > again: >- read_seqbegin_or_lock(&rename_lock, &seq); > this_parent = parent; > spin_lock(&this_parent->d_lock); > >@@ -1301,13 +1273,13 @@ resume: > */ > if (this_parent != parent) { > struct dentry *child = this_parent; >- this_parent = try_to_ascend(this_parent, seq); >+ this_parent = try_to_ascend(this_parent, locked, seq); > if (!this_parent) > goto rename_retry; > next = child->d_u.d_child.next; > goto resume; > } >- if (need_seqretry(&rename_lock, seq)) { >+ if (!locked && read_seqretry(&rename_lock, seq)) { > spin_unlock(&this_parent->d_lock); > goto rename_retry; > } >@@ -1316,13 +1288,17 @@ resume: > > out_unlock: > spin_unlock(&this_parent->d_lock); >- done_seqretry(&rename_lock, seq); >+ if (locked) >+ write_sequnlock(&rename_lock); > return; > > rename_retry: > if (!retry) > return; >- seq = 1; >+ if (locked) >+ goto again; >+ locked = 1; >+ write_seqlock(&rename_lock); > goto again; > } > >@@ -2825,39 +2801,9 @@ static int prepend(char **buffer, int *buflen, const char *str, int namelen) > return 0; > } > >-/** >- * prepend_name - prepend a pathname in front of current buffer pointer >- * @buffer: buffer pointer >- * @buflen: allocated length of the buffer >- * @name: name string and length qstr structure >- * >- * With RCU path tracing, it may race with d_move(). Use ACCESS_ONCE() to >- * make sure that either the old or the new name pointer and length are >- * fetched. However, there may be mismatch between length and pointer. >- * The length cannot be trusted, we need to copy it byte-by-byte until >- * the length is reached or a null byte is found. It also prepends "/" at >- * the beginning of the name. The sequence number check at the caller will >- * retry it again when a d_move() does happen. So any garbage in the buffer >- * due to mismatched pointer and length will be discarded. >- */ > static int prepend_name(char **buffer, int *buflen, struct qstr *name) > { >- const char *dname = ACCESS_ONCE(name->name); >- u32 dlen = ACCESS_ONCE(name->len); >- char *p; >- >- if (*buflen < dlen + 1) >- return -ENAMETOOLONG; >- *buflen -= dlen + 1; >- p = *buffer -= dlen + 1; >- *p++ = '/'; >- while (dlen--) { >- char c = *dname++; >- if (!c) >- break; >- *p++ = c; >- } >- return 0; >+ return prepend(buffer, buflen, name->name, name->len); > } > > /** >@@ -2867,15 +2813,7 @@ static int prepend_name(char **buffer, int *buflen, struct qstr *name) > * @buffer: pointer to the end of the buffer > * @buflen: pointer to buffer length > * >- * The function will first try to write out the pathname without taking any >- * lock other than the RCU read lock to make sure that dentries won't go away. >- * It only checks the sequence number of the global rename_lock as any change >- * in the dentry's d_seq will be preceded by changes in the rename_lock >- * sequence number. If the sequence number had been changed, it will restart >- * the whole pathname back-tracing sequence again by taking the rename_lock. >- * In this case, there is no need to take the RCU read lock as the recursive >- * parent pointer references will keep the dentry chain alive as long as no >- * rename operation is performed. >+ * Caller holds the rename_lock. > */ > static int prepend_path(const struct path *path, > const struct path *root, >@@ -2884,66 +2822,54 @@ static int prepend_path(const struct path *path, > struct dentry *dentry = path->dentry; > struct vfsmount *vfsmnt = path->mnt; > struct mount *mnt = real_mount(vfsmnt); >+ bool slash = false; > int error = 0; >- unsigned seq = 0; >- char *bptr; >- int blen; > >- rcu_read_lock(); >-restart: >- bptr = *buffer; >- blen = *buflen; >- read_seqbegin_or_lock(&rename_lock, &seq); > while (dentry != root->dentry || vfsmnt != root->mnt) { > struct dentry * parent; > > if (dentry == vfsmnt->mnt_root || IS_ROOT(dentry)) { > /* Global root? */ >- if (mnt_has_parent(mnt)) { >- dentry = mnt->mnt_mountpoint; >- mnt = mnt->mnt_parent; >- vfsmnt = &mnt->mnt; >- continue; >- } >- /* >- * Filesystems needing to implement special "root names" >- * should do so with ->d_dname() >- */ >- if (IS_ROOT(dentry) && >- (dentry->d_name.len != 1 || >- dentry->d_name.name[0] != '/')) { >- WARN(1, "Root dentry has weird name <%.*s>\n", >- (int) dentry->d_name.len, >- dentry->d_name.name); >- } >- if (!error) >- error = is_mounted(vfsmnt) ? 1 : 2; >- break; >+ if (!mnt_has_parent(mnt)) >+ goto global_root; >+ dentry = mnt->mnt_mountpoint; >+ mnt = mnt->mnt_parent; >+ vfsmnt = &mnt->mnt; >+ continue; > } > parent = dentry->d_parent; > prefetch(parent); >- error = prepend_name(&bptr, &blen, &dentry->d_name); >+ spin_lock(&dentry->d_lock); >+ error = prepend_name(buffer, buflen, &dentry->d_name); >+ spin_unlock(&dentry->d_lock); >+ if (!error) >+ error = prepend(buffer, buflen, "/", 1); > if (error) > break; > >+ slash = true; > dentry = parent; > } >- if (!(seq & 1)) >- rcu_read_unlock(); >- if (need_seqretry(&rename_lock, seq)) { >- seq = 1; >- goto restart; >- } >- done_seqretry(&rename_lock, seq); > >- if (error >= 0 && bptr == *buffer) { >- if (--blen < 0) >- error = -ENAMETOOLONG; >- else >- *--bptr = '/'; >- } >- *buffer = bptr; >- *buflen = blen; >+ if (!error && !slash) >+ error = prepend(buffer, buflen, "/", 1); >+ >+ return error; >+ >+global_root: >+ /* >+ * Filesystems needing to implement special "root names" >+ * should do so with ->d_dname() >+ */ >+ if (IS_ROOT(dentry) && >+ (dentry->d_name.len != 1 || dentry->d_name.name[0] != '/')) { >+ WARN(1, "Root dentry has weird name <%.*s>\n", >+ (int) dentry->d_name.len, dentry->d_name.name); >+ } >+ if (!slash) >+ error = prepend(buffer, buflen, "/", 1); >+ if (!error) >+ error = is_mounted(vfsmnt) ? 1 : 2; > return error; > } > >@@ -2972,7 +2898,9 @@ char *__d_path(const struct path *path, > > prepend(&res, &buflen, "\0", 1); > br_read_lock(&vfsmount_lock); >+ write_seqlock(&rename_lock); > error = prepend_path(path, root, &res, &buflen); >+ write_sequnlock(&rename_lock); > br_read_unlock(&vfsmount_lock); > > if (error < 0) >@@ -2991,7 +2919,9 @@ char *d_absolute_path(const struct path *path, > > prepend(&res, &buflen, "\0", 1); > br_read_lock(&vfsmount_lock); >+ write_seqlock(&rename_lock); > error = prepend_path(path, &root, &res, &buflen); >+ write_sequnlock(&rename_lock); > br_read_unlock(&vfsmount_lock); > > if (error > 1) >@@ -3068,7 +2998,9 @@ char *d_path(const struct path *path, char *buf, int buflen) > rcu_read_lock(); > get_fs_root_rcu(current->fs, &root); > br_read_lock(&vfsmount_lock); >+ write_seqlock(&rename_lock); > error = path_with_deleted(path, &root, &res, &buflen); >+ write_sequnlock(&rename_lock); > br_read_unlock(&vfsmount_lock); > rcu_read_unlock(); > >@@ -3104,10 +3036,10 @@ char *simple_dname(struct dentry *dentry, char *buffer, int buflen) > char *end = buffer + buflen; > /* these dentries are never renamed, so d_lock is not needed */ > if (prepend(&end, &buflen, " (deleted)", 11) || >- prepend(&end, &buflen, dentry->d_name.name, dentry->d_name.len) || >+ prepend_name(&end, &buflen, &dentry->d_name) || > prepend(&end, &buflen, "/", 1)) > end = ERR_PTR(-ENAMETOOLONG); >- return end; >+ return end; > } > > /* >@@ -3115,42 +3047,30 @@ char *simple_dname(struct dentry *dentry, char *buffer, int buflen) > */ > static char *__dentry_path(struct dentry *dentry, char *buf, int buflen) > { >- char *end, *retval; >- int len, seq = 0; >- int error = 0; >+ char *end = buf + buflen; >+ char *retval; > >- rcu_read_lock(); >-restart: >- end = buf + buflen; >- len = buflen; >- prepend(&end, &len, "\0", 1); >+ prepend(&end, &buflen, "\0", 1); > if (buflen < 1) > goto Elong; > /* Get '/' right */ > retval = end-1; > *retval = '/'; >- read_seqbegin_or_lock(&rename_lock, &seq); >+ > while (!IS_ROOT(dentry)) { > struct dentry *parent = dentry->d_parent; > int error; > > prefetch(parent); >- error = prepend_name(&end, &len, &dentry->d_name); >- if (error) >- break; >+ spin_lock(&dentry->d_lock); >+ error = prepend_name(&end, &buflen, &dentry->d_name); >+ spin_unlock(&dentry->d_lock); >+ if (error != 0 || prepend(&end, &buflen, "/", 1) != 0) >+ goto Elong; > > retval = end; > dentry = parent; > } >- if (!(seq & 1)) >- rcu_read_unlock(); >- if (need_seqretry(&rename_lock, seq)) { >- seq = 1; >- goto restart; >- } >- done_seqretry(&rename_lock, seq); >- if (error) >- goto Elong; > return retval; > Elong: > return ERR_PTR(-ENAMETOOLONG); >@@ -3158,7 +3078,13 @@ Elong: > > char *dentry_path_raw(struct dentry *dentry, char *buf, int buflen) > { >- return __dentry_path(dentry, buf, buflen); >+ char *retval; >+ >+ write_seqlock(&rename_lock); >+ retval = __dentry_path(dentry, buf, buflen); >+ write_sequnlock(&rename_lock); >+ >+ return retval; > } > EXPORT_SYMBOL(dentry_path_raw); > >@@ -3167,6 +3093,7 @@ char *dentry_path(struct dentry *dentry, char *buf, int buflen) > char *p = NULL; > char *retval; > >+ write_seqlock(&rename_lock); > if (d_unlinked(dentry)) { > p = buf + buflen; > if (prepend(&p, &buflen, "//deleted", 10) != 0) >@@ -3174,6 +3101,7 @@ char *dentry_path(struct dentry *dentry, char *buf, int buflen) > buflen++; > } > retval = __dentry_path(dentry, buf, buflen); >+ write_sequnlock(&rename_lock); > if (!IS_ERR(retval) && p) > *p = '/'; /* restore '/' overriden with '\0' */ > return retval; >@@ -3225,6 +3153,7 @@ SYSCALL_DEFINE2(getcwd, char __user *, buf, unsigned long, size) > > error = -ENOENT; > br_read_lock(&vfsmount_lock); >+ write_seqlock(&rename_lock); > if (!d_unlinked(pwd.dentry)) { > unsigned long len; > char *cwd = page + PATH_MAX; >@@ -3232,6 +3161,7 @@ SYSCALL_DEFINE2(getcwd, char __user *, buf, unsigned long, size) > > prepend(&cwd, &buflen, "\0", 1); > error = prepend_path(&pwd, &root, &cwd, &buflen); >+ write_sequnlock(&rename_lock); > br_read_unlock(&vfsmount_lock); > rcu_read_unlock(); > >@@ -3253,6 +3183,7 @@ SYSCALL_DEFINE2(getcwd, char __user *, buf, unsigned long, size) > error = -EFAULT; > } > } else { >+ write_sequnlock(&rename_lock); > br_read_unlock(&vfsmount_lock); > rcu_read_unlock(); > } >-- >1.8.1.5 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 488492
: 363178