All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luck, Tony" <tony.luck@intel.com>
To: Reinette Chatre <reinette.chatre@intel.com>
Cc: Fenghua Yu <fenghuay@nvidia.com>,
	"Wieczor-Retman, Maciej" <maciej.wieczor-retman@intel.com>,
	Peter Newman <peternewman@google.com>,
	James Morse <james.morse@arm.com>,
	Babu Moger <babu.moger@amd.com>,
	"Drew Fustini" <dfustini@baylibre.com>,
	Dave Martin <Dave.Martin@arm.com>,
	"Chen, Yu C" <yu.c.chen@intel.com>,
	"Box, David E" <david.e.box@intel.com>,
	"x86@kernel.org" <x86@kernel.org>,
	Christoph Hellwig <hch@infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"patches@lists.linux.dev" <patches@lists.linux.dev>
Subject: Re: [PATCH v7 11/14] mpam,x86/resctrl: Resolve INTEL_PMT_TELEMETRY symbols at runtime
Date: Thu, 11 Jun 2026 15:27:16 -0700	[thread overview]
Message-ID: <ais2ROlQe9JqsN3w@agluck-desk3> (raw)
In-Reply-To: <84004bd5-db4d-4abc-a676-6565abf6320d@intel.com>

On Thu, Jun 11, 2026 at 02:22:23PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 6/11/26 11:01 AM, Luck, Tony wrote:
> > With renamed variable, and comment. Is this clearer?
> > 
> > 
> > /*
> >  * intel_aet_register_enumeration() provides the value of "THIS_MODULE"
> >  * from the the pmt_telemetry module. Note that this is NULL when
> >  * CONFIG_INTEL_PMT_TELEMETRY=y so it serves as a flag to decide whether
> >  * try_module_get() and module_put() need to be called.
> >  */
> > static struct module *pmt_is_a_module;
> > 
> 
> Using a single variable to contain both data and state has caused problem in
> the past. Could it be helpful to create two utilities, for example get_pmt()
> and put_pmt() although naming is not at issue at the moment, that contains
> and makes obvious these "is INTEL_PMT_TELEMETRY a module or not" checks? These would
> then be no-op when INTEL_PMT_TELEMETRY=y. This would simplify the code in the
> callers by containing this logic and not rely on callers doing this check so
> many times.

I can code this up to see how it looks.

> Even so, it is not clear to me how the AET lifetime will look like since 
> INTEL_PMT_TELEMETRY's .remove() may be called at other times so resctrl
> may need other hooks that could replace this module handling.

Case 1: No races

Resctrl mount: intel_aet_pre_mount() gets a success return from
try_module_get() - so the hold on the pmt_module will block the
kernel from removing the module (-EBUSY).

Case 2: Race between resctrl mount and rmmod pmt_telemetry

Mount code gets into intel_aet_pre_mount() and rmmod gets to intel_aet_unregister_enumeration()
where both try to acquire aet_register_lock.

	If intel_aet_pre_mount() gets the mutex it calls try_module_get(), which
	fails because if the pmt_telemetry code is in the .remove() functions,
	then the module state is already in MODULE_STATE_GOING.

	If rmmod process wins the mutex it sets get_feature and put_feature to
	NULL and releases the mutex. intel_aet_pre_mount() runs and returns
	false because get_feature and put_feature are NULL.

Is there some other race condition to consider?

> Reinette

-Tony

