From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755689Ab0BKAyx (ORCPT ); Wed, 10 Feb 2010 19:54:53 -0500 Received: from cantor2.suse.de ([195.135.220.15]:43656 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755364Ab0BKAyw (ORCPT ); Wed, 10 Feb 2010 19:54:52 -0500 Date: Wed, 10 Feb 2010 15:06:25 -0800 From: Greg KH To: Neil Brown Cc: "Eric W . Biederman" , Tejun Heo , linux-kernel@vger.kernel.org Subject: Re: [PATCH] sysfs: differentiate between locking links and non-links Message-ID: <20100210230625.GB678@suse.de> References: <19314.1869.847327.15190@notabene.brown> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <19314.1869.847327.15190@notabene.brown> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Feb 10, 2010 at 12:09:33PM +1100, Neil Brown wrote: > > Hi, > I've just spent a while sorting out some lockdep complaints triggered > by the recent addition of the "s_active" lockdep annotation in sysfs > (commit 846f99749ab68bbc7f75c74fec305de675b1a1bf) > > Some of them are genuine and I have submitted a fix for those. > Some are, I think, debatable and I get to that is a minute. I've > submitted a fix for them anyway. > But some are to my mind clearly bogus and I'm hoping that can be > fixed by the change below (or similar). > The 'bogus' ones are triggered by writing to a sysfs attribute file > for which the handler tries to delete a symlink from sysfs. > This appears to be a recursion on s_active as s_active is held while > the handler runs and is again needed to effect the delete. However > as the thing being deleted is a symlink, it is very clearly a > different object to the thing triggering the delete, so there is no > real loop. > > The following patch splits the lockdep context in two - one for > symlink and one for everything else. This removes the apparent loop. > (An example report can be seen in > http://bugzilla.kernel.org/show_bug.cgi?id=15142). > > The "debatable" dependency loops happen when writing to one attribute > causes a different attribute to be deleted. In my (md) case this can > actually cause a deadlock as both the attributes take the same lock > while the handler is running. This is because deleting the attribute > will block until the all accesses of that attribute have completed (I > think). > However it should be possible to delete a name from sysfs while there > are still accesses pending (it works for normal files!!). So if > sysfs could be changed to simply unlink the file and leave deletion to > happen when the refcount become zero it would certainly make my life > a lot easier, and allow the removal of some ugly code from md.c. > I don't know sysfs well enough to suggest a patch though. > > Thanks, > NeilBrown > > > > commit 2e502cfe444b68f6ef6b8b2abe83b6112564095b > Author: NeilBrown > Date: Wed Feb 10 09:43:45 2010 +1100 > > sysfs: differentiate between locking links and non-links for sysfs > > symlinks and non-symlink is sysfs are very different. > A symlink can never be locked (active) while an attribute > modification routine is running. So removing symlink from an > attribute 'store' routine should be permitted without any lockdep > warnings. > > So split the lockdep context for 's_active' in two, one for symlinks > and other for everything else. > > Signed-off-by: NeilBrown Nice patch, I'll queue it up for .34. thanks, greg k-h