All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Shakeel Butt <shakeel.butt@linux.dev>
Cc: cgroups@vger.kernel.org, linux-mm@kvack.org
Subject: [bug report] memcg: multi-memcg percpu charge cache - fix 2
Date: Wed, 30 Apr 2025 11:09:50 +0300	[thread overview]
Message-ID: <aBHazntT1ypcMPfD@stanley.mountain> (raw)

Hello Shakeel Butt,

Commit 1db4ee9862f9 ("memcg: multi-memcg percpu charge cache - fix
2") from Apr 25, 2025 (linux-next), leads to the following Smatch
static checker warning:

	mm/memcontrol.c:1959 refill_stock()
	error: uninitialized symbol 'stock_pages'.

mm/memcontrol.c
    1907 static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
    1908 {
    1909         struct memcg_stock_pcp *stock;
    1910         struct mem_cgroup *cached;
    1911         uint8_t stock_pages;
                         ^^^^^^^^^^^

    1912         unsigned long flags;
    1913         bool success = false;
    1914         int empty_slot = -1;
    1915         int i;
    1916 
    1917         /*
    1918          * For now limit MEMCG_CHARGE_BATCH to 127 and less. In future if we
    1919          * decide to increase it more than 127 then we will need more careful
    1920          * handling of nr_pages[] in struct memcg_stock_pcp.
    1921          */
    1922         BUILD_BUG_ON(MEMCG_CHARGE_BATCH > S8_MAX);
    1923 
    1924         VM_WARN_ON_ONCE(mem_cgroup_is_root(memcg));
    1925 
    1926         if (nr_pages > MEMCG_CHARGE_BATCH ||
    1927             !local_trylock_irqsave(&memcg_stock.stock_lock, flags)) {
    1928                 /*
    1929                  * In case of larger than batch refill or unlikely failure to
    1930                  * lock the percpu stock_lock, uncharge memcg directly.
    1931                  */
    1932                 memcg_uncharge(memcg, nr_pages);
    1933                 return;
    1934         }
    1935 
    1936         stock = this_cpu_ptr(&memcg_stock);
    1937         for (i = 0; i < NR_MEMCG_STOCK; ++i) {
    1938                 cached = READ_ONCE(stock->cached[i]);
    1939                 if (!cached && empty_slot == -1)
    1940                         empty_slot = i;
    1941                 if (memcg == READ_ONCE(stock->cached[i])) {
    1942                         stock_pages = READ_ONCE(stock->nr_pages[i]) + nr_pages;
    1943                         WRITE_ONCE(stock->nr_pages[i], stock_pages);
    1944                         if (stock_pages > MEMCG_CHARGE_BATCH)
    1945                                 drain_stock(stock, i);
    1946                         success = true;
                                 ^^^^^^^^^^^^^^
When stock_pages is initialized then success is true.

    1947                         break;
    1948                 }
    1949         }
    1950 
    1951         if (!success) {
                     ^^^^^^^^
success is false.

    1952                 i = empty_slot;
    1953                 if (i == -1) {
    1954                         i = get_random_u32_below(NR_MEMCG_STOCK);
    1955                         drain_stock(stock, i);
    1956                 }
    1957                 css_get(&memcg->css);
    1958                 WRITE_ONCE(stock->cached[i], memcg);
--> 1959                 WRITE_ONCE(stock->nr_pages[i], stock_pages);
                                                        ^^^^^^^^^^^
This is always uninitialized at this point.  Probably on your test system
you are automatically initializing stack variables to zero.

    1960         }
    1961 
    1962         local_unlock_irqrestore(&memcg_stock.stock_lock, flags);
    1963 }

regards,
dan carpenter

             reply	other threads:[~2025-04-30  8:09 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-30  8:09 Dan Carpenter [this message]
2025-04-30  8:25 ` [bug report] memcg: multi-memcg percpu charge cache - fix 2 Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aBHazntT1ypcMPfD@stanley.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=cgroups@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=shakeel.butt@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.