On Wed, Mar 20, 2024 at 01:40:05PM +0900, Tetsuo Handa wrote:
> Hmm, I guess that this is not an expected user of refcount API.
> If it is correct behavior that refcount becomes 0 here, you need to explain like
>
> - refcount_sub_and_test(nr_base_pages, &stack_record->count);
> + if (refcount_sub_and_test(nr_base_pages, &stack_record->count)) {
> + // Explain why nothing to do here, and explain where/how
> + // refcount again becomes positive value using refcount_set().
> + }
>
> or replace refcount_t with atomic_t where it is legal to make refcount positive
> without using atomic_set().
No, it is not expected for the refcount to become 0.
I do know why, but I lost a chunk in the middle of a rebase.
This should have the follwing on top:
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 2613805cb665..e477a71d6adc 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -222,8 +222,11 @@ static void dec_stack_record_count(depot_stack_handle_t handle,
{
struct stack_record *stack_record = __stack_depot_get_stack_record(handle);
- if (stack_record)
- refcount_sub_and_test(nr_base_pages, &stack_record->count);
+ if (!stack_record)
+ return;
+
+ if (refcount_sub_and_test(nr_base_pages, &stack_record->count))
+ WARN(1, "%s refcount went to 0 for %u handle\n", __func__, handle);
}
--
Oscar Salvador
SUSE Labs
