From owner-freebsd-fs@FreeBSD.ORG Sat Oct 1 19:44:05 2011 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 687F0106566B; Sat, 1 Oct 2011 19:44:05 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-qw0-f54.google.com (mail-qw0-f54.google.com [209.85.216.54]) by mx1.freebsd.org (Postfix) with ESMTP id F17F68FC16; Sat, 1 Oct 2011 19:44:04 +0000 (UTC) Received: by qadz30 with SMTP id z30so1212829qad.13 for ; Sat, 01 Oct 2011 12:44:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=UWP9IV5p30gm43Lb8zIlr3ygNEORjCtrWPAdAzMRxt8=; b=F6t6NKZjyCNIy+BEnL9BnqTZ7Vkn/fB+OxX18mitAkP8JdaOWu8BRp9AAlTNjUJ4vj k0LWxTkCNrxp/CjYGTuZ6gO+0uL0yjsXrk/xgTuzwV6L8NcNlnM6J016brsVCqAkaIwQ MSN+fPxAUBQSDFTSDcB04jPkCma8RKY6PmIbs= MIME-Version: 1.0 Received: by 10.224.215.133 with SMTP id he5mr9861274qab.224.1317498244353; Sat, 01 Oct 2011 12:44:04 -0700 (PDT) Received: by 10.224.74.82 with HTTP; Sat, 1 Oct 2011 12:44:04 -0700 (PDT) In-Reply-To: References: <201109301820.p8UIKSGj039445@chez.mckusick.com> <20110930201851.GB1511@deviant.kiev.zoral.com.ua> Date: Sat, 1 Oct 2011 12:44:04 -0700 Message-ID: From: Garrett Cooper To: Attilio Rao Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Kirk McKusick , Xin LI , freebsd-fs@freebsd.org Subject: Re: Need to force sync(2) before umounting UFS1 filesystems? X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 01 Oct 2011 19:44:05 -0000 On Sat, Oct 1, 2011 at 5:39 AM, Attilio Rao wrote: > 2011/9/30 Kostik Belousov : >> On Fri, Sep 30, 2011 at 11:20:28AM -0700, Kirk McKusick wrote: >>> > Date: Fri, 30 Sep 2011 15:31:56 +0200 >>> > Subject: Re: Need to force sync(2) before umounting UFS1 filesystems? >>> > From: Attilio Rao >>> > To: Kirk McKusick >>> > Cc: Konstantin Belousov , >>> > =A0 =A0 Garrett Cooper , >>> > =A0 =A0 freebsd-fs@freebsd.org, Xin LI >>> > >>> > 2011/9/30 Kirk McKusick : >>> > >>> > > Here is my proposed fix. It does the unroll originally found in the >>> > > non-FORCE case before sleeping waiting for the vfs_busy to clear. >>> > > Is it acceptable to hold the mount mutex while calling VOP_UNLOCK? >>> > > If not, then it needs to be released before the unlock, reacquired >>> > > afterwards, and the check to see if the sleep is needed redone. >>> > >>> > I thought about this approach when sending the e-mail, but there is a >>> > problem: you need to handle the MNTK_UNMOUNT flag checking and >>> > subsequent setting after coveredvnode is held, otherwise at the first >>> > looping you will just return EBUSY. >>> > >>> > You can avoid the unlock by passing PVFS | PDROP. >>> > >>> > Attilio >>> >>> Problem noted. I have updated the patch to clear the MNTK_UNMOUNT >>> (and other flags set above it) after it returns from the sleep. >>> Which means I cannot use the PDROP flag now, but it is good to >>> know about it for future reference. >>> >>> Still not clear to me if it is acceptable to hold the mount mutex >>> while calling VOP_UNLOCK. Should I drop the mount mutex around the >>> VOP_UNLOCK(coveredvp)? Other than that possible problem, this patch >>> appears to solve the EBUSY problem and avoid possible deadlocks. >> I do not understand which deadlock is talked about there. >> It seems thay Attilio concern was with acquiring covered vnode lock >> after mounted fs is busied, but this is prohibited. >> >> See r166167 for more detailed description of the order. > > Ok, so that is the invariant I was forgetting, thanks Kostik. > > Kirk, you can make the 'forced unmount' behaviour by default for me, > now, thanks. > It would be great to have a comment on top of vfs_busy() or > dounmount() check of mnt_ref on why this deadlock cannot happen, > likely squeezing some good words from tegge's description of r166167. > Kirk may be the best person to do it, but I can have his backs if he > doesn't have time right now. Ok. Now that I know this is the direction you guys want to go, I'll start testing the change. Thanks! -Garrett