All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Vikas Shivappa <vikas.shivappa@linux.intel.com>
Cc: vikas.shivappa@intel.com, x86@kernel.org,
	linux-kernel@vger.kernel.org, hpa@zytor.com, mingo@kernel.org,
	peterz@infradead.org, ravi.v.shankar@intel.com,
	tony.luck@intel.com, fenghua.yu@intel.com,
	h.peter.anvin@intel.com
Subject: Re: [PATCH 4/8] x86/intel_rct/mba: Add MBA structures and initialize MBA
Date: Wed, 5 Apr 2017 17:40:35 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.20.1704051731200.2803@nanos> (raw)
In-Reply-To: <1491256652-18729-5-git-send-email-vikas.shivappa@linux.intel.com>

On Mon, 3 Apr 2017, Vikas Shivappa wrote:
>  
>  /**
> + * struct rdt_domain - group of cpus sharing an RDT resource
> + * @list:	all instances of this resource
> + * @id:		unique id for this instance
> + * @cpu_mask:	which cpus share this resource
> + * @ctrl_val:	array of cache or mem ctrl values (indexed by CLOSID)
> + * @new_cbm:	new cbm value to be loaded
> + * @have_new_cbm: did user provide new_cbm for this domain

The version which you removed below has the kernel-doc comments correct ....

> +/**
>   * struct rdt_resource - attributes of an RDT resource
>   * @enabled:			Is this feature enabled on this machine
>   * @capable:			Is this feature available on this machine
> @@ -78,6 +109,16 @@ struct rftype {
>   * @data_width:		Character width of data when displaying
>   * @min_cbm_bits:		Minimum number of consecutive bits to be set
>   *				in a cache bit mask
> + * @msr_update:		Function pointer to update QOS MSRs
> + * @max_delay:			Max throttle delay. Delay is the hardware
> + *				understandable value for memory bandwidth.
> + * @min_bw:			Minimum memory bandwidth percentage user
> + *				can request
> + * @bw_gran:			Granularity at which the memory bandwidth
> + *				is allocated
> + * @delay_linear:		True if memory b/w delay is in linear scale
> + * @mb_map:			Mapping of memory b/w percentage to
> + *				memory b/w delay values
>   * @domains:			All domains for this resource
>   * @msr_base:			Base MSR address for CBMs
>   * @cache_level:		Which cache level defines scope of this domain
> @@ -94,6 +135,14 @@ struct rdt_resource {
>  	int			min_cbm_bits;
>  	u32			default_ctrl;
>  	int			data_width;
> +	void (*msr_update)	(struct rdt_domain *d, struct msr_param *m,
> +				 struct rdt_resource *r);
> +	u32			max_delay;
> +	u32			min_bw;
> +	u32			bw_gran;
> +	u32			delay_linear;
> +	u32			*mb_map;

I don't know what other weird controls will be added over time, but we are
probably better off to have

struct cache_ctrl {
	int		cbm_len;
	int		min_cbm_bits;
};

struct mba_ctrl {
	u32			max_delay;
	u32			min_bw;
	u32			bw_gran;
	u32			delay_linear;
	u32			*mb_map;
};

and in then in struct rdt_resource:

       <common fields>
       union {
       		struct cache_ctrl	foo;
		struct mba_ctrl		bla;
	} ctrl;


That avoids that rdt_resource becomes a hodgepodge of unrelated or even
contradicting fields.

Hmm?

Thanks,

	tglx

  parent reply	other threads:[~2017-04-05 15:42 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-03 21:57 [PATCH 0/8 V3] x86/intel_rdt: Intel Memory bandwidth allocation Vikas Shivappa
2017-04-03 21:57 ` [PATCH 1/8] Documentation, x86: " Vikas Shivappa
2017-04-05 15:30   ` Thomas Gleixner
2017-04-05 18:04     ` Shivappa Vikas
2017-04-03 21:57 ` [PATCH 2/8] x86/intel_rdt/mba: Generalize the naming to get ready for MBA Vikas Shivappa
2017-04-03 21:57 ` [PATCH 3/8] x86/intel_rdt/mba: Memory b/w allocation feature detect Vikas Shivappa
2017-04-03 21:57 ` [PATCH 4/8] x86/intel_rct/mba: Add MBA structures and initialize MBA Vikas Shivappa
     [not found]   ` <CAChUvXM8gWAz6-AJ6jkyKjf5Yz0ze-2XAtvdZvze3Go44TPD8A@mail.gmail.com>
2017-04-04 18:50     ` Shivappa Vikas
2017-04-05 15:40   ` Thomas Gleixner [this message]
2017-04-05 18:09     ` Shivappa Vikas
2017-04-03 21:57 ` [PATCH 5/8] x86/intel_rdt: Prep to add info files for MBA Vikas Shivappa
2017-04-03 21:57 ` [PATCH 6/8] x86/intel_rdt/mba: Add info directory " Vikas Shivappa
2017-04-03 21:57 ` [PATCH 7/8] x86/intel_rdt: Prep to add schemata file " Vikas Shivappa
2017-04-03 21:57 ` [PATCH 8/8] x86/intel_rdt/mba: Add schemata file support " Vikas Shivappa
  -- strict thread matches above, loose matches on Subject: below --
2017-04-08  0:33 [PATCH 0/8 V4] x86/intel_rdt: Intel Memory bandwidth allocation Vikas Shivappa
2017-04-08  0:33 ` [PATCH 4/8] x86/intel_rct/mba: Add MBA structures and initialize MBA Vikas Shivappa
2017-04-04 19:02 Tracy Smith
2017-02-17 19:58 [PATCH 0/8 V2] x86/intel_rdt: Intel Memory bandwidth allocation Vikas Shivappa
2017-02-17 19:58 ` [PATCH 4/8] x86/intel_rct/mba: Add MBA structures and initialize MBA Vikas Shivappa
2017-03-01 15:24   ` Thomas Gleixner
2017-03-10 21:51     ` Shivappa Vikas

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=alpine.DEB.2.20.1704051731200.2803@nanos \
    --to=tglx@linutronix.de \
    --cc=fenghua.yu@intel.com \
    --cc=h.peter.anvin@intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=ravi.v.shankar@intel.com \
    --cc=tony.luck@intel.com \
    --cc=vikas.shivappa@intel.com \
    --cc=vikas.shivappa@linux.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.