From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Kent Subject: Re: [autofs] [RFC PATCH]autofs4: hang and proposed fix Date: Sat, 19 Nov 2005 10:52:44 +0800 (WST) Message-ID: References: <20051116101740.GA9551@RAM> <1132159817.5720.33.camel@localhost> <1132192362.5720.163.camel@localhost> <437CD7D2.40003@us.ibm.com> <437DF12A.5050805@us.ibm.com> <437E0B62.4000306@us.ibm.com> <1132340265.5720.191.camel@localhost> <437E34D9.4050102@us.ibm.com> Mime-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Cc: Ram Pai , autofs mailing list , linux-fsdevel Return-path: Received: from wombat.indigo.net.au ([202.0.185.19]:7436 "EHLO wombat.indigo.net.au") by vger.kernel.org with ESMTP id S1161213AbVKSCxU (ORCPT ); Fri, 18 Nov 2005 21:53:20 -0500 To: "William H. Taber" In-Reply-To: <437E34D9.4050102@us.ibm.com> Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Fri, 18 Nov 2005, William H. Taber wrote: > Ram Pai wrote: > > On Fri, 2005-11-18 at 09:12, William H. Taber wrote: > > > > > I think the problem is with cached_lookup(). It is the only place which > > calls ->revalidate() holding the parent's inode-semaphore AFAICT. > > > > note: cached_lookup() is only called from __lookup_hash() and > > __lookup_hash() is always called holding the semaphore. > > > > VFS experts agree? > > RP > > > Ram, > Lookup_one_len calls lookup_hash and it is the callers of lookup_one_len that > are problematical. Just as an example, lookup_one_len is called from > nfs_sillyrename which is called, among other places in the nfs_rename code. > In that path the parent i_sem is obtained in do_rename in the vfs code > (namei.c). I would think that it would be extremely difficult to to change > that usage. The alternative is to move the obtaining of the parent i_sem from > real_lookup to do_lookup. We would also have to put the locking around the > d_revalidate call at return_reval in __link_path_walk. > Perhaps we are making this altogether to complicated. I'm sure that there are good reasons for the locking being the way it is and any attempt to change it is likely to be a disaster. So what about solving this by defining a usage policy based on the intent of the functions concerned. For example. The lookup_one_len a special use funtion to return the dentry corresponding to a path element and by definition it does not follow mounts or symlinks. To function correctly autofs needs to follow mounts and some time soon I will be posting patches that will use the the follow_link method as well. So the policy could be that if autofs revalidate is called with the directory inode semaphore held it must validate the autofs dentry itself and not cause a mount request be triggered. The responsibility then moves to the filesystem to check if the dentry is an autofs dentry and to decide if it needs to then make an unlocked revalidate call. It is easy enough to check if the semaphore is held the autofs module. The filesystem check is easy enough to do once the filesystem magic number is moved to one of the common autofs header files. Thoughts? Ian