Messages in this thread |  | | | Date | Fri, 17 Sep 2004 09:55:41 +0200 (CEST) | | From | Simon Derr <> | | Subject | Re: [Patch] cpusets: fix race in cpuset_add_file() |
| |
On Fri, 17 Sep 2004, Paul Jackson wrote:
> You can continue to ignore this patch, Andrew. I'm still thinking it > through with Simon. > > Here's another possible way to skin this cat, Simon. > > Instead of adding an inode lock, how about just extending the cpuset_sem > window. If we hold cpuset_sem for the entire cpuset_mkdir() operation, > then no other cpuset_mkdir can overlap, and there should be no > confused overlapping directory creations. > no - the inode lock is necessary.
don't let my second mail with the deadlock example confuse you : the original problem (i.e the problem my patch fixes) is a race between a cpuset mkdir() and another operation in the newly created directory, for instance just `ls'.
the race is:
mkdir a/b | ls a/b/cpus | cpuset_add_file(b, "cpus") | vfs_stat() cpuset_get_dentry(b, "cpus") | user_path_walk() lookup_hash(b, "cpus") | path_lookup() cached_lookup(b, "cpus") | link_path_walk() d_alloc(b, "cpus") | do_lookup() | real_lookup() | down(b->i_sem); | d_lookup(b, "cpus"); | d_alloc(b, "cpus");
The result is that `ls' and `mkdir' both create a dentry for a/b/cpus, and the dentry created by `ls' is bogus since it does not point to the cpuset data.
The proper way to prevent this is to lock the i_sem of directory b. This can be done in cpuset_populate_dir(), in cpuset_add_file(), or cpuset_get_dentry().
The similar piece of code in sysfs does it in add_file().
If your are not convinced try the following script. Without my patch it triggers the bug in a few seconds.
Simon.
#! /bin/bash
a() { name=$1 echo dir is /dev/cpuset/$name while :; do mkdir /dev/cpuset/$name if ! test -r /dev/cpuset/$name/cpus; then echo missing /dev/cpuset/$name/cpus exit 1 fi rmdir /dev/cpuset/$name done } b() { name=$1 while :; do test -r /dev/cpuset/$name/cpus done } p=$$
b $p & a $p - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |