Messages in this thread | | | Date | Thu, 30 Apr 2009 11:18:28 +0200 | From | Louis Rilling <> | Subject | Re: [PATCH 2/2] configfs: Rework configfs_depend_item() locking and make lockdep happy |
| |
On 29/04/09 11:52 -0700, Joel Becker wrote: > On Wed, Jan 28, 2009 at 07:18:33PM +0100, Louis Rilling wrote: > > configfs_depend_item() recursively locks all inodes mutex from configfs root to > > the target item, which makes lockdep unhappy. The purpose of this recursive > > locking is to ensure that the item tree can be safely parsed and that the target > > item, if found, is not about to leave. > > > > This patch reworks configfs_depend_item() locking using configfs_dirent_lock. > > Since configfs_dirent_lock protects all changes to the configfs_dirent tree, and > > protects tagging of items to be removed, this lock can be used instead of the > > inodes mutex lock chain. > > This needs that the check for dependents be done atomically with > > CONFIGFS_USET_DROPPING tagging. > > These patches are now in the 'lockdep' branch of the configfs > tree. I'm planning to send them in the next merge window. I've made > one change. > > > + * Note: items in the middle of attachment start with s_type = 0 > > + * (configfs_new_dirent()), and configfs_make_dirent() (called from > > + * create_dir()) sets s_type = CONFIGFS_DIR|CONFIGFS_USET_CREATING. In both > > + * cases the item is ignored. Since s_type is an int, we rely on the CPU to > > + * atomically update the value, without making configfs_make_dirent() take > > + * configfs_dirent_lock. > > I've added configfs_dirent_lock in configfs_make_dirent(), > because it is not safe at all to rely on the fact that s_type is an int. > It's an atomic set on one CPU, but there's no guarantee that it's seen > correctly on other CPUs. Plus, there's no real need for speed here. So > we properly take configfs_dirent_lock around s_type in > configfs_make_dirent(), and that ensures we see things correctly on SMP.
Agreed, I was going to suggest something like this. Actually I'd push the initialization of s_type down to configfs_new_dirent(), so that s_type either is always NULL, or always shows the correct type of object.
Thanks,
Louis
-- Dr Louis Rilling Kerlabs Skype: louis.rilling Batiment Germanium Phone: (+33|0) 6 80 89 08 23 80 avenue des Buttes de Coesmes http://www.kerlabs.com/ 35700 Rennes [unhandled content-type:application/pgp-signature] | |