Re: git: a52b30ff98cd - main - sys_pipe: consistently use cr_ruidinfo for accounting of pipebuf

From: John Baldwin <jhb_at_FreeBSD.org>
Date: Wed, 25 Sep 2024 14:02:22 UTC
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