Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 25 Nov 2014 12:03:57 +0200
From:      Andriy Gapon <avg@FreeBSD.org>
To:        d@delphij.net, Xin LI <delphij@FreeBSD.org>, src-committers@FreeBSD.org, svn-src-all@FreeBSD.org, svn-src-head@FreeBSD.org
Subject:   Re: svn commit: r273060 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Message-ID:  <5474540D.2000607@FreeBSD.org>
In-Reply-To: <546F9FC3.10402@delphij.net>
References:  <201410132039.s9DKdpmh055573@svn.freebsd.org> <546F57B1.2020602@FreeBSD.org> <546F9FC3.10402@delphij.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On 21/11/2014 22:25, Xin Li wrote:
> On 11/21/14 07:18, Andriy Gapon wrote:
>> On 13/10/2014 23:39, Xin LI wrote:
>>> Author: delphij Date: Mon Oct 13 20:39:51 2014 New Revision:
>>> 273060 URL: https://svnweb.freebsd.org/changeset/base/273060
>>>
>>> Log: Use write_psize instead of write_asize when doing
>>> vdev_space_update. Without this change the accounting of L2ARC
>>> usage would be wrong and give 16EB free space because the number
>>> became negative and overflows.
>>>
>>> Obtained from:	FreeNAS (issue #6239)
> 
>> First, a link to the issue would be more convenient for reviewers.
>> Here it is https://bugs.freenas.org/issues/6239
> 
>> Then, I would like to see a technical explanation for this change. 
>> I could not find any explanation here or in the FreeNAS issue or in
>> the FreeNAS commit.
> 
>> As far as I can see, all calls to vdev_space_update() in the ARC
>> code are passed l2hdr->b_asize or a sum of b_asize fields of
>> multiple buffers. Thus, I am really surprised with this change and
>> would like to see reasoning behind it.
> 
> Hmm I think you are right that my change doesn't make sense but I
> can't remember the details either and suggests there is some other
> issues that was covered up by this.

Maybe you have some other changes in FreeNAS and the change in question is
supposed to work along with them?

Meanwhile, perhaps the discussed commit needs to be reverted?

> It looks like that I was confused because the L2ARC clock hand
> advances by buf_p_sz (unrelated but it should really be called
> buf_a_size) and the way we calculate allocated L2ARC free.

Agree about 'a' vs 'p'.  And, BTW, we have some related changes in ClusterHQ
that I will upstream.

-- 
Andriy Gapon



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5474540D.2000607>