All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Dennis Zhou <dennis@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Tejun Heo <tj@kernel.org>, Christoph Lameter <cl@linux.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] percpu: add basic double free check
Date: Mon, 19 Jan 2026 08:48:13 +0100	[thread overview]
Message-ID: <20260119074813.ecAFsGaT@linutronix.de> (raw)
In-Reply-To: <aWsa9SdO9kLJTfz4@snowbird>

On 2026-01-16 21:15:33 [-0800], Dennis Zhou wrote:
> > The patch does appear to do that which it set out to do.  But do we
> > want to do it?  Is there a history of callers double-freeing percpu
> > memory?  Was there some bug which would have been more rapidly and
> > easily solved had this change been in place?
> > 
> 
> Originally, Sebastian posted he ran into the issue where he double freed
> in [1] (linked in patch). Maybe he can elaborate how that bug was
> introduced.
> 
> Wrt do we want to do it - I think it doesn't hurt and makes it more
> explicit that something very wrong occurred. Percpu memory really 
> expects users to be good samaritans. If you do happen to accidentally
> double free without the warning, in a contrived case you could
> experience unexplained behavior for some time before crashing in a spot
> that would leave your head scratching. If anything I think there could
> be an argument to fail louder.

I did write some code and there was a free path I did not expect to
happen. While it was rare to happen, it did not always crash right away.
Once I had the pattern I was wondering why none of the memory debug
switches did something. 
Adding this does not look expensive, allocating per-CPU is hardly done
in a hot path so I did not add anything but I don't mind hiding it
behind a config switch similar to SLAB_DEBUG.
While we do have way less per-CPU allocations compared to SLAB, I think
it is important to have some kind of sanity checks for it to ensure it
is used properly.
I would go with a WARN_ON_ONCE but if there is a desire for rate limited
multiple warnings, fine.

> [1] https://lore.kernel.org/linux-mm/20250904143514.Yk6Ap-jy@linutronix.de/
> 
> Thanks,
> Dennis

Sebastian


      parent reply	other threads:[~2026-01-19  7:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-16  2:32 [PATCH v2] percpu: add basic double free check Dennis Zhou
2026-01-17  3:15 ` Andrew Morton
2026-01-17  5:15   ` Dennis Zhou
2026-01-18  1:09     ` Andrew Morton
2026-01-19  7:48     ` Sebastian Andrzej Siewior [this message]

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=20260119074813.ecAFsGaT@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=dennis@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=tj@kernel.org \
    /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.