All of lore.kernel.org
 help / color / mirror / Atom feed
From: <dan.j.williams@intel.com>
To: Dave Jiang <dave.jiang@intel.com>,
	Dan Williams <dan.j.williams@intel.com>
Cc: <linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<Smita.KoralahalliChannabasappa@amd.com>,
	<alison.schofield@intel.com>, <terry.bowman@amd.com>,
	<alejandro.lucero-palau@amd.com>, <linux-pci@vger.kernel.org>,
	<Jonathan.Cameron@huawei.com>, Shiju Jose <shiju.jose@huawei.com>
Subject: Re: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion
Date: Thu, 4 Dec 2025 12:15:18 -0800	[thread overview]
Message-ID: <6931ebd626df6_1e021001d@dwillia2-mobl4.notmuch> (raw)
In-Reply-To: <2010f8dc-7cc6-4a42-a181-37e65fffd22f@intel.com>

Dave Jiang wrote:
> 
> 
> On 12/3/25 7:21 PM, Dan Williams wrote:
> > A device release method is only for undoing allocations on the path to
> > preparing the device for device_add(). In contrast, devm allocations are
> > post device_add(), are acquired during / after ->probe() and are released
> > synchronous with ->remove().
> > 
> > So, a "devm" helper in a "release" method is a clear anti-pattern.
> > 
> > Move this devm release action where it belongs, an action created at edac
> > object creation time. Otherwise, this leaks resources until
> > cxl_memdev_release() time which may be long after these xarray and error
> > record caches have gone idle.
> > 
> > Note, this also fixes up the type of @cxlmd->err_rec_array which needlessly
> > dropped type-safety.
> > 
> > Fixes: 0b5ccb0de1e2 ("cxl/edac: Support for finding memory operation attributes from the current boot")
> > Cc: Dave Jiang <dave.jiang@intel.com>
> > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Shiju Jose <shiju.jose@huawei.com>
> > Cc: Alison Schofield <alison.schofield@intel.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> 
> This can go into fixes right? Does not look like a dependency for the rest of the series.

It could go into fixes, but until someone reports a practical problem
the urgency is low. The impacts of the leak are minor especially on
typical builds where CONFIG_DEBUG_KOBJECT_RELEASE is disabled. The main
motivation to push it along is to interdict any further attempts to make
the "release" vs "devm" mistake.

  reply	other threads:[~2025-12-04 20:15 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-04  2:21 [PATCH 0/6] cxl: Initialization reworks in support Soft Reserve Recovery and Accelerator Memory Dan Williams
2025-12-04  2:21 ` [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion Dan Williams
2025-12-04 16:48   ` Dave Jiang
2025-12-04 20:15     ` dan.j.williams [this message]
2025-12-04 19:09   ` Cheatham, Benjamin
2025-12-05  2:46   ` Alison Schofield
2025-12-08 14:19   ` Alejandro Lucero Palau
2025-12-15 21:11     ` dan.j.williams
2025-12-08 19:20   ` Shiju Jose
2025-12-15 12:00   ` Jonathan Cameron
2025-12-04  2:21 ` [PATCH 2/6] cxl/mem: Arrange for always-synchronous memdev attach Dan Williams
2025-12-04 16:58   ` Dave Jiang
2025-12-04 19:09   ` Cheatham, Benjamin
2025-12-05  2:49   ` Alison Schofield
2025-12-15 12:08   ` Jonathan Cameron
2025-12-04  2:21 ` [PATCH 3/6] cxl/port: Arrange for always synchronous endpoint attach Dan Williams
2025-12-04 18:36   ` Dave Jiang
2025-12-04 19:09   ` Cheatham, Benjamin
2025-12-05  3:36   ` Alison Schofield
2025-12-15 12:09   ` Jonathan Cameron
2025-12-04  2:21 ` [PATCH 4/6] cxl/mem: Convert devm_cxl_add_memdev() to scope-based-cleanup Dan Williams
2025-12-04 18:58   ` Dave Jiang
2025-12-04 19:09   ` Cheatham, Benjamin
2025-12-04 20:50     ` dan.j.williams
2025-12-05  3:37   ` Alison Schofield
2025-12-04  2:21 ` [PATCH 5/6] cxl/mem: Drop @host argument to devm_cxl_add_memdev() Dan Williams
2025-12-04 19:09   ` Cheatham, Benjamin
2025-12-04 20:02   ` Dave Jiang
2025-12-05  3:38   ` Alison Schofield
2025-12-15 12:15   ` Jonathan Cameron
2025-12-04  2:21 ` [PATCH 6/6] cxl/mem: Introduce a memdev creation ->probe() operation Dan Williams
2025-12-04 19:10   ` Cheatham, Benjamin
2025-12-04 21:11     ` dan.j.williams
2025-12-04 22:02       ` dan.j.williams
2025-12-04 22:15         ` Cheatham, Benjamin
2025-12-04 20:03   ` Dave Jiang
2025-12-05 15:15 ` [PATCH 0/6] cxl: Initialization reworks in support Soft Reserve Recovery and Accelerator Memory Alejandro Lucero Palau
2025-12-05 21:17   ` dan.j.williams
2025-12-08 14:04     ` Alejandro Lucero Palau
2025-12-09  7:53       ` dan.j.williams
2025-12-08 17:04 ` Alejandro Lucero Palau
2025-12-15 23:29   ` dan.j.williams

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=6931ebd626df6_1e021001d@dwillia2-mobl4.notmuch \
    --to=dan.j.williams@intel.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=Smita.KoralahalliChannabasappa@amd.com \
    --cc=alejandro.lucero-palau@amd.com \
    --cc=alison.schofield@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=shiju.jose@huawei.com \
    --cc=terry.bowman@amd.com \
    /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.