Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 11 Aug 2015 19:58:14 +0300
From:      Alexander Motin <mav@FreeBSD.org>
To:        Steven Hartland <steven.hartland@multiplay.co.uk>,  Andriy Gapon <avg@FreeBSD.org>, src-committers@FreeBSD.org, svn-src-all@FreeBSD.org,  svn-src-head@FreeBSD.org
Subject:   Re: svn commit: r286625 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Message-ID:  <55CA29A6.1000406@FreeBSD.org>
In-Reply-To: <55CA1921.6030606@multiplay.co.uk>
References:  <201508111039.t7BAdK1x071658@repo.freebsd.org> <55C9D3A5.1020000@FreeBSD.org> <55CA1921.6030606@multiplay.co.uk>

next in thread | previous in thread | raw e-mail | index | archive | help
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@delphix.com>
>>>    Reviewed by: George Wilson <george.wilson@delphix.com>
>>>    Reviewed by: Steven Hartland <killing@multiplay.co.uk>
>>>    Reviewed by: Richard Elling <richard.elling@richardelling.com>
>>>    Approved by: Dan McDonald <danmcd@omniti.com>
>>>    Author: Matthew Ahrens <mahrens@delphix.com>
>>>       illumos/illumos-gate@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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?55CA29A6.1000406>