Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 7 Sep 2022 06:00:55 +0200
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>,
        Eric Dumazet <edumazet@google.com>,
        Waiman Long <longman@redhat.com>,
        Suren Baghdasaryan <surenb@google.com>,
        Andrey Konovalov <andreyknvl@gmail.com>,
        Alexander Potapenko <glider@google.com>
Subject: Re: [PATCH v2 2/3] mm, page_owner: Add page_owner_stacks file to
 print out only stacks and their counter
Message-ID: <YxgXd0Z+cqRk7Y7U@localhost.localdomain>
References: <20220905031012.4450-1-osalvador@suse.de>
 <20220905031012.4450-3-osalvador@suse.de>
 <YxXyThZanSl3wboo@elver.google.com>
 <Yxb6PiwBDVuCOp1Q@localhost.localdomain>
 <CANpmjNMNxsoyBqR4U8ybP1ZzNGfkFDcZYJfpiv73wgD=xi6TDA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CANpmjNMNxsoyBqR4U8ybP1ZzNGfkFDcZYJfpiv73wgD=xi6TDA@mail.gmail.com>
Precedence: bulk

On Tue, Sep 06, 2022 at 10:35:00AM +0200, Marco Elver wrote:
> I think it's clear from the fact we're using the stack depot that any
> printing will print stacks. To mirror the existing
> 'stack_depot_print()', I'd go with 'stack_depot_print_all_count()'.

Fair enough, I will rename it then.
 
> Moderately better, but still not great. Essentially you need 2
> cursors, but with loff_t you only get 1.
> 
> I think the loff_t parameter can be used to encode both cursors. In
> the kernel, loff_t is always 'long long', so it'll always be 64-bit.
> 
> Let's assume that collisions in the hash table are rare, so the number
> of stacks per bucket are typically small. Then you can encode the
> index into the bucket in bits 0-31 and the bucket index in bits 32-63.
> STACK_HASH_ORDER_MAX is 20, so 32 bits is plenty to encode the index.

I see, I didn't think of it to be honest.

Then, the below (completely untested) should the trick:

<----
 int stack_depot_print_all_count(char *buf, size_t size, loff_t *pos)
 {
         int ret = 0, stack_i, table_i;
         struct stack_record **stacks, *stack;
         unsigned long stack_table_entries = stack_hash_mask + 1;
 
         stack_i = (*pos & 31);
         table_i = (*pos >> 32);
 new_table:
         stacks = &stack_table[table_i];
         stack = ((struct stack_record *)stacks) + stack_i;
 
         for (; stack; stack = stack->next, stack_i++) {
                 if (!stack->size || stack->size < 0 ||
                     stack->size > size || stack->handle.valid != 1 ||
                     refcount_read(&stack->count) < 1)
                         continue;
 
                 ret += stack_trace_snprint(buf, size, stack->entries, stack->size, 0);
                 ret += scnprintf(buf + ret, size - ret, "stack count: %d\n\n",
                                  refcount_read(&stack->count));
                 *pos |= stack_i;
                 *pos |= ((long long)table_i << 32);
                 return ret;
         }
 
         table_i++;
         /* Keep looking all tables for valid stacks */
         if (table_i < stack_table_entries)
                 goto new_table;
 
         return 0;
 }
---->

I will give it a go.

Thanks Marco!


-- 
Oscar Salvador
SUSE Labs