Received-SPF: pass (google.com: domain of linux-kernel+bounces-84698-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <6db0f0c8-81cb-4d04-9560-ba73d63db4b8@suse.cz>
Date: Wed, 28 Feb 2024 09:47:17 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 19/36] mm: create new codetag references during page
 splitting
Content-Language: en-US
To: Suren Baghdasaryan <surenb@google.com>
Cc: akpm@linux-foundation.org, kent.overstreet@linux.dev, mhocko@suse.com,
 hannes@cmpxchg.org, roman.gushchin@linux.dev, mgorman@suse.de,
 dave@stgolabs.net, willy@infradead.org, liam.howlett@oracle.com,
 penguin-kernel@i-love.sakura.ne.jp, corbet@lwn.net, void@manifault.com,
 peterz@infradead.org, juri.lelli@redhat.com, catalin.marinas@arm.com,
 will@kernel.org, arnd@arndb.de, tglx@linutronix.de, mingo@redhat.com,
 dave.hansen@linux.intel.com, x86@kernel.org, peterx@redhat.com,
 david@redhat.com, axboe@kernel.dk, mcgrof@kernel.org, masahiroy@kernel.org,
 nathan@kernel.org, dennis@kernel.org, tj@kernel.org, muchun.song@linux.dev,
 rppt@kernel.org, paulmck@kernel.org, pasha.tatashin@soleen.com,
 yosryahmed@google.com, yuzhao@google.com, dhowells@redhat.com,
 hughd@google.com, andreyknvl@gmail.com, keescook@chromium.org,
 ndesaulniers@google.com, vvvvvv@google.com, gregkh@linuxfoundation.org,
 ebiggers@google.com, ytcoode@gmail.com, vincent.guittot@linaro.org,
 dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com,
 bristot@redhat.com, vschneid@redhat.com, cl@linux.com, penberg@kernel.org,
 iamjoonsoo.kim@lge.com, 42.hyeyoo@gmail.com, glider@google.com,
 elver@google.com, dvyukov@google.com, shakeelb@google.com,
 songmuchun@bytedance.com, jbaron@akamai.com, rientjes@google.com,
 minchan@google.com, kaleshsingh@google.com, kernel-team@android.com,
 linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
 iommu@lists.linux.dev, linux-arch@vger.kernel.org,
 linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
 linux-modules@vger.kernel.org, kasan-dev@googlegroups.com,
 cgroups@vger.kernel.org
References: <20240221194052.927623-1-surenb@google.com>
 <20240221194052.927623-20-surenb@google.com>
 <2daf5f5a-401a-4ef7-8193-6dca4c064ea0@suse.cz>
 <CAJuCfpGt+zfFzfLSXEjeTo79gw2Be-UWBcJq=eL1qAnPf9PaiA@mail.gmail.com>
From: Vlastimil Babka <vbabka@suse.cz>
In-Reply-To: <CAJuCfpGt+zfFzfLSXEjeTo79gw2Be-UWBcJq=eL1qAnPf9PaiA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On 2/27/24 17:38, Suren Baghdasaryan wrote:
> On Tue, Feb 27, 2024 at 2:10 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>>
>> On 2/21/24 20:40, Suren Baghdasaryan wrote:
>> > When a high-order page is split into smaller ones, each newly split
>> > page should get its codetag. The original codetag is reused for these
>> > pages but it's recorded as 0-byte allocation because original codetag
>> > already accounts for the original high-order allocated page.
>>
>> This was v3 but then you refactored (for the better) so the commit log
>> could reflect it?
> 
> Yes, technically mechnism didn't change but I should word it better.
> Smth like this:
> 
> When a high-order page is split into smaller ones, each newly split
> page should get its codetag. After the split each split page will be
> referencing the original codetag. The codetag's "bytes" counter
> remains the same because the amount of allocated memory has not
> changed, however the "calls" counter gets increased to keep the
> counter correct when these individual pages get freed.

Great, thanks.
The concern with __free_pages() is not really related to splitting, so for
this patch:

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> 
>>
>> > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
>>
>> I was going to R-b, but now I recalled the trickiness of
>> __free_pages() for non-compound pages if it loses the race to a
>> speculative reference. Will the codetag handling work fine there?
> 
> I think so. Each non-compoud page has its individual reference to its
> codetag and will decrement it whenever the page is freed. IIUC the
> logic in  __free_pages(), when it loses race to a speculative
> reference it will free all pages except for the first one and the

The "tail" pages of this non-compound high-order page will AFAICS not have
code tags assigned, so alloc_tag_sub() will be a no-op (or a warning with
_DEBUG).

> first one will be freed when the last put_page() happens. If prior to
> this all these pages were split from one page then all of them will
> have their own reference which points to the same codetag.

Yeah I'm assuming there's no split before the freeing. This patch about
splitting just reminded me of that tricky freeing scenario.

So IIUC the "else if (!head)" path of __free_pages() will do nothing about
the "tail" pages wrt code tags as there are no code tags.
Then whoever took the speculative "head" page reference will put_page() and
free it, which will end up in alloc_tag_sub(). This will decrement calls
properly, but bytes will become imbalanced, because that put_page() will
pass order-0 worth of bytes - the original order is lost.

Now this might be rare enough that it's not worth fixing if that would be
too complicated, just FYI.


> Every time
> one of these pages are freed that codetag's "bytes" and "calls"
> counters will be decremented. I think accounting will work correctly
> irrespective of where these pages are freed, in __free_pages() or by
> put_page().
>