From mboxrd@z Thu Jan 1 00:00:00 1970 From: ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman) Subject: Re: [PATCH 06/14] sysfs: Rewrite sysfs_get_dentry Date: Tue, 31 Jul 2007 13:24:37 -0600 Message-ID: References: <46AF15F6.6010202@suse.de> <46AF1E57.3030209@suse.de> <20070731141613.GC13674@htj.dyndns.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: In-Reply-To: <20070731141613.GC13674-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org> (Tejun Heo's message of "Tue, 31 Jul 2007 23:16:13 +0900") List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Tejun Heo Cc: Linux Containers , Greg KH , Greg KH , Dave Hansen List-Id: containers.vger.kernel.org Tejun Heo writes: > On Tue, Jul 31, 2007 at 08:34:47PM +0900, Tejun Heo wrote: >> > If sysfs_mutex nested the other way things would be easier, >> > and we could grab all of the i_mutexes we wanted. I wonder if we can >> > be annoying in sysfs_lookup and treat that as the lock inversion >> > case using mutex_trylock etc. And have sysfs_mutex be on the >> > outside for the rest of the cases? >> >> The problem with treating sysfs_lookup as inversion case is that vfs >> layer grabs i_mutex outside of sysfs_lookup. Releasing i_mutex from >> inside sysfs_lookup would be a hacky layering violation. >> >> Then again, the clean up which can come from the new sysfs_looukp_dentry >> is very significant. I'll think about it a bit more. > > How about something like this? __sysfs_get_dentry() never creates any > dentry, it just looks up existing ones. sysfs_get_dentry() calls > __sysfs_get_dentry() and if it fails, it builds a path string and look > up using regular vfs_path_lookup(). Once in the creation path, > sysfs_get_dentry() is allowed to fail, so allocating path buf is fine. > > It still needs to retry when vfs_path_lookup() returns -ENOENT or the > wrong dentry but things are much simpler now. It doesn't violate any > VFS locking rule while maintaining all the benefits of > sysfs_get_dentry() cleanup. > > Something like LOOKUP_KERNEL is needed to ignore security checks; > otherwise, we'll need to resurrect lookup_one_len_kern() and open code > look up. > > The patch is on top of all your patches and is in barely working form. Thanks. I need to look some more. I've got a case where __sysfs_get_dentry called from sysfs_drop_dentry is not successfully walking of the sysfs_dirent tree to the initial root directory. (This is with deleted sysfs_dirents but that expected from the context). I haven't yet had a chance to dig in and see what is going on yet. Your patch doesn't touch that part of my logic so I don't know yet what is going on. Eric