Re: [PATCH v2 1/2] mm,page_owner: Fix refcount imbalance

[Oscar Salvador <osalvador@suse.de>]

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