svn commit: r286625 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

Alexander Motin mav at FreeBSD.org
Tue Aug 11 16:58:21 UTC 2015


On 11.08.2015 18:47, Steven Hartland wrote:
> On 11/08/2015 11:51, Andriy Gapon wrote:
>> On 11/08/2015 13:39, Alexander Motin wrote:
>>> Author: mav
>>> Date: Tue Aug 11 10:39:19 2015
>>> New Revision: 286625
>>> URL: https://svnweb.freebsd.org/changeset/base/286625
>>>
>>> Log:
>>>    MFV r277425:
>>>    5376 arc_kmem_reap_now() should not result in clearing arc_no_grow
>>>    Reviewed by: Christopher Siden <christopher.siden at delphix.com>
>>>    Reviewed by: George Wilson <george.wilson at delphix.com>
>>>    Reviewed by: Steven Hartland <killing at multiplay.co.uk>
>>>    Reviewed by: Richard Elling <richard.elling at richardelling.com>
>>>    Approved by: Dan McDonald <danmcd at omniti.com>
>>>    Author: Matthew Ahrens <mahrens at delphix.com>
>>>       illumos/illumos-gate at 2ec99e3e987d8aa273f1e9ba2b983557d058198c
>>>
>>> Modified:
>>>    head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
>> Alexander,
>>
>> thank you very much for bringing all these upstream changes into our
>> tree.
>> It seems that some of the changes, though, non-trivially overlap with
>> FreeBSD-specific changes to ZFS code.  I think that this change is one
>> of the examples.
>> It would be good if a strategy of the resolution of each non-trivial
>> conflict was described and possibly discussed.  Reviewing the change
>> without knowing the general idea behind it is not always easy.
>>
> I actually eliminated most of the miss-matches between illumos and
> FreeBSD in this area pretty recently, so I'm not sure that there's
> actually many FreeBSD specific changes, hence conflicts. Given I worked
> in this area before I did also make a point of reviewing the upstream
> commit.
> 
> That's not to say it wouldn't be good to review these sorts of changes
> especially given the potential impact, however we don't have a very good
> track record (myself included) in reviewing things so I'm concerned that
> would just become a real progress blocker.

The fact that we had 6+ months of lag from upstream having several
developers in the area tells me that we accumulated too much divergence,
so that merging of already reviewed patches makes people worry about
another review. I think we should just take big sledgehammer and break
through this wall.

For those who want to polish their reviewing skills, I'll soon prepare
more interesting patch to look at -- merging the new implementation of
improved ARC locking from Illumos. That seems to be one of the biggest
divergence points. I've managed find a way to apply those patches almost
clean, to hope that it will work when the build completes. :)

-- 
Alexander Motin


More information about the svn-src-all mailing list