Date: Thu, 29 Sep 2011 18:13:34 +0200 From: Attilio Rao <attilio@freebsd.org> To: Kirk McKusick <mckusick@mckusick.com>, Konstantin Belousov <kib@freebsd.org> Cc: Garrett Cooper <yanegomi@gmail.com>, freebsd-fs@freebsd.org, Xin LI <delphij@freebsd.org> Subject: Re: Need to force sync(2) before umounting UFS1 filesystems? Message-ID: <CAJ-FndBUhzBH3KaaDBvZk3OnNifLd83NedYLPvmLHTL7PTCmCA@mail.gmail.com> In-Reply-To: <201109291559.p8TFxc63084067@chez.mckusick.com> References: <CAJ-FndDxsVYkPQfqiYfshh%2BkpCyfbGTL0m%2BoCN37qXyjAucM6g@mail.gmail.com> <201109291559.p8TFxc63084067@chez.mckusick.com>
next in thread | previous in thread | raw e-mail | index | archive | help
2011/9/29 Kirk McKusick <mckusick@mckusick.com>: >> Date: Thu, 29 Sep 2011 17:40:43 +0200 >> Subject: Re: Need to force sync(2) before umounting UFS1 filesystems? >> From: Attilio Rao <attilio@freebsd.org> >> To: Kirk McKusick <mckusick@mckusick.com> >> Cc: Garrett Cooper <yanegomi@gmail.com>, freebsd-fs@freebsd.org, >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 Xin LI <delphij@freebsd.org> >> >> 2011/9/29 Kirk McKusick <mckusick@mckusick.com>: >> >> > 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndBUhzBH3KaaDBvZk3OnNifLd83NedYLPvmLHTL7PTCmCA>