Received-SPF: pass (google.com: domain of linux-kernel+bounces-63191-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 13 Feb 2024 10:19:45 +0100
From: Oscar Salvador <osalvador@suse.de>
To: Marco Elver <elver@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, Michal Hocko <mhocko@suse.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Andrey Konovalov <andreyknvl@gmail.com>,
	Alexander Potapenko <glider@google.com>
Subject: Re: [PATCH v8 3/5] mm,page_owner: Display all stacks and their count
Message-ID: <Zcs0MdpWwRTv_Uzy@localhost.localdomain>
References: <20240212223029.30769-1-osalvador@suse.de>
 <20240212223029.30769-4-osalvador@suse.de>
 <CANpmjNM8pWMvEYW1YffTjy5xRgyeTOP0sGJoV31pbKU1dH5b9w@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CANpmjNM8pWMvEYW1YffTjy5xRgyeTOP0sGJoV31pbKU1dH5b9w@mail.gmail.com>

On Tue, Feb 13, 2024 at 09:38:43AM +0100, Marco Elver wrote:
> On Mon, 12 Feb 2024 at 23:29, Oscar Salvador <osalvador@suse.de> wrote:
> > Signed-off-by: Oscar Salvador <osalvador@suse.de>
> 
> Acked-by: Marco Elver <elver@google.com>

Thanks!

 
> > +                       /* This pairs with smp_load_acquire() from function
> 
> Comment should be
> 
> /*
>  *
> ...
>  */

Yap, fat fingers here.

> > +       if (!*ppos) {
> > +               /*
> > +                * This pairs with smp_store_release() from function
> > +                * add_stack_record_to_list(), so we get a consistent
> > +                * value of stack_list.
> > +                */
> > +               stack = smp_load_acquire(&stack_list);
> 
> I'm not sure if it'd make your code simpler or not: there is
> <linux/llist.h> for singly-linked linked lists, although the code to
> manage the list is simple enough I'm indifferent here. Only consider
> it if it helps you make the code simpler.

I will check if it eases the code somehow.


> > +static void stack_stop(struct seq_file *m, void *v)
> > +{
> > +}
> 
> Is this function even needed if it's empty? I recall there were some
> boilerplate "nop" functions that could be used.

I will check if seq already provides a dummy function for these cases.

-- 
Oscar Salvador
SUSE Labs