From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tejun Heo Subject: Re: [PATCH 04/20] sysfs: Handle the general case of removing of directories with subdirectories Date: Thu, 21 May 2009 17:37:54 +0900 Message-ID: <4A1512E2.2040505@kernel.org> References: <1242865694-2100-1-git-send-email-ebiederm@xmission.com> <1242865694-2100-2-git-send-email-ebiederm@xmission.com> <1242865694-2100-3-git-send-email-ebiederm@xmission.com> <1242865694-2100-4-git-send-email-ebiederm@xmission.com> <4A14F356.3030501@kernel.org> <4A15046A.10106@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Andrew Morton , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Cornelia Huck , linux-fsdevel@vger.kernel.org, "Eric W. Biederman" To: "Eric W. Biederman" Return-path: Received: from hera.kernel.org ([140.211.167.34]:56352 "EHLO hera.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751639AbZEUIix (ORCPT ); Thu, 21 May 2009 04:38:53 -0400 In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Hi, Eric W. Biederman wrote: >>> The locking is correct, something that is much more difficult to >>> tell with your version. >> Why? :-) > > Because mine is all in a single place and there is no optimization > to get locks I don't need. > > Unless I have misread your patch you are failing to get the > i_mutex for child directories, if it possible to get it. > > Something that it is trivial to see that I always do correctly. > Simply because the distance between the lock and where I depend on > it is so small. If this patch series works out, we don't need to grab i_mutexes while manipulating sd's, right? >>> I would prefer to only remove empty directories. But when I >>> instrumented things up I found cases where that does indeed happen. >> IIRC, my version did the whole thing while holding sysfs_mutex, so >> it's safe against such races. I can't really see why ops like this >> can't be atomic in sysfs. I don't really care how things are done but >> please make it atomic. > > Nope. Holding the sysfs_mutex does not make you safe from such races. > It actually makes you more prone to someone adding a directory entry to > a deleted directory and not having it deleted. I have a chance of > deleting the added directory entry. > > The problem is that sysfs_add_one takes to sysfs_dirents. The look up > of the directory is done before we take the sysfs_mutex. So the > sysfs_dirent could be grabbed at any time. Well, it can be trivially fixed by checking the removed flag. The add/rm thing is designed to help additions and removals of multiple nodes at one go and I'd really like to see it working that way. Any chance you can change code toward that direction? Thanks. -- tejun