Re: [RFC] [PATCH 1/5] cgroups: revamp subsys array

[Ben Blum <bblum@andrew.cmu.edu>]

On Tue, Dec 08, 2009 at 03:38:43PM +0800, Li Zefan wrote:
> > @@ -1291,6 +1324,7 @@ static int cgroup_get_sb(struct file_system_type *fs_type,
> >  	struct cgroupfs_root *new_root;
> >  
> >  	/* First find the desired set of subsystems */
> > +	down_read(&subsys_mutex);
> 
> Hmm.. this can lead to deadlock. sget() returns success with sb->s_umount
> held, so here we have:
> 
> 	down_read(&subsys_mutex);
> 
> 	down_write(&sb->s_umount);
> 
> On the other hand, sb->s_umount is held before calling kill_sb(),
> so when umounting we have:
> 
> 	down_write(&sb->s_umount);
> 
> 	down_read(&subsys_mutex);

Unless I'm gravely mistaken, you can't have deadlock on an rwsem when
it's being taken for reading in both cases? You would have to have at
least one of the cases being down_write.

In fairness to readability, perhaps subsys_mutex should instead be
subsys_rwsem? It seemed to me to be that calling it "mutex" was
conventional anyway.

-- bblum