Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 25 Sep 2024 10:02:22 -0400
From:      John Baldwin <jhb@FreeBSD.org>
To:        Konstantin Belousov <kostikbel@gmail.com>, =?UTF-8?Q?Dag-Erling_Sm=C3=B8rgrav?= <des@freebsd.org>
Cc:        src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org
Subject:   Re: git: a52b30ff98cd - main - sys_pipe: consistently use cr_ruidinfo for accounting of pipebuf
Message-ID:  <88935da3-253c-4145-b958-f7137485ee31@FreeBSD.org>
In-Reply-To: <Zu8xxRee7cJsPtAW@kib.kiev.ua>
References:  <202409202109.48KL9RZ1078677@gitrepo.freebsd.org> <867cb5z2c3.fsf@ltc.des.dev> <Zu7A8bFd_cDIUKTD@kib.kiev.ua> <86y13lx7l5.fsf@ltc.des.dev> <Zu8xxRee7cJsPtAW@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
On 9/21/24 16:51, Konstantin Belousov wrote:
> On Sat, Sep 21, 2024 at 07:03:18PM +0200, Dag-Erling Smørgrav wrote:
>> Konstantin Belousov <kostikbel@gmail.com> writes:
>>> Dag-Erling Smørgrav <des@FreeBSD.org> writes:
>>>> Konstantin Belousov <kib@FreeBSD.org> writes:
>>>>> commit a52b30ff98cdab82af140285fa7fcdf1036fef27
>>>>>
>>>>>      sys_pipe: consistently use cr_ruidinfo for accounting of pipebuf
>>>>>      
>>>>>      Tested by:      yasu
>>>>>      Sponsored by:   The FreeBSD Foundation
>>>>   >     MFC after:      1 week
>>>> This appears to be the opposite of the patch which you posted on
>>>> -current and which yasu@ tested [...]
>>> Before committing anything, I did a self-review and remembered that I
>>> have did a lot of considerations when implementing swap accounting and
>>> decided that ruid is the right target for charge.
>>>
>>> Besides stating the obvious fact above, what do you expect me to answer/
>>> react to your mail?
>>
>> My point is that the commit message claims the patch was tested by yasu@
>> when in fact it wasn't.  If you're convinced that this is the correct
>> solution then that's fine, and it does appear to work, but don't claim
>> that it's been tested by others when it hasn't.
> 
> The main part of the patch was to ensure consistency in updates: it must
> be either always uidinfo, or always ruidinfo.  Which one specifically
> would affect the limit's semantic, but not the buggy behavior you reported.

In similar situations with reviews I've sometimes added a "(earlier version)"
suffix after the annotation, e.g.:

Reviewed by:    foo (earlier version)

Using a similar annotation here might have been clearer.

-- 
John Baldwin




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?88935da3-253c-4145-b958-f7137485ee31>