On Sat, 2006-01-07 at 03:36 -0800, Andrew Morton wrote:
>
> Aren't there other places where we need to take this lock? Code which
> traverses that list, code which adds things to it?
>
Andrew,
How's this patch look? I tested this with the following module:
http://www.kihontech.com/tests/proc/proc_stress.c
Without the patch, I could hang the processes (the processes would
either crash, or just get stuck spinning inside the list). With the
patch, the module ran to completion each time.
I don't believe any of the calls are called from interrupt context, so I
held off from using the _irqsave versions of spin_lock.
-- Steve
Index: linux-2.6.15/fs/proc/generic.c
===================================================================
--- linux-2.6.15.orig/fs/proc/generic.c 2006-01-09 13:58:23.000000000 -0500
+++ linux-2.6.15/fs/proc/generic.c 2006-01-09 13:58:34.000000000 -0500
@@ -19,6 +19,7 @@
#include <linux/idr.h>
#include <linux/namei.h>
#include <linux/bitops.h>
+#include <linux/spinlock.h>
#include <asm/uaccess.h>
static ssize_t proc_file_read(struct file *file, char __user *buf,
@@ -27,6 +28,8 @@
size_t count, loff_t *ppos);
static loff_t proc_file_lseek(struct file *, loff_t, int);
+DEFINE_SPINLOCK(proc_subdir_lock);
+
int proc_match(int len, const char *name, struct proc_dir_entry *de)
{
if (de->namelen != len)
@@ -275,7 +278,9 @@
const char *cp = name, *next;
struct proc_dir_entry *de;
int len;
+ int rtn = 0;
+ spin_lock(&proc_subdir_lock);
de = &proc_root;
while (1) {
next = strchr(cp, '/');
@@ -287,13 +292,17 @@
if (proc_match(len, cp, de))
break;
}
- if (!de)
- return -ENOENT;
+ if (!de) {
+ rtn = -ENOENT;
+ goto out;
+ }
cp += len + 1;
}
*residual = cp;
*ret = de;
- return 0;
+out:
+ spin_unlock(&proc_subdir_lock);
+ return rtn;
}
static DEFINE_IDR(proc_inum_idr);
@@ -378,6 +387,7 @@
int error = -ENOENT;
lock_kernel();
+ spin_lock(&proc_subdir_lock);
de = PDE(dir);
if (de) {
for (de = de->subdir; de ; de = de->next) {
@@ -386,12 +396,15 @@
if (!memcmp(dentry->d_name.name, de->name, de->namelen)) {
unsigned int ino = de->low_ino;
+ spin_unlock(&proc_subdir_lock);
error = -EINVAL;
inode = proc_get_inode(dir->i_sb, ino, de);
+ spin_lock(&proc_subdir_lock);
break;
}
}
}
+ spin_unlock(&proc_subdir_lock);
unlock_kernel();
if (inode) {
@@ -445,11 +458,13 @@
filp->f_pos++;
/* fall through */
default:
+ spin_lock(&proc_subdir_lock);
de = de->subdir;
i -= 2;
for (;;) {
if (!de) {
ret = 1;
+ spin_unlock(&proc_subdir_lock);
goto out;
}
if (!i)
@@ -459,12 +474,16 @@
}
do {
+ /* filldir passes info to user space */
+ spin_unlock(&proc_subdir_lock);
if (filldir(dirent, de->name, de->namelen, filp->f_pos,
de->low_ino, de->mode >> 12) < 0)
goto out;
+ spin_lock(&proc_subdir_lock);
filp->f_pos++;
de = de->next;
} while (de);
+ spin_unlock(&proc_subdir_lock);
}
ret = 1;
out: unlock_kernel();
@@ -498,9 +517,13 @@
if (i == 0)
return -EAGAIN;
dp->low_ino = i;
+
+ spin_lock(&proc_subdir_lock);
dp->next = dir->subdir;
dp->parent = dir;
dir->subdir = dp;
+ spin_unlock(&proc_subdir_lock);
+
if (S_ISDIR(dp->mode)) {
if (dp->proc_iops == NULL) {
dp->proc_fops = &proc_dir_operations;
@@ -692,6 +715,8 @@
if (!parent && xlate_proc_name(name, &parent, &fn) != 0)
goto out;
len = strlen(fn);
+
+ spin_lock(&proc_subdir_lock);
for (p = &parent->subdir; *p; p=&(*p)->next ) {
if (!proc_match(len, fn, *p))
continue;
@@ -712,6 +737,7 @@
}
break;
}
+ spin_unlock(&proc_subdir_lock);
out:
return;
}
Index: linux-2.6.15/fs/proc/proc_devtree.c
===================================================================
--- linux-2.6.15.orig/fs/proc/proc_devtree.c 2006-01-09 13:58:23.000000000 -0500
+++ linux-2.6.15/fs/proc/proc_devtree.c 2006-01-09 14:05:10.000000000 -0500
@@ -112,9 +112,11 @@
* properties are quite unimportant for us though, thus we
* simply "skip" them here, but we do have to check.
*/
+ spin_lock(&proc_subdir_lock);
for (ent = de->subdir; ent != NULL; ent = ent->next)
if (!strcmp(ent->name, pp->name))
break;
+ spin_unlock(&proc_subdir_lock);
if (ent != NULL) {
printk(KERN_WARNING "device-tree: property \"%s\" name"
" conflicts with node in %s\n", pp->name,
Index: linux-2.6.15/include/linux/proc_fs.h
===================================================================
--- linux-2.6.15.orig/include/linux/proc_fs.h 2006-01-09 08:59:13.000000000 -0500
+++ linux-2.6.15/include/linux/proc_fs.h 2006-01-09 14:07:09.000000000 -0500
@@ -4,6 +4,7 @@
#include <linux/config.h>
#include <linux/slab.h>
#include <linux/fs.h>
+#include <linux/spinlock.h>
#include <asm/atomic.h>
/*
@@ -92,6 +93,8 @@
extern struct proc_dir_entry *proc_root_driver;
extern struct proc_dir_entry *proc_root_kcore;
+extern spinlock_t proc_subdir_lock;
+
extern void proc_root_init(void);
extern void proc_misc_init(void);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
