Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <8315a507-3f27-5b08-7d0c-c6056b64c3d6@suse.cz>
Date:   Mon, 24 Apr 2023 16:21:41 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH linux-next v2] mm: workingset: update description of the
 source file
Content-Language: en-US
To:     Yang Yang <yang.yang29@zte.com.cn>, akpm@linux-foundation.org
Cc:     hannes@cmpxchg.org, iamjoonsoo.kim@lge.com,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org, rppt@kernel.org
References: <0ddb50f0-2e2d-e750-3867-3842a3ea12cc@suse.cz>
 <20230421113130.240586-1-yang.yang29@zte.com.cn>
From:   Vlastimil Babka <vbabka@suse.cz>
In-Reply-To: <20230421113130.240586-1-yang.yang29@zte.com.cn>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 4/21/23 13:31, Yang Yang wrote:
>> I'm late but FWIW, not supper happy that while the updated calculations are
>> now accurate wrt the actual code, the explanation (which was written at the
>> time of page cache-only workinset) was more easier to follow in the simpler
>> form. Now it's still mostly talking about page cache and explaining the
>>balance between its active and inactive list only, and then suddenly the
>> anon lists appear out of nowhere in the final equations.
> 
> Thanks for your reviewing! I should update the whole parts. Sorry for
> hadn't do it better, please drop the patch, I will try to submit patchv3
> to fix this.

I think it's too late to drop, and not worth rebasing the git tree in early
merge window for that.

>> In other words, I think it would have been better to leave that explanation
>> as it was, and then add a new part describing the extension to anon pages.
> 
> I read the description of the source file again carefully, and think that
> there is no need to creat a new part, if we explain at the begining that
> the word 'pages' include page cache and anonymous page, and do some minor 
> adjustments. For example:
> Per node, two kinds of clock lists are maintained for pages..

I think it would make following the explanation more complicated, and it's
already difficult enough. Reasoning just about active vs inactive file list
is much simpler to follow the equations and observations behind them. Wonder
what Johannes as the original author thinks, anyway.