From owner-freebsd-fs@FreeBSD.ORG Thu Sep 29 02:19:54 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 16A65106564A; Thu, 29 Sep 2011 02:19:54 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-qy0-f175.google.com (mail-qy0-f175.google.com [209.85.216.175]) by mx1.freebsd.org (Postfix) with ESMTP id 90BE48FC1A; Thu, 29 Sep 2011 02:19:53 +0000 (UTC) Received: by qyk10 with SMTP id 10so3403735qyk.13 for ; Wed, 28 Sep 2011 19:19:52 -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=wN/t/EtuC6GgjdP3mnmpJrSXXlQfyFD/2zCxSnlgdco=; b=HFpvz0l5UENN5laaQRB8In6A1zMTfmMi24i3yfyb5w0cVbG5xup/WLEYXXgmlR9+2o /Qdj7mPGyI/bn8YI4bAtxRqBgyr8rC7S8QM0IimnFtTKAi9KQFshxklL2205glwUPAhm LNum7FXvaVbZ9aIuNfVzTW3EI5Bk37gWI6dmw= MIME-Version: 1.0 Received: by 10.224.217.137 with SMTP id hm9mr7564312qab.124.1317262792677; Wed, 28 Sep 2011 19:19:52 -0700 (PDT) Received: by 10.224.74.82 with HTTP; Wed, 28 Sep 2011 19:19:52 -0700 (PDT) In-Reply-To: <20110929013635.GG1511@deviant.kiev.zoral.com.ua> References: <201109280019.p8S0JVUW067163@chez.mckusick.com> <20110929013635.GG1511@deviant.kiev.zoral.com.ua> Date: Wed, 28 Sep 2011 19:19:52 -0700 Message-ID: From: Garrett Cooper To: Kostik Belousov Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Kirk McKusick , freebsd-fs@freebsd.org, Xin LI , bug-followup@freebsd.org Subject: Re: PR kern/161016 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 02:19:54 -0000 On Wed, Sep 28, 2011 at 6:36 PM, Kostik Belousov wrot= e: > On Tue, Sep 27, 2011 at 05:19:31PM -0700, Kirk McKusick wrote: >> > Date: Sun, 25 Sep 2011 12:07:18 -0700 >> > From: Garrett Cooper >> > To: lev@freebsd.org >> > Cc: freebsd-fs@freebsd.org, Xin LI , current@free= bsd.org >> > Subject: Re: Need to force sync(2) before umounting UFS1 filesystems? >> > >> > 2011/9/25 Lev Serebryakov : >> > > Hello, Garrett. >> > > You wrote 25 =3DD3=3DC5=3DCE=3DD4=3DD1=3DC2=3DD2=3DD1 2011 =3DC7., 1= 2:06:05: >> > > >> > >> =3D9A =3D9A Talking to Xin yesterday, he was convinced that this wa= s a >> > >> filesystem//kern bug. Before I file a PR, I'm wondering if anyone e= lse >> > >> has seen this issue.. >> > > =3D9AYes, and I posted message about it in embedded@ (Message-ID >> > > <1175277342.20110821215629@serebryakov.spb.ru>), I've got additional >> > > question from Warner Losh about base (underlying) file system, witho= ut >> > > any additional reaction. >> > >> > Thanks for the comments Adrian and Lev! I've filed PR 161016 to track >> > the issue, because it might be due to changes in the SU code, md, or a >> > subtle race condition in umount (highly unlikely, but it's been >> > noted). >> > -Garrett >> > _______________________________________________ >> > freebsd-fs@freebsd.org mailing list >> > http://lists.freebsd.org/mailman/listinfo/freebsd-fs >> > To unsubscribe, send any mail to "freebsd-fs-unsubscribe@freebsd.org" >> >> I have taken responsibility for working on this bug report (PR kern/1610= 16). >> >> I propose the following change to correct it: >> >> Index: sys/kern/vfs_mount.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- sys/kern/vfs_mount.c =A0 =A0 =A0(revision 225807) >> +++ sys/kern/vfs_mount.c =A0 =A0 =A0(working copy) >> @@ -1227,18 +1227,6 @@ >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 mp->mnt_kern_flag |=3D MNTK_UNMOUNTF; >> =A0 =A0 =A0 error =3D 0; >> =A0 =A0 =A0 if (mp->mnt_lockref) { >> - =A0 =A0 =A0 =A0 =A0 =A0 if ((flags & MNT_FORCE) =3D=3D 0) { >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mp->mnt_kern_flag &=3D ~(MNTK_= UNMOUNT | MNTK_NOINSMNTQ | >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 MNTK_UNMOUNTF); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (mp->mnt_kern_flag & MNTK_M= WAIT) { >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mp->mnt_kern_f= lag &=3D ~MNTK_MWAIT; >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 wakeup(mp); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 MNT_IUNLOCK(mp); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (coveredvp) >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 VOP_UNLOCK(cov= eredvp, 0); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (EBUSY); >> - =A0 =A0 =A0 =A0 =A0 =A0 } >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 mp->mnt_kern_flag |=3D MNTK_DRAINING; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 error =3D msleep(&mp->mnt_lockref, MNT_MTX(m= p), PVFS, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "mount drain", 0); >> >> The things to check for are: >> >> 1) That it fixes the EBUSY on unmount. >> >> 2) That it does not cause unmount to hang. >> >> I would appreciate feedback as to whether this fix helps. > > I think the item 2) should be tested mostly on the hung NFS server. > > I understand what you are doing, you do not want a transient mount point > busy caller to fail the unmount. But my belief is that this is the > intended mode of operation for non-forced unmounts. > > As I compare the original bug report and your change, the reason that > UFS gives spurious EBUSY on soft unmounts is that SU code busies mp > around some processing. Is my guess right ? Then, restoring some amount > of sync(2) before the unmount would be useful, please see r222466 for > the most likely reason why the issue appeared. > > Might be, the best route would be to add a kludge mnt_flag that request > dounmount() to do a VFS_SYNC() before checking for the busy holder ? This would undo some of the changes attillio added for the locking stuff in r184554. Not sure what the prior behavior was because I only traced back the change a little bit. Thanks, -Garrett 1. http://svnweb.freebsd.org/base/head/sys/kern/vfs_mount.c?revision=3D1845= 54&view=3Dmarkup