Date: Wed, 9 Jun 2010 15:35:21 +0200 From: Attilio Rao <attilio@freebsd.org> To: Jaakko Heinonen <jh@freebsd.org> Cc: freebsd-fs@freebsd.org, kib@freebsd.org Subject: Re: syncer vnode leak because of nmount() race Message-ID: <AANLkTika1fZN40iPLxBAN8DKZp4v8CALfQ7DWJnPBZXO@mail.gmail.com> In-Reply-To: <20100605190659.GA3369@a91-153-117-195.elisa-laajakaista.fi> References: <20100603143501.GA3176@a91-153-117-195.elisa-laajakaista.fi> <AANLkTimuJJcZ0D1TMDvTHjpb3advHetSB0aJw2IevSC1@mail.gmail.com> <20100605190659.GA3369@a91-153-117-195.elisa-laajakaista.fi>
next in thread | previous in thread | raw e-mail | index | archive | help
2010/6/5 Jaakko Heinonen <jh@freebsd.org>: > > Thank you for the reply. > > On 2010-06-04, Attilio Rao wrote: >> I think that, luckilly, it is not a very common condition to have the >> mount still in flight and get updates... :) > > Agreed, but mountd(8) increases chances because it does an update mount > for all local file systems when it receives SIGHUP. > >> However, I think that the real breakage here is that the check on >> mnt->mnt_syncer is done lockless and it is unsafe. > >> I found also this bug when rewriting the syncer and I resolved that by >> using a separate flag for that (in my case it was simpler and more >> beneficial actually for some other reasons, but you may do the same >> thing with a mnt_kern_flag entry). > > OK, I will take a look at this approach. > >> Additively, note that vfs_busy() here is not intended to protect >> against such situations but against unmount. >> >> > PS. vfs_unbusy(9) manual page is out of date after r184554 and IMO >> > =C2=A0 =C2=A0vfs_busy(9) manual page is misleading because it talks ab= out >> > =C2=A0 =C2=A0synchronizing access to a mount point. >> >> May you be more precise on what is misleading please? > > As you wrote above it protects only against unmount. At least I got > feeling that it does more than that when I read this: "The purpose of > this function is to synchronize access to a mount point. =C2=A0It also de= lays > unmounting by sleeping on mp if the MNTK_UNMOUNT flag is set in > mp->mnt_kern_flag and the LK_NOWAIT flag is not set.". > > I did some updates for the manual pages: > > =C2=A0 =C2=A0 =C2=A0 =C2=A0http://people.freebsd.org/~jh/patches/vfs_busy= -vfs_unbusy.diff That patch is fine. I'd just avoid to mention the mnt_lockref flag name, just use the generic 'refcount' word. Thanks, Attilio --=20 Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTika1fZN40iPLxBAN8DKZp4v8CALfQ7DWJnPBZXO>