All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Chen, Yu C" <yu.c.chen@intel.com>
To: Reinette Chatre <reinette.chatre@intel.com>, Chen Yu <chen.yu@linux.dev>
Cc: <tony.luck@intel.com>, <tglx@kernel.org>, <bp@alien8.de>,
	<mingo@redhat.com>, <dave.hansen@linux.intel.com>,
	<hpa@zytor.com>, <fenghuay@nvidia.com>, <babu.moger@amd.com>,
	<x86@kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v6 3/9] x86/resctrl: Parse ACPI ERDT table and save CACD cpumask for RMDD domains
Date: Wed, 26 Aug 2026 00:18:51 +0800	[thread overview]
Message-ID: <993f64a0-40b2-4ea6-8604-c1475bad7816@intel.com> (raw)
In-Reply-To: <9229fbc4-812d-4747-8d2f-f5db3179037f@intel.com>

On 8/24/2026 11:54 PM, Reinette Chatre wrote:
> Hi Chenyu,
> 

[ ... ]

> 
> Yes, code readability is a priority. It is not all just about code readability. Please consider
> all the points I mentioned above. I do not see any justification for get_rdt_resources()
> leaving dangling state on failure. This is something that will get tripped over in the next
> inevitable refactor. Apart from that, adding a single "cleanup" function at one location down in
> the call stack is convenient for *this* implementation based on the *current* state of the code but
> while doing so it breaks custom, not just of resctrl but of the rest of the kernel also, and because
> of that makes this code difficult to build on and maintain.
> 
	goto out;
> 
> Above is the issue I mentioned earlier where get_rdt_resources() leaves dangling state.
> If it cleaned up after itself on failure as is the custom in the kernel then
> resctrl_arch_late_init() does not have to.
> 
> If the goal is to avoid goto in __resctrl_arch_late_init() then the failure paths that
> follow get_rdt_resources() can just call erdt_exit() directly to make obvious where what
> needs to be cleaned up.
> 
> As the suggestion is implemented the cleanup is inconsistent with __resctrl_arch_late_init()
> doing its own cleanup for code run _after_ erdt_init() and leaving the erdt_init() cleanup
> to be done further down the stack by __resctrl_arch_late_init()'s caller. While technically
> correct this inconsistency adds unnecessary complexity and another place that will get tripped
> over in the next inevitable refactor.
> 

I see. This is very helpful guidance. I'll adjust the code accordingly.

thanks,
Chenyu

  reply	other threads:[~2026-08-25 16:19 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-25  9:20 [PATCH v6 0/9] Introduce MMIO-based CMT access for Enhanced RDT Chen Yu
2026-07-25  9:22 ` [PATCH v6 1/9] x86/topology: Export topo_lookup_cpuid() for resctrl use Chen Yu
2026-08-19 22:55   ` Reinette Chatre
2026-08-22  4:16     ` Chen Yu
2026-07-25  9:22 ` [PATCH v6 2/9] x86/resctrl: Require 64-bit x86 for resctrl support Chen Yu
2026-08-19 22:55   ` Reinette Chatre
2026-08-20 15:20     ` Luck, Tony
2026-08-20 15:54       ` Reinette Chatre
2026-08-20 17:01         ` Luck, Tony
2026-08-20 17:12           ` Dave Hansen
2026-08-20 17:48             ` Reinette Chatre
2026-08-21  2:37               ` Borislav Petkov
2026-08-21 15:47                 ` Reinette Chatre
2026-08-21 15:54                   ` Borislav Petkov
2026-08-25 13:12                   ` Chen Yu
2026-08-24 14:18                     ` Dave Hansen
2026-08-24 15:15                       ` Chen, Yu C
2026-08-25  2:40                       ` Borislav Petkov
2026-08-21 11:28             ` Peter Zijlstra
2026-08-21 15:52               ` Borislav Petkov
2026-08-21 16:58                 ` Luck, Tony
2026-08-22  0:04                   ` Borislav Petkov
2026-07-25  9:22 ` [PATCH v6 3/9] x86/resctrl: Parse ACPI ERDT table and save CACD cpumask for RMDD domains Chen Yu
2026-08-19 23:01   ` Reinette Chatre
2026-08-25  8:06     ` Chen Yu
2026-08-24 15:54       ` Reinette Chatre
2026-08-25 16:18         ` Chen, Yu C [this message]
2026-07-25  9:23 ` [PATCH v6 4/9] x86/resctrl: Attach ACPI ERDT information to L3 mon domain on CPU online Chen Yu
2026-08-19 23:04   ` Reinette Chatre
2026-08-25  5:54     ` Chen, Yu C
2026-07-25  9:23 ` [PATCH v6 5/9] x86/resctrl: Parse ACPI CMRC table Chen Yu
2026-08-19 23:06   ` Reinette Chatre
2026-08-25  9:19     ` Chen, Yu C
2026-08-25 15:39       ` Reinette Chatre
2026-08-25 16:11         ` Chen, Yu C
2026-08-25 16:38           ` Luck, Tony
2026-07-25  9:23 ` [PATCH v6 6/9] x86/resctrl: Refactor the monitor read function Chen Yu
2026-08-19 23:07   ` Reinette Chatre
2026-08-25 10:03     ` Chen, Yu C
2026-07-25  9:23 ` [PATCH v6 7/9] fs/resctrl: Do not invoke smp_processor_id() in preemptible context Chen Yu
2026-08-19 23:08   ` Reinette Chatre
2026-08-25 11:17     ` Chen, Yu C
2026-07-25  9:23 ` [PATCH v6 8/9] x86/resctrl: Introduce erdt_cpu_has() and erdt_support() Chen Yu
2026-08-19 23:08   ` Reinette Chatre
2026-08-25 11:55     ` Chen, Yu C
2026-07-25  9:23 ` [PATCH v6 9/9] x86/resctrl: Add MMIO-based LLC occupancy monitoring support Chen Yu
2026-08-19 23:10   ` Reinette Chatre
2026-08-25 16:11     ` Chen, Yu C
2026-08-25 17:10       ` Reinette Chatre
2026-08-13  6:43 ` [PATCH v6 0/9] Introduce MMIO-based CMT access for Enhanced RDT Chen Yu

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=993f64a0-40b2-4ea6-8604-c1475bad7816@intel.com \
    --to=yu.c.chen@intel.com \
    --cc=babu.moger@amd.com \
    --cc=bp@alien8.de \
    --cc=chen.yu@linux.dev \
    --cc=dave.hansen@linux.intel.com \
    --cc=fenghuay@nvidia.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=reinette.chatre@intel.com \
    --cc=tglx@kernel.org \
    --cc=tony.luck@intel.com \
    --cc=x86@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.