On 1/30/21 12:45 AM, Georgi Djakov wrote:
> Collect the time when each allocation is freed, to help with memory
> analysis with kdump/ramdump.
>
> Having another timestamp when we free the page helps for debugging
> page migration issues. For example both alloc and free timestamps
> being the same can gave hints that there is an issue with migrating
> memory, as opposed to a page just being dropped during migration.
Besides crash dump analysis, we should also provide this timestamp in the
page_owner file and dump_page()?
> Signed-off-by: Georgi Djakov <georgi.djakov@xxxxxxxxxx>
> ---
> Documentation/vm/page_owner.rst | 2 +-
> mm/page_owner.c | 5 +++++
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/vm/page_owner.rst b/Documentation/vm/page_owner.rst
> index 4e67c2e9bbed..5d7a62c2be28 100644
> --- a/Documentation/vm/page_owner.rst
> +++ b/Documentation/vm/page_owner.rst
> @@ -47,7 +47,7 @@ size change due to this facility.
>
> text data bss dec hex filename
> 48800 2445 644 51889 cab1 mm/page_alloc.o
> - 6574 108 29 6711 1a37 mm/page_owner.o
> + 6644 108 29 6777 1a79 mm/page_owner.o
> 1025 8 8 1041 411 mm/page_ext.o
>
> Although, roughly, 8 KB code is added in total, page_alloc.o increase by
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index d15c7c4994f5..fbdf064e7494 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -27,6 +27,7 @@ struct page_owner {
> depot_stack_handle_t handle;
> depot_stack_handle_t free_handle;
> u64 ts_nsec;
> + u64 free_ts_nsec;
> pid_t pid;
> };
>
> @@ -148,6 +149,7 @@ void __reset_page_owner(struct page *page, unsigned int order)
> struct page_ext *page_ext;
> depot_stack_handle_t handle = 0;
> struct page_owner *page_owner;
> + u64 free_ts_nsec = local_clock();
>
> handle = save_stack(GFP_NOWAIT | __GFP_NOWARN);
>
> @@ -158,6 +160,7 @@ void __reset_page_owner(struct page *page, unsigned int order)
> __clear_bit(PAGE_EXT_OWNER_ALLOCATED, &page_ext->flags);
> page_owner = get_page_owner(page_ext);
> page_owner->free_handle = handle;
> + page_owner->free_ts_nsec = free_ts_nsec;
> page_ext = page_ext_next(page_ext);
> }
> }
> @@ -177,6 +180,7 @@ static inline void __set_page_owner_handle(struct page *page,
> page_owner->last_migrate_reason = -1;
> page_owner->pid = current->pid;
> page_owner->ts_nsec = local_clock();
> + page_owner->free_ts_nsec = 0;
Might be more useful not to reset the time of previous free when the page is
reallocated?
> __set_bit(PAGE_EXT_OWNER, &page_ext->flags);
> __set_bit(PAGE_EXT_OWNER_ALLOCATED, &page_ext->flags);
>
> @@ -243,6 +247,7 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
> new_page_owner->handle = old_page_owner->handle;
> new_page_owner->pid = old_page_owner->pid;
> new_page_owner->ts_nsec = old_page_owner->ts_nsec;
> + new_page_owner->free_ts_nsec = old_page_owner->ts_nsec;
>
> /*
> * We don't clear the bit on the oldpage as it's going to be freed
>
