svn commit: r248084 - in head/sys: amd64/amd64 arm/arm cddl/compat/opensolaris/kern cddl/compat/opensolaris/sys cddl/contrib/opensolaris/uts/common/fs/zfs cddl/contrib/opensolaris/uts/common/fs/zfs...

[Garrett Cooper]

On Tue, Jul 9, 2013 at 11:10 AM, Alfred Perlstein <alfred at freebsd.org> wrote:
> On 7/8/13 11:30 PM, Garrett Cooper wrote:
>>
>> On Mar 8, 2013, at 6:32 PM, Attilio Rao wrote:
>>
>>> Author: attilio
>>> Date: Sat Mar  9 02:32:23 2013
>>> New Revision: 248084
>>> URL: http://svnweb.freebsd.org/changeset/base/248084
>>>
>>> Log:
>>> Switch the vm_object mutex to be a rwlock.  This will enable in the
>>> future further optimizations where the vm_object lock will be held
>>> in read mode most of the time the page cache resident pool of pages
>>> are accessed for reading purposes.
>>>
>>> The change is mostly mechanical but few notes are reported:
>>> * The KPI changes as follow:
>>>   - VM_OBJECT_LOCK() -> VM_OBJECT_WLOCK()
>>>   - VM_OBJECT_TRYLOCK() -> VM_OBJECT_TRYWLOCK()
>>>   - VM_OBJECT_UNLOCK() -> VM_OBJECT_WUNLOCK()
>>>   - VM_OBJECT_LOCK_ASSERT(MA_OWNED) -> VM_OBJECT_ASSERT_WLOCKED()
>>>     (in order to avoid visibility of implementation details)
>>>   - The read-mode operations are added:
>>>     VM_OBJECT_RLOCK(), VM_OBJECT_TRYRLOCK(), VM_OBJECT_RUNLOCK(),
>>>     VM_OBJECT_ASSERT_RLOCKED(), VM_OBJECT_ASSERT_LOCKED()
>>> * The vm/vm_pager.h namespace pollution avoidance (forcing requiring
>>>   sys/mutex.h in consumers directly to cater its inlining functions
>>>   using VM_OBJECT_LOCK()) imposes that all the vm/vm_pager.h
>>>   consumers now must include also sys/rwlock.h.
>>> * zfs requires a quite convoluted fix to include FreeBSD rwlocks into
>>>   the compat layer because the name clash between FreeBSD and solaris
>>>   versions must be avoided.
>>>   At this purpose zfs redefines the vm_object locking functions
>>>   directly, isolating the FreeBSD components in specific compat stubs.
>>>
>>> The KPI results heavilly broken by this commit.  Thirdy part ports must
>>> be updated accordingly (I can think off-hand of VirtualBox, for example).
>>>
>>> Sponsored by:   EMC / Isilon storage division
>>> Reviewed by:    jeff
>>> Reviewed by:    pjd (ZFS specific review)
>>> Discussed with: alc
>>> Tested by:      pho
>>
>>         This commit broke emulators/open-vm-tools (which helps with
>> hardware acceleration and other guest OS services on VMware, et al) and it's
>> been broken for ~4 months now. Please ask portmgr@ to do an exp- run before
>> making KPI changes. open-vm-tools, nvidia-driver, qemu*, and virtualbox-ose*
>> are and have been particularly vulnerable to sweeping changes like this in
>> the past (I've had to patch a lot of 3rd party software broken by KPI
>> changes in 10.x, more than in prior releases) and a lot of developers depend
>> on this functionality to be sane in order to develop software on -CURRENT
>> and -STABLE (and it allows us to better test your code).
>> Thanks,
>> -Garrett
>
>
> Thanks Garrett,
>
> It would be great if we could track vm changes like this.  Is there a doc
> that describes the process of an exp-run for us src guys?
>
> Can there a be a link or a hint towards this near some of the commonly
> bumped variables in freebsd base to let us know what to do?

In general (as most devs know), anytime that __FreeBSD_version__ needs
to be bumped for a change, there really should be an exp- run. I would
hope that intuition would at least allow a chicken switch between APIs
for a period of time so that people could at least be allowed time to
transition code over and then make the change.

> I fear even though we may educate the current crop of devs, that we will
> experience lossage as new developers are brought on unless we document this
> somewhere in src, or at least link to documentation elsewhere from src.

I wish the dev handbook was more canonical in this way, but it's
experiencing bitrot quicker than the handbook is. That ultimately
would be the best place to put things IMO.

> by the way, do you think this change may be what is making virtualbox
> insta-panic on my -current box?  Is there some trick we can use to version
> between the port? Maybe we need a set of macros or something to note the
> breakage of ABI?

I don't know. I'm having heck of a time getting open-vm-tools, qemu,
and virtualbox-ose to compile on CURRENT latest because of clang, gcc
warnings issues, mount locking changes, and vm changes (and I'm using
the patches I have in my git ports branch because stuff still doesn't
compile on ports head with open-vm-tools, etc, which is sad given that
I have a number of ports PRs that have been open for almost 6 months
now that haven't been acted on).

I'll get back to you later on in the week after I figure out whether
or not this is the case because I'd like to figure out whether or not
it works as well so I might be able to install 10.0-RELEASE in the
near future on my dev workstation and things won't all fall to pieces;
this has happened in the past, and I'm adverse to running CURRENT on
machines that require X11 because it involves a lot of pain with the
fact that things aren't properly tested -- even from a compilation
perspective -- before people commit things to base.

Thanks,
-Garrett