[PATCH makedumpfile 4/9] Introduce a stat for pages retained by extension
HAGIO KAZUHITO(萩尾 一仁)
k-hagio-ab at nec.com
Wed Aug 19 00:52:56 PDT 2026
On 2026/08/18 4:58, Stephen Brennan wrote:
> Hello Tao, Kazu,
>
> I apologize for not responding on this patch's review sooner.
> Thank you both for reviewing carefully.
>
> Tao Liu <ltao at redhat.com> writes:
>> Hi Kazu,
>>
>> On Thu, Aug 13, 2026 at 4:54 PM HAGIO KAZUHITO(萩尾 一仁)
>> <k-hagio-ab at nec.com> wrote:
>>>
>>> On 2026/08/13 11:15, Tao Liu wrote:
>>>> Hi Stephen,
>>>>
>>>> On Mon, Jul 13, 2026 at 05:45:37PM -0700, Stephen Brennan wrote:
>>>>> Extensions can mark pages to be excluded, but those pages may already be
>>>>> excluded due to the dump level. We have a statistic to count pages
>>>>> excluded by extensions. It counts only pages which were excluded because
>>>>> no other criteria excluded them.
>>>>>
>>>>> Extensions can mark pages to be retained, but there is no statistic to
>>>>> count them. Adding a counter to the code as-is would not give us the
>>>>> value that we care about. Just as above, pages marked for inclusion may
>>>>> have been included anyway due to the dump-level configuration. The most
>>>>> useful statistic is the one that tells us how many pages were included
>>>>> by the extension, which would not have been included otherwise.
>>>>>
>>>>> Introduce a statistic that counts this amount. To do so, we have to
>>>>> skip the short-circuit evaluation when PG_INCLUDE is returned. This
>>>>> seems like a worthwhile trade-off, since the dump-level checks are all
>>>>> reasonably efficient.
>>>>>
>>>>> Signed-off-by: Stephen Brennan <stephen.s.brennan at oracle.com>
>>>>> ---
>>>>> makedumpfile.c | 13 +++++++++++--
>>>>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/makedumpfile.c b/makedumpfile.c
>>>>> index a4c9bbf..cf6a38f 100644
>>>>> --- a/makedumpfile.c
>>>>> +++ b/makedumpfile.c
>>>>> @@ -106,6 +106,7 @@ mdf_pfn_t pfn_elf_excluded;
>>>>> mdf_pfn_t pfn_extension;
>>>>>
>>>>> mdf_pfn_t num_dumped;
>>>>> +mdf_pfn_t num_extension_retained;
>>>>>
>>>>> int retcd = FAILED; /* return code */
>>>>>
>>>>> @@ -6638,8 +6639,6 @@ check_order:
>>>>> * makedumpfile extensions
>>>>> */
>>>>> filter_pg = run_extension_callback(pfn, pcache, &i);
>>>>> - if (filter_pg == PG_INCLUDE)
>>>>> - continue;
>>>>>
>>>>> /*
>>>>> * Exclude the free page managed by a buddy
>>>>> @@ -6722,6 +6721,13 @@ check_order:
>>>>> else
>>>>> continue;
>>>>>
>>>>> + if (filter_pg == PG_INCLUDE) {
>>>>> + /* Account pages which would have been excluded, but were
>>>>> + * retained by an extension. */
>>>>> + num_extension_retained += nr_pages;
>>>>> + continue;
>>>>
>>>> Maybe I'm wrong, from the code we are trying to retain nr_pages, don't
>>>> we need to do
>>>> pfn += nr_pages
>>>> to update the pfn of the next for-loop?
>>>
>>> Thank you for your review, Tao.
>>>
>>> The current code below does not use nr_pages to skip tail pages to be
>>> retained, so I think this patch's code matches this.
>>>
>>> /*
>>> * Unexcludable page
>>> */
>>> else
>>> continue;
>>>
>> After rethinking, I guess both ways work:
>>
>> 1)
>> if (filter_pg == PG_INCLUDE) {
>> num_extension_retained += nr_pages;
>> continue;
>> }
>> Then in the next for-loop, all the tail pages(for-loop nr_pages times)
>> will hit continue:
>> if (i.compound_head & 1)
>> continue;
>> so all pages are kept within vmcore.
>>
>> 2)
>> if (filter_pg == PG_INCLUDE) {
>> num_extension_retained += nr_pages;
>> pfn += nr_pages
>> continue;
>> }
>> Then in the next for-loop, all tail pages are skiped(only for-loop 1 time).
>>
>> The effect is the same, all the nr_pages are hit by continue, so all kept.
>
> Yes, as you noted in (1) the current design is that we detect compound
> tail pages near the beginning of the loop, so we don't need to adjust
> pfn. (For option (2), both mem_map and pfn must be adjusted, and they
> must be incremented by (nr_pages - 1), not nr_pages. We must be really
> careful when skipping the loop forward or else we corrupt the vmcore,
> which is why I relied on option (1) in this patch.)
>
>>> (Retained tail pages will be skipped at compound_head check. If we
>>> change this behavior, a separate patch would be preferable.)
>>>
>>> But on the other hand, if we set nr_pages to 1 when PG_EXCLUDE,
>>> extensions cannot exclude tail pages?
>>>
>>> else if (filter_pg == PG_EXCLUDE) {
>>> nr_pages = 1;
>>> pfn_counter = &pfn_extension;
>
> Hi Kazu,
>
> Yes, I agree this is a problem. if PG_EXCLUDE sets nr_pages = 1, then
> extensions cannot exclude tail pages.
>
> We could drop this "nr_pages = 1" line here. But I think this could be
> more completely fixed by Tao's approach below.
>
>> In fact, I didn't consider the case of compond pages when implementing
>> makedumpfile extension. My original thought was, makedumpfile pass
>> information of page one-by-one to extensions, allowing the extension
>> to decide whether to keep or discard it. Thus the filtering logic for
>> makedumpfile would be much simpler.
>>
>> Extension however, check page type(compound page or normal page) by
>> itself. E.g. if it want to exclude a compond page:
>>
>> makedumpfile:
>>
>> for (pfn;;pfn++) {
>> run_extension_callback(pfn, pcache, &i);
>> ...
>> }
>>
>> extension_call_back:
>> if ((check_page_type(pfn, pcache, page_info)) == compond_page) {
>> record_compond_range(pfn, pfn + nr_pages);
>> }
>> if (pfn within compond_range) {
>> return PG_EXCLUDE;
>> }
>>
>> In this way, I guess we even don't need to introduce PG_INCLUDE_HEAD
>> to makedumpfile, because it will be the same as PG_INCLUDE.
>
> Yes, I thought this was your intention. But the current code (prior to
> this series) doesn't respect tail page decisions, as described in the
> Patch 1 body. The current code makes decisions by compound page, not by
> individual PFN.
>
> It is possible to handle each PFN in the way you suggest. It makes the
> extension API simpler and more flexible, but it makes
> __exclude_unnecessary_pages() more complex:
>
> 1. We need a special case in the middle of the loop to handle
> excluded/included compound tails. In this special case, we need
> special bookkeeping to ensure we decrement the correct PFN counter
> for PG_INCLUDE pages which were previously excluded.
>
> 2. Pending multi-page exclusions at the beginning of a cycle must still
> be performed, but the loop must still process them to ensure that
> extensions have an opportunity to re-include them.
>
> 3. When extensions return PG_INCLUDE on a head page, yet makedumpfile
> would exclude the compound page, we need a special case to ensure
> that we exclude all the tail pages.
>
> When I originally decided to implement it the way shown in this PR
> (calling extensions only for the compound head), I thought that doing it
> your way would be too complex. Today I tried to implement it and found
> that it's not too bad. Here's a version of this patch series which calls
> extensions for each PFN:
>
> https://github.com/makedumpfile/makedumpfile/compare/master...brenns10:makedumpfile:userstack_alternative_api?expand=1
>
> In particular, this commit is the one that matters:
>
> https://github.com/makedumpfile/makedumpfile/commit/a6695155a58ff5a3078e38deeaf768adbc7cbdbf
>
> The nice thing about this is that, with "struct pginfo", extensions can
> now very easily test whether a page is a compound_head or not, without
> relying on re-implementing makedumpfile's version-specific logic. So
> this very nicely allows extensions to implement any exclusion they want:
> they can either repeat their decision for each compound tail, or they
> can do something more complex.
>
> If Kazu is willing to accept this modest complexity for
> __exclude_unnecessary_pages(), then I think it's worth it for the added
> flexibility, and I would be happy to send that instead as v2 of this
> series.
hmm, personally I was thinking that it would be better to process compound
pages as compound page also for extensions, because that's the current
makedumpfile process model and simple, like
PG_EXCLUDE -> exclude a (compound) page
PG_INCLUDE -> include a (compound) page
PG_INCLUDE_HEAD -> include only a head page
But looking at your (Stephen and Tao) extensions, apparently there are
cases where more flexibility is required for size efficiency or accuracy
in excluding/including pages. So I understood that we have to accept a
certain complexity.
For now, I don't think of another way, please proceed with it.
Thanks,
Kazu
>
>> Thanks,
>> Tao Liu
>>
>>>
>>> Thanks,
>>> Kazu
>>>
>>>>
>>>>> + }
>>>>> +
>>>>> /*
>>>>> * Execute exclusion
>>>>> */
>>>>> @@ -8265,6 +8271,7 @@ write_elf_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page)
>>>>> if (info->flag_cyclic) {
>>>>> pfn_zero = pfn_cache = pfn_cache_private = 0;
>>>>> pfn_user = pfn_free = pfn_hwpoison = pfn_offline = pfn_extension = 0;
>>>>> + num_extension_retained = 0;
>>>>> pfn_memhole = info->max_mapnr;
>>>>> }
>>>>>
>>>>> @@ -9610,6 +9617,7 @@ write_kdump_pages_and_bitmap_cyclic(struct cache_data *cd_header, struct cache_d
>>>>> */
>>>>> pfn_zero = pfn_cache = pfn_cache_private = 0;
>>>>> pfn_user = pfn_free = pfn_hwpoison = pfn_offline = pfn_extension = 0;
>>>>> + num_extension_retained = 0;
>>>>> pfn_memhole = info->max_mapnr;
>>>>>
>>>>> /*
>>>>> @@ -10575,6 +10583,7 @@ print_report(void)
>>>>> REPORT_MSG(" Hwpoison pages : 0x%016llx\n", pfn_hwpoison);
>>>>> REPORT_MSG(" Offline pages : 0x%016llx\n", pfn_offline);
>>>>> REPORT_MSG(" Extension filter pages : 0x%016llx\n", pfn_extension);
>>>>> + REPORT_MSG(" Retained by extension : 0x%016llx\n", num_extension_retained);
>>>>> REPORT_MSG(" Remaining pages : 0x%016llx\n",
>>>>> pfn_original - pfn_excluded);
>>>>>
>>>> I suggest to reorder the "print_report" as follows:
>>>>
>>>> Original pages :
>>>> Excluded pages :
>>>> Pages filled with zero :
>>>> Non-private cache pages :
>>>> Private cache pages :
>>>> User process data pages :
>>>> Free pages :
>>>> Hwpoison pages :
>>>> Offline pages :
>>>> Extension filter pages :
>>>> Remaining pages :
>>>> Extension retain pages :
>>>>
>>>> IMHO, this is clearer to represent "Original pages" == "Excluded pages" + "Remaining pages";
>>>> and "Extension retained pages" is a subset of "Remaining pages".
>
> I agree, thank you for that. I will change the order of the lines in v2.
>
> Thank you,
> Stephen
>
>>>> Thanks,
>>>> Tao Liu
>>>>> --
>>>>> 2.47.3
>>>>>
More information about the kexec
mailing list