From owner-freebsd-fs@FreeBSD.ORG Thu Sep 29 16:13:37 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 150941065688; Thu, 29 Sep 2011 16:13:37 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 3F5078FC0A; Thu, 29 Sep 2011 16:13:35 +0000 (UTC) Received: by wyj26 with SMTP id 26so207208wyj.13 for ; Thu, 29 Sep 2011 09:13:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=Aiohl0RQJzS3yU6BnakC+UJWMqxybg6JUodr7NeyUrc=; b=PySw9qQBPENCCK0E1QjiEgwuIhWnzkXsNh+dzVoZSHb6wuSNAj8wOtdUu8kREP3Mil u89TZAqCrftgSIN7RZXfId/A/LFI16JYOBlPZLDoe24mIGrCrykYcy8CfIiuk/gar33X jG9gvEABsJOAROc3siSW9qM8k4/UOAsFEULxk= MIME-Version: 1.0 Received: by 10.216.229.134 with SMTP id h6mr1148480weq.42.1317312814929; Thu, 29 Sep 2011 09:13:34 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.216.182.3 with HTTP; Thu, 29 Sep 2011 09:13:34 -0700 (PDT) In-Reply-To: <201109291559.p8TFxc63084067@chez.mckusick.com> References: <201109291559.p8TFxc63084067@chez.mckusick.com> Date: Thu, 29 Sep 2011 18:13:34 +0200 X-Google-Sender-Auth: 7AwbG_xLw2EqSfE7WfRbKHRk_sc Message-ID: From: Attilio Rao To: Kirk McKusick , Konstantin Belousov Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: Garrett Cooper , freebsd-fs@freebsd.org, Xin LI 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: Thu, 29 Sep 2011 16:13:37 -0000 2011/9/29 Kirk McKusick : >> Date: Thu, 29 Sep 2011 17:40:43 +0200 >> Subject: Re: Need to force sync(2) before umounting UFS1 filesystems? >> From: Attilio Rao >> To: Kirk McKusick >> Cc: Garrett Cooper , freebsd-fs@freebsd.org, >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 Xin LI >> >> 2011/9/29 Kirk McKusick : >> >> > Thanks for providing a bit more of the history on this codepath. >> > >> > Since 9-stable has now been branched, I believe that the best path >> > forward is to check this change into head and let it sit there for >> > several months so that we can get some experience with it. If it >> > causes folks problems we can back it out. If it does not cause >> > problems, then we can MFC it to 9-stable. >> > >> > Does this seem like a reasonable approach? >> >> In general yes, but I'd like to understand why unmount should fail so >> much with SU... do we do extended period with vfs_busy()'ed >> filesystem? >> >> I need more context here, likely I'd need to look into the PRs too >> before to give an informative answer. >> >> Attilio > > I am definitely not in a rush on this, so by all means take some time > to look it over. The EBUSY unmount has been in its current state > for several years, so I am fine with taking a few weeks to sort out > the correct solution. Indeed, I am glad that Garrett has volunteered > to do some more serious testing. > > If this general approach is not correct, I can put a hook in for just > UFS so that it can have its historic behavior. As you have noted, the > SU code has a lot of activity that gets done under the protection of > vfs_busy. So it may be the only filesystem for which draining the > vfs_busy lock during unmount is needed. Honestly, my first thought was exactly that -- an option that was forcing the unmount sleep if SU is compiled in the kernel. When you mention 'historical behaviour' you mean the behaviour UFS had even prior the introduction of the 'complete' VFS layer or it was the behaviour unmount(2) was expected to implemented since the beginning? My guess is that recent SU improvement by you and Jeff may have lead to higher vfs_busy() contention, thus making this behaviour just more visible. BTW, I'm afraid the forced unmount case may have a possible deadlock. thread1 is doing whatever codepath it wants and thread2 is doing unmount (forced right now): thread1::vfs_busy() thread2::lock coveredvnode thread1::contests coveredvnode thread2::sleep because of thread1 vfs_busy I think this deadlock was actually possible even with the old code, it was just a LOR between a vnode lock and mount lock. I'm not sure if there was any invariant I discussed with Kostik in the past, preventing one way or another I'm forgetting about, but it seems a possible deadlock to me. If you see this issue I'll make a patch for it. Attilio --=20 Peace can only be achieved by understanding - A. Einstein