All of lore.kernel.org
 help / color / mirror / Atom feed
From: Reinette Chatre <reinette.chatre@intel.com>
To: James Morse <james.morse@arm.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Cc: Fenghua Yu <fenghua.yu@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	H Peter Anvin <hpa@zytor.com>, Babu Moger <Babu.Moger@amd.com>,
	shameerali.kolothum.thodi@huawei.com,
	Jamie Iles <jamie@nuviainc.com>,
	D Scott Phillips OS <scott@os.amperecomputing.com>
Subject: Re: [PATCH v2 02/24] x86/resctrl: Split struct rdt_domain
Date: Thu, 8 Apr 2021 13:13:01 -0700	[thread overview]
Message-ID: <905a2bbf-f117-4b20-b875-dc1133d83f0d@intel.com> (raw)
In-Reply-To: <c8c1a559-9045-a28c-46c0-dddeb9b38a14@arm.com>

Hi James,

On 4/8/2021 10:20 AM, James Morse wrote:
> On 31/03/2021 22:36, Reinette Chatre wrote:
>> On 3/12/2021 9:58 AM, James Morse wrote:
>>> resctrl is the defacto Linux ABI for SoC resource partitioning features.
>>> To support it on another architecture, it needs to be abstracted from
>>> the features provided by Intel RDT and AMD PQoS, and moved to /fs/.
>>>
>>> Split struct rdt_domain up too. Move everything that that is particular
>>
>> s/that that/that/
>>
>>> to resctrl into a new header file. resctrl code paths touching a 'hw'
>>> struct indicates where an abstraction is needed.
>>
>> Similar to previous patch it would help to explain how this split was chosen. For example,
>> why are the CPUs to which a resource is associated not considered a hardware property?
> 
> Similarly, because the meaning of those CPUs doesn't differ or change between architectures.

Got it. This seems to be a repeat of the discussion about patch 1. 
Please note that the description of rdt_hw_domain in this patch reads 
"hw attributes of a group of CPUs that share a resource". This can be 
understood to mean that the struct contains attributes discovered from 
hardware.


> I've expanded the middle paragraph in the commit message to explain why the arch specific
> things are arch specific:
> | Continue by splitting struct rdt_domain, into an architecture private
> | 'hw' struct, which contains the common resctrl structure that would be
> | used by any architecture.
> |
> | The hardware values in ctrl_val and mbps_val need to be accessed via
> | helpers to allow another architecture to convert these into a different
> | format if necessary.
> |
> | After this split, filesystem code code paths touching a 'hw' struct
> | indicates where an abstraction is needed.
> 
> and similarly changed the kernel doc comment.

Thank you. I assume this includes changing the "hw attributes of a group 
of CPUs that share a resource" I mention above.

> 
> Let me know if you prefer some other struct name.
> 

I am ok with current naming. Other folks may have better ideas.

Thank you

