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 <[email protected]>
>> ---
>> 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;
(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;
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".
>
> Thanks,
> Tao Liu
>> --
>> 2.47.3
>>
