syncer vnode leak because of nmount() race

Attilio Rao attilio at freebsd.org
Wed Jun 9 13:35:23 UTC 2010


2010/6/5 Jaakko Heinonen <jh at 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
>> >    vfs_busy(9) manual page is misleading because it talks about
>> >    synchronizing 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.  It also delays
> 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:
>
>        http://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


-- 
Peace can only be achieved by understanding - A. Einstein


More information about the freebsd-fs mailing list