P.S. David Box is OK with setting suppress_bind_attrs for the telemetry
driver. So that will make the unbind issue moot.
> 

  reply	other threads:[~2026-06-11 22:27 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-01 19:56 [PATCH v7 00/14] Allow AET to use PMT as loadable module Tony Luck
2026-06-01 19:56 ` [PATCH v7 01/14] fs/resctrl: Move functions to avoid forward references in subsequent fixes Tony Luck
2026-06-01 19:56 ` [PATCH v7 02/14] fs/resctrl: Free mon_data structures on rdt_get_tree() failure Tony Luck
2026-06-01 19:56 ` [PATCH v7 03/14] fs/resctrl: Fix use-after-free during unmount Tony Luck
2026-06-01 19:56 ` [PATCH v7 04/14] fs/resctrl: Fix deadlock for errors during mount Tony Luck
2026-06-01 19:56 ` [PATCH v7 05/14] x86/resctrl: Stop setting event_group::force_off on RMID shortage Tony Luck
2026-06-08 23:16   ` Reinette Chatre
2026-06-09 16:51     ` Luck, Tony
2026-06-09 23:02       ` Reinette Chatre
2026-06-10 20:01         ` Luck, Tony
2026-06-01 19:56 ` [PATCH v7 06/14] fs/resctrl: Add interface to disable a monitor event Tony Luck
2026-06-08 23:18   ` Reinette Chatre
2026-06-09 17:21     ` Luck, Tony
2026-06-09 23:02       ` Reinette Chatre
2026-06-10 20:56         ` Luck, Tony
2026-06-10 22:26           ` Reinette Chatre
2026-06-10 23:19             ` Luck, Tony
2026-06-11 21:22               ` Reinette Chatre
2026-06-01 19:56 ` [PATCH v7 07/14] x86/resctrl: Maintain a count of enabled monitor features Tony Luck
2026-06-08 23:18   ` Reinette Chatre
2026-06-09 18:46     ` Luck, Tony
2026-06-09 23:03       ` Reinette Chatre
2026-06-11 17:27         ` Luck, Tony
2026-06-01 19:56 ` [PATCH v7 08/14] fs,x86,mpam/resctrl: Handle change in number of RMIDs on each mount Tony Luck
2026-06-08 23:21   ` Reinette Chatre
2026-06-09 21:58     ` Luck, Tony
2026-06-09 23:35       ` Reinette Chatre
2026-06-11 17:40         ` Luck, Tony
2026-06-01 19:56 ` [PATCH v7 09/14] x86/resctrl: Add PMT registration API for AET enumeration callbacks Tony Luck
2026-06-08 23:21   ` Reinette Chatre
2026-06-01 19:56 ` [PATCH v7 10/14] platform/x86/intel/pmt: Register enumeration functions with resctrl Tony Luck
2026-06-08 23:22   ` Reinette Chatre
2026-06-09 22:11     ` Luck, Tony
2026-06-18 21:15     ` Luck, Tony
2026-06-22 15:46       ` Reinette Chatre
2026-06-22 23:00         ` Luck, Tony
2026-06-23 15:45           ` Reinette Chatre
2026-06-23 18:24             ` Luck, Tony
2026-06-01 19:56 ` [PATCH v7 11/14] mpam,x86/resctrl: Resolve INTEL_PMT_TELEMETRY symbols at runtime Tony Luck
2026-06-08 23:25   ` Reinette Chatre
2026-06-10  0:08     ` Luck, Tony
2026-06-10 15:27       ` Reinette Chatre
2026-06-10 15:49         ` Luck, Tony
2026-06-10 16:21           ` Reinette Chatre
2026-06-10 16:34             ` Luck, Tony
2026-06-10 16:46               ` Reinette Chatre
2026-06-10 17:24                 ` Luck, Tony
2026-06-10 17:58                   ` Reinette Chatre
2026-06-10 22:09                     ` Luck, Tony
2026-06-11 18:01                       ` Luck, Tony
2026-06-11 21:22                         ` Reinette Chatre
2026-06-11 22:27                           ` Luck, Tony [this message]
2026-06-12 18:04                             ` Luck, Tony
2026-06-01 19:56 ` [PATCH v7 12/14] fs/resctrl: Call architecture hooks for every mount/unmount Tony Luck
2026-06-08 23:26   ` Reinette Chatre
2026-06-10 16:16     ` Luck, Tony
2026-06-01 19:56 ` [PATCH v7 13/14] x86/resctrl: Simplify Kconfig options for resctrl Tony Luck
2026-06-01 19:56 ` [PATCH v7 14/14] Documentation/filesystems/resctrl: Add footnote for telemetry fstab mount caveat Tony Luck
2026-06-08 23:26   ` Reinette Chatre
2026-06-10 16:19     ` Luck, Tony

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=ais2ROlQe9JqsN3w@agluck-desk3 \
    --to=tony.luck@intel.com \
    --cc=Dave.Martin@arm.com \
    --cc=babu.moger@amd.com \
    --cc=david.e.box@intel.com \
    --cc=dfustini@baylibre.com \
    --cc=fenghuay@nvidia.com \
    --cc=hch@infradead.org \
    --cc=james.morse@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maciej.wieczor-retman@intel.com \
    --cc=patches@lists.linux.dev \
    --cc=peternewman@google.com \
    --cc=reinette.chatre@intel.com \
    --cc=x86@kernel.org \
    --cc=yu.c.chen@intel.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.