Reinette


  reply	other threads:[~2021-04-08 20:13 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-12 17:58 [PATCH v2 00/24] x86/resctrl: Merge the CDP resources James Morse
2021-03-12 17:58 ` [PATCH v2 01/24] x86/resctrl: Split struct rdt_resource James Morse
2021-03-31 21:35   ` Reinette Chatre
2021-04-06 17:13     ` James Morse
2021-04-06 23:42       ` Reinette Chatre
2021-04-08 17:20         ` James Morse
2021-04-08 20:04           ` Reinette Chatre
2021-03-12 17:58 ` [PATCH v2 02/24] x86/resctrl: Split struct rdt_domain James Morse
2021-03-31 21:36   ` Reinette Chatre
2021-04-08 17:20     ` James Morse
2021-04-08 20:13       ` Reinette Chatre [this message]
2021-03-12 17:58 ` [PATCH v2 03/24] x86/resctrl: Add a separate schema list for resctrl James Morse
2021-03-31 21:37   ` Reinette Chatre
2021-03-12 17:58 ` [PATCH v2 04/24] x86/resctrl: Pass the schema in info dir's private pointer James Morse
2021-03-12 17:58 ` [PATCH v2 05/24] x86/resctrl: Label the resources with their configuration type James Morse
2021-03-31 21:37   ` Reinette Chatre
2021-03-12 17:58 ` [PATCH v2 06/24] x86/resctrl: Walk the resctrl schema list instead of an arch list James Morse
2021-03-31 21:38   ` Reinette Chatre
2021-03-12 17:58 ` [PATCH v2 07/24] x86/resctrl: Store the effective num_closid in the schema James Morse
2021-03-31 21:39   ` Reinette Chatre
2021-03-12 17:58 ` [PATCH v2 08/24] x86/resctrl: Add resctrl_arch_get_num_closid() James Morse
2021-03-12 17:58 ` [PATCH v2 09/24] x86/resctrl: Pass the schema to resctrl filesystem functions James Morse
2021-03-12 17:58 ` [PATCH v2 10/24] x86/resctrl: Swizzle rdt_resource and resctrl_schema in pseudo_lock_region James Morse
2021-03-31 21:39   ` Reinette Chatre
2021-03-12 17:58 ` [PATCH v2 11/24] x86/resctrl: Move the schemata names into struct resctrl_schema James Morse
2021-03-12 17:58 ` [PATCH v2 12/24] x86/resctrl: Group staged configuration into a separate struct James Morse
2021-03-12 17:58 ` [PATCH v2 13/24] x86/resctrl: Allow different CODE/DATA configurations to be staged James Morse
2021-03-12 17:58 ` [PATCH v2 14/24] x86/resctrl: Rename update_domains() resctrl_arch_update_domains() James Morse
2021-03-12 17:58 ` [PATCH v2 15/24] x86/resctrl: Add a helper to read a closid's configuration James Morse
2021-03-12 17:58 ` [PATCH v2 16/24] x86/resctrl: Add a helper to read/set the CDP configuration James Morse
2021-03-12 17:58 ` [PATCH v2 17/24] x86/resctrl: Use cdp_enabled in rdt_domain_reconfigure_cdp() James Morse
2021-03-12 17:58 ` [PATCH v2 18/24] x86/resctrl: Pass configuration type to resctrl_arch_get_config() James Morse
2021-03-12 17:58 ` [PATCH v2 19/24] x86/resctrl: Make ctrlval arrays the same size James Morse
2021-03-12 17:58 ` [PATCH v2 20/24] x86/resctrl: Apply offset correction when config is staged James Morse
2021-03-12 17:58 ` [PATCH v2 21/24] x86/resctrl: Calculate the index from the configuration type James Morse
2021-03-12 17:58 ` [PATCH v2 22/24] x86/resctrl: Merge the ctrl_val arrays James Morse
2021-03-12 17:58 ` [PATCH v2 23/24] x86/resctrl: Remove rdt_cdp_peer_get() James Morse
2021-03-12 17:58 ` [PATCH v2 24/24] x86/resctrl: Merge the CDP resources James Morse
2021-03-30 20:36 ` [PATCH v2 00/24] " Babu Moger
2021-04-06 17:19   ` James Morse
2021-04-06 21:37     ` Babu Moger
2021-04-08 17:19       ` James Morse

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=905a2bbf-f117-4b20-b875-dc1133d83f0d@intel.com \
    --to=reinette.chatre@intel.com \
    --cc=Babu.Moger@amd.com \
    --cc=bp@alien8.de \
    --cc=fenghua.yu@intel.com \
    --cc=hpa@zytor.com \
    --cc=james.morse@arm.com \
    --cc=jamie@nuviainc.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=scott@os.amperecomputing.com \
    --cc=shameerali.kolothum.thodi@huawei.com \
    --cc=tglx@linutronix.de \
    --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.