From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D6362163 for ; Wed, 5 Feb 2025 00:07:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.174 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738714052; cv=none; b=F8si8udFr66eSCkPIft9R1K4X6l2pHqTsfiszs81+gjv5wLod3QzO3gkp6e0ZXGhx8s7x/HzKWYnsTmzZ+wfKrCPDEaQ/9r4UtWcW/9qnuPyKs/zYoIo9//WKjmL/Nrw61iEyHvxKAHgwWk0Rx7SNRtHu2iYArYnSkt4kWX7VRo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738714052; c=relaxed/simple; bh=ANtLj6RIbWy6XzHdQEw3GfN8T4f5//7k96FZf/3F7So=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=e87fSbCceVFEwGOQhpkBNJqijZYFRadULCWlGirYzG7R+51KA4AufDGqiLVJFTlAnglnfV4lWORbO6U0NRIQKIgtcstoS0BalQ3O5qLy8tvjpnCTazwW/3m2bEiLdOsM83LO8TPc/WL5krziH3Rv20kQQM5cWuehGzm7G8di5Ig= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=fromorbit.com; spf=pass smtp.mailfrom=fromorbit.com; dkim=pass (2048-bit key) header.d=fromorbit-com.20230601.gappssmtp.com header.i=@fromorbit-com.20230601.gappssmtp.com header.b=rCAxfnHv; arc=none smtp.client-ip=209.85.214.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=fromorbit.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=fromorbit.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=fromorbit-com.20230601.gappssmtp.com header.i=@fromorbit-com.20230601.gappssmtp.com header.b="rCAxfnHv" Received: by mail-pl1-f174.google.com with SMTP id d9443c01a7336-21670dce0a7so129107385ad.1 for ; Tue, 04 Feb 2025 16:07:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20230601.gappssmtp.com; s=20230601; t=1738714050; x=1739318850; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=8fj0rH4qbIkbkk6TgTl2qCvtRslxxK8xPGVrgdKhZjE=; b=rCAxfnHvPdR0vo3kXcxfW/zLi3c+XXf0pRtyiw9+f2xzHD1biIB3L7yeTsZRiUqMT2 1jBuufEuRqWaRXkylKDpOxXmrWBY2lW0EaaLSuIRRkLspDJ2YFhWngiluELFrP2HWKUL tifgmfKUMy+9t1WSGmPIjhoQCsFKLjs0HeegqHYBOQZ4SBlCyyQu/8traO04ohfBHJx3 l6+cmBeFk4Uqw2UbJlmVd6mNPsdq2gdD5R6Ov8GhMIyF5ETinZZFwPB5AZg6bxBfghE1 nJyP4jUpwlhNUeSwiEimu17ax9kyoSiaJ0yaVjD5odJxY6N7l2oUjwNCFLObp3/nfVRP WmXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738714050; x=1739318850; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=8fj0rH4qbIkbkk6TgTl2qCvtRslxxK8xPGVrgdKhZjE=; b=p8hovj0edWC0ZHj7XE0kNWiO77M3B5HxByxjD2uzPahhbrqN6ksc5LFrMMDewwhaac BHBudjt2YLlQckYq/8F3AtTCbXJsTe849CiNGK80WQ3KW6yxr04GFIzUk4+bR2qqrgqR ucVga46uXLhKJYZdw0uVT5iNdjJPvEO1hWi6Chj4frCKSFJdlG3GDKweq8wSB84DxYBp uygpIjU91MGEh8KOlGUqe+ONC/5q7frkK6MUtrbdgqHTqAPt04SZWjDcEBYgzZHvSgPj 9vrpUSobVs0XqxDOEDciQCPEtM1GDw4GaR2H+roL09gSaaM9FW4qiexVY0KtvGDK9Il/ RS7w== X-Forwarded-Encrypted: i=1; AJvYcCX8xjGyACPgPuy395PdZRsW5Kd18yj80W6gSK7OdyNgiHgaAmtBikIs4XE6JIaxrT5Dsw9OgXz7@vger.kernel.org X-Gm-Message-State: AOJu0YwqihUn27NbgNuf0QWUvdTn/xGU49vyKOS8qwOOf56WD1DiyTS5 FJW3DlBtxAn5+VBL0EqlfZ9TOxQEcIRQmFDWWx8I+eX5uP2Z3k7KoONjiG3QPMk= X-Gm-Gg: ASbGncs/zilGlXgwAsAbG1jHRg2/Nvgujei3qd24Dsm+8jGZRXQjW+tu+mvrorKeAio NzjwhOtV+GpyFT1K0J37gea7U2SOvo8R3F5hF1w2MIb3Ehdwt9PcJUcATna7p2iuybNCUGN1LaM THSxtZBYoAvqbrhLrA0TfyrCWxg2Jg1HkcTjuSFoBpI0Vu8QoKSaUdCEsoLVFN0XOby+yR6/2iY 4xDO7NgZZUcyrWP2mu3CpnJGlXXvRKR+eBdpNz+Y0AmjuwyUEhMoYXXznaebLyNvioEi+65/hyS VChlO5m1a+a6hepTZOF3MRKhd79dIOMyOoaFL4RqvTcKa7SRMTGJVjnQ X-Google-Smtp-Source: AGHT+IHdE2VVS12iE2YllVRBGqqkFrNxzGLqng7/tVH3iPmFSttAe7R4c7qJJmfD+/aCeMdUxJNKCg== X-Received: by 2002:a17:903:8c4:b0:215:a3fd:61f5 with SMTP id d9443c01a7336-21f17dde0c8mr12110915ad.5.1738714050013; Tue, 04 Feb 2025 16:07:30 -0800 (PST) Received: from dread.disaster.area (pa49-186-89-135.pa.vic.optusnet.com.au. [49.186.89.135]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-21de31f6f74sm102391075ad.67.2025.02.04.16.07.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Feb 2025 16:07:29 -0800 (PST) Received: from dave by dread.disaster.area with local (Exim 4.98) (envelope-from ) id 1tfSww-0000000EikI-2nFd; Wed, 05 Feb 2025 11:07:26 +1100 Date: Wed, 5 Feb 2025 11:07:26 +1100 From: Dave Chinner To: "Darrick J. Wong" Cc: zlang@redhat.com, fstests@vger.kernel.org, linux-xfs@vger.kernel.org Subject: Re: [PATCH 06/34] common/rc: revert recursive unmount in _clear_mount_stack Message-ID: References: <173870406063.546134.14070590745847431026.stgit@frogsfrogsfrogs> <173870406199.546134.3521494633346683975.stgit@frogsfrogsfrogs> Precedence: bulk X-Mailing-List: fstests@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <173870406199.546134.3521494633346683975.stgit@frogsfrogsfrogs> On Tue, Feb 04, 2025 at 01:23:51PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong > > Zorro complained about the following regression in generic/589 that I > can't reproduce here on Debian 12: .... > > Orignal _clear_mount_stack trys to umount all of them. But Dave gave -R option to > > umount command, so > > `umount -R /mnt/test/589-dst/201227_mpC` and `umount -R /mnt/test/589-src/201227_mpA` > > already umount /mnt/test/589-dst and /mnt/test/589-src. recursively. > > Then `umount -R /mnt/test/589-dst` shows "not mounted". > > I /think/ this is a result of commit 4c6bc4565105e6 performing this > "conversion" in _clear_mount_stack: > > - $UMOUNT_PROG $MOUNTED_POINT_STACK > + _unmount -R $MOUNTED_POINT_STACK > > This is not a 1:1 conversion here -- previously there was no > -R(ecursive) unmount option, and now there is. It looks as though > umount parses /proc/self/mountinfo to figure out what to unmount, and > maybe on Zorro's system it balks if the argument passed is not present > in that file? Debian 12's umount doesn't care. Ah, I think I added that whilst trying to work out the weird shenanigans that the mount namespace cloning that these tests did caused. It was cloning a shared mount namespace that many tests interacted and so any changes to the mount namespace inside that test were visible everywhere. If two of these mount namespace tests ran at the same time, they stepped on each other and they failed to unmount their own stack cleanly because of "mount busy at unmount" errors. This would then cascade into other test failures because the test/scratch devices couldn't be unmounted. The fix I came up with was to run a recursive unmount for the stack, which seemed to clear all the mounts the test made regardless of whatever other test could see them. It didn't through any errors on my test machines (debian unstable) so I promptly forgot about it. > Regardless, there was no justification for this change in behavior that > was buried in what is otherwise a hoist patch, so revert it. The author > can resubmit the change with proper documentation. I suspect that changes made later on (i.e. private mount namespaces per runner instance) fixed the underlying namespace interaction problem and made the recursive unmount unnecessary. The lack of errors from it meant I likely forgot about it before I posted the initial RFC that was merged... > > Cc: # v2024.12.08 > Fixes: 4c6bc4565105e6 ("fstests: clean up mount and unmount operations") > Signed-off-by: "Darrick J. Wong" > --- > common/rc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > > diff --git a/common/rc b/common/rc > index 4658e3b8be747f..07646927bad523 100644 > --- a/common/rc > +++ b/common/rc > @@ -334,7 +334,7 @@ _put_mount() > _clear_mount_stack() > { > if [ -n "$MOUNTED_POINT_STACK" ]; then > - _unmount -R $MOUNTED_POINT_STACK > + _unmount $MOUNTED_POINT_STACK > fi > MOUNTED_POINT_STACK="" > } That doesn't appear to cause any problems on my current check-parallel stack, so I think the above explains how it got there. Reviewed-by: Dave Chinner -- Dave Chinner david@fromorbit.com