Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 11 May 2013 16:34:50 +0200
From:      Attilio Rao <attilio@freebsd.org>
To:        John Baldwin <jhb@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, Marcel Moolenaar <marcel@freebsd.org>, src-committers@freebsd.org, Marcel Moolenaar <marcel@xcllnt.net>
Subject:   Re: svn commit: r250411 - in head/sys: conf kern sys
Message-ID:  <CAJ-FndBHD4sLquFcSYERkD2Mfo4iAbjBBuvtdOmp0PrqJE2MrA@mail.gmail.com>
In-Reply-To: <201305101533.26992.jhb@freebsd.org>
References:  <201305091628.r49GSI33039873@svn.freebsd.org> <201305101211.35808.jhb@freebsd.org> <6CBEB766-087B-41F4-B549-2D60F4FD2701@xcllnt.net> <201305101533.26992.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, May 10, 2013 at 9:33 PM, John Baldwin <jhb@freebsd.org> wrote:
> On Friday, May 10, 2013 2:51:20 pm Marcel Moolenaar wrote:
>>
>> On May 10, 2013, at 9:11 AM, John Baldwin <jhb@freebsd.org> wrote:
>>
>> > On Friday, May 10, 2013 11:46:54 am Marcel Moolenaar wrote:
>> >>>
>> >>> 2) vnode locks from a local filesystem that report a LOR with a "devfs"
>> >>>  vnode.  Typical reports are either "ufs" -> "devfs" or in some cases
>> >>>  "ufs" -> "devfs" -> "ufs".  As with 1), I would much rather tag the
>> >>>  offending location than to disable all WITNESS checking on vnode locks.
>> >>
>> >> With more file system types in use, this will get mixed up with the
>> >> other file systems and noise you get is rather severe. It is a big
>> >> problem for us at Juniper.
>> >
>> > Note, it is very specific that the second lock is always "devfs".  I think
>> > that points to this being isolated to a few specific places, not a generic
>> > ordering problem.
>>
>> Alas, that's not the case. These LORs are reported between ufs and unionfs,
>> or ufs and isofs, etc. It's not just between something and devfs.
>
> Ugh, I have only seen them with devfs so had presumed them to be more
> localized (and thus more easily targeted).  In that case your change
> may be as fine-grained as we can get.  I would also like to still keep
> WITNESS checking between vnode locks and other lock types, and LK_NOWITNESS
> would remove that, so between your change and Attilio's approach I do
> prefer yours.
>
>> I'm not sure the only options we have are to ignore the problem
>> or implement a general fix. If we set out to silence witness for
>> the known false positives then it's ok to handle them on a case
>> by case basis. We'll see patterns soon enough and then re-code
>> the solutions in terms of those patterns. If we're lucky we see
>> a single general solution, but if not, then it's fine to have a
>> handful of special case. The worse we can do is not address it
>> at all.
>
> I was assuming that the reversals were far more specific, and knowing about
> other false positives like the dirhash one, I want a generic way to do more
> fine-grained marking of false positives.  If there were only a few places we
> would need to mark to fix the reversals you see, then I would prefer the
> suspend/resume approach for your case.  However, the reversals you are masking
> sound too widespread to support that.

The solution to this is what I proposed: pass down a LK_NOWITNESS for
instances where you don't want to check for witness.
This is per lock-call.
You localize the fix to the instance you want to shut down and you
skip witness for every false-positive.
When you commit the LK_NOWITNESS you mention explicitely the reason
why the reported LOR is a false positive so it is also documented on
svn.

I still don't understand what you are objecting. Marcel objections'
were completely no-sense (Don't want to involve Witness) but at least
I expect some decent one by you.

Attilio


--
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-FndBHD4sLquFcSYERkD2Mfo4iAbjBBuvtdOmp0PrqJE2MrA>