From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161411AbcKACCL (ORCPT ); Mon, 31 Oct 2016 22:02:11 -0400 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:41750 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161347AbcKACCJ (ORCPT ); Mon, 31 Oct 2016 22:02:09 -0400 X-ME-Sender: X-Sasl-enc: O/nhPL54VLr6viO6f5bQZIcoG1G9fNVwZS9Xc+NcSwWH 1477965727 Message-ID: <1477965723.2798.13.camel@themaw.net> Subject: Re: [PATCH 1/8] vfs - change d_manage() to take a struct path From: Ian Kent To: Al Viro Cc: Andrew Morton , autofs mailing list , Kernel Mailing List , "Eric W. Biederman" , linux-fsdevel , Omar Sandoval Date: Tue, 01 Nov 2016 10:02:03 +0800 In-Reply-To: <1477551028.2898.25.camel@themaw.net> References: <20161011053352.27645.83962.stgit@pluto.themaw.net> <20161019124031.94a44fbdd0970989f05d1893@linux-foundation.org> <1477006776.3207.14.camel@themaw.net> <20161027021111.GI19539@ZenIV.linux.org.uk> <1477536470.2898.12.camel@themaw.net> <1477551028.2898.25.camel@themaw.net> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.2 (3.18.5.2-1.fc23) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2016-10-27 at 14:50 +0800, Ian Kent wrote: > On Thu, 2016-10-27 at 10:47 +0800, Ian Kent wrote: > > > > On Thu, 2016-10-27 at 03:11 +0100, Al Viro wrote: > > > > > >   > > > > > > How much testing did it get?  I've several test setups involving > > > autofs, but they are nowhere near exhaustive and I don't have good > > > enough feel of the codebase to slap together something with decent > > > coverage... > > It got my standard testing. > > > > For that I use a modified version of the autofs Connectathon system. > > > > It's more about testing a wide variety of syntax and map setups and so > > exercises > > a large number of different types of autofs mounts. > > > > It's meant to check normal operation but not so much stress testing even > > though > > it does perform quite a few mounts (around 250-300, not to mention the > > autofs > > mounts themselves). > > > > I have another standard test I call the submount-test and it was originally > > done > > to stress test the most common problem I see, concurrent expire to mount. > > > > I didn't see any problems I couldn't explain in these but I might need to > > re- > > visit the submount-test to see if it is still doing what I want. > > > > OTOH, the pattern of mount and umount I see when the submount-test is run > > does > > look like it is doing what I want but it might not be getting all the way to > > the > > top of the tree of mounts enough times over the course of the test. > > > > So I'm happy with my testing, just not as happy as I could be. > Well, almost happy with my testing. > > Naturally I also tested the specific case this series is meant to fix. > > Basically: > ls /mnt/foo            # do the initial automount > unshare -m sleep 10 &  # hold the automount in a new namespace > umount /mnt/foo        # pretend the mount timed out > ls /mnt/foo            # try to access it again > ls: cannot open directory '/mnt/foo': Too many levels of symbolic links > > as seen on the autofs mailing list. My specific test was a little different > but > verified this was resolved. > > Now that Al seems reasonably OK with the series, with some changes, I'll test > some other use cases, mainly to verify the expire still functions as required. > That might need more work. I have done some further tests, specifically for (what I believe are) the two most common use cases. First, using automount(8) entirely within a container, as expected works fine. But the second case, one where automount(8) is run in the root namespace and has automount directories bound into a container does have a problem. The problem is due to may_umount_tree() only considering mounts in the root namespace and leads to expire attempts on mounts even if they are in use in another namespace. It's not a serious problem as the umount attempt fails because the mount is busy but it would be good to avoid the call back overhead. Unfortunately it looks like transforming may_umount_tree() to use a similar check to may_umount() introduces a race (picked up by my submount-test) which I'm struggling to understand, I'll continue to work on it. Ian