Re: [PATCH 4/7] node: Add memory caching attributes
On Mon, Nov 19, 2018 at 09:44:00AM +0530, Anshuman Khandual wrote: > On 11/15/2018 04:19 AM, Keith Busch wrote: > > System memory may have side caches to help improve access speed. While > > the system provided cache is transparent to the software accessing > > these memory ranges, applications can optimize their own access based > > on cache attributes. > > Cache is not a separate memory attribute. It impacts how the real attributes > like bandwidth, latency e.g which are already captured in the previous patch. > What is the purpose of adding this as a separate attribute ? Can you explain > how this is going to help the user space apart from the hints it has already > received with bandwidth, latency etc properties. I am not sure I understand the question here. Access bandwidth and latency are entirely attributes different than what this patch provides. If the system side-caches memory, the associativity, line size, and total size can optionally be used by software to improve performance. > > In preparation for such systems, provide a new API for the kernel to > > register these memory side caches under the memory node that provides it. > > Under target memory node interface /sys/devices/system/node/nodeY/target* ? Yes. > > > > The kernel's sysfs representation is modeled from the cpu cacheinfo > > attributes, as seen from /sys/devices/system/cpu/cpuX/cache/. Unlike CPU > > cacheinfo, though, a higher node's memory cache level is nearer to the > > CPU, while lower levels are closer to the backing memory. Also unlike > > CPU cache, the system handles flushing any dirty cached memory to the > > last level the memory on a power failure if the range is persistent. > > Lets assume that a CPU has got four levels of caches L1, L2, L3, L4 before > reaching memory. L4 is the backing cache for the memory I don't quite understand this question either. The cache doesn't back the memory; the system side caches access to memory. > and L1-L3 is from > CPU till the system bus. Hence some of them will be represented as CPU > caches and some of them will be represented as memory caches ? > > /sys/devices/system/cpu/cpuX/cache/ --> L1, L2, L3 > /sys/devices/system/node/nodeY/target --> L4 > > L4 will be listed even if the node is memory only ? The system provided memory side caches are independent of the CPU. I'm just providing the CPU caches as a more familiar example to compare/contrast system memory cache attributes.
Re: [PATCH 4/7] node: Add memory caching attributes
On 11/20/2018 04:36 AM, Keith Busch wrote: > On Mon, Nov 19, 2018 at 09:44:00AM +0530, Anshuman Khandual wrote: >> On 11/15/2018 04:19 AM, Keith Busch wrote: >>> System memory may have side caches to help improve access speed. While >>> the system provided cache is transparent to the software accessing >>> these memory ranges, applications can optimize their own access based >>> on cache attributes. >> >> Cache is not a separate memory attribute. It impacts how the real attributes >> like bandwidth, latency e.g which are already captured in the previous patch. >> What is the purpose of adding this as a separate attribute ? Can you explain >> how this is going to help the user space apart from the hints it has already >> received with bandwidth, latency etc properties. > > I am not sure I understand the question here. Access bandwidth and latency > are entirely attributes different than what this patch provides. If the > system side-caches memory, the associativity, line size, and total size > can optionally be used by software to improve performance. Okay but then does this belong to this series which about memory attributes ?
Re: [PATCH 4/7] node: Add memory caching attributes
On Thu, Nov 22, 2018 at 06:59:21PM +0530, Anshuman Khandual wrote: > > > On 11/20/2018 04:36 AM, Keith Busch wrote: > > On Mon, Nov 19, 2018 at 09:44:00AM +0530, Anshuman Khandual wrote: > >> On 11/15/2018 04:19 AM, Keith Busch wrote: > >>> System memory may have side caches to help improve access speed. While > >>> the system provided cache is transparent to the software accessing > >>> these memory ranges, applications can optimize their own access based > >>> on cache attributes. > >> > >> Cache is not a separate memory attribute. It impacts how the real > >> attributes > >> like bandwidth, latency e.g which are already captured in the previous > >> patch. > >> What is the purpose of adding this as a separate attribute ? Can you > >> explain > >> how this is going to help the user space apart from the hints it has > >> already > >> received with bandwidth, latency etc properties. > > > > I am not sure I understand the question here. Access bandwidth and latency > > are entirely attributes different than what this patch provides. If the > > system side-caches memory, the associativity, line size, and total size > > can optionally be used by software to improve performance. > > Okay but then does this belong to this series which about memory attributes ? This patch series is about exporting memory attributes, and this system memory caching is one such attribute, so yes, I think it belongs.
Re: [PATCH 4/7] node: Add memory caching attributes
On Wed, Nov 14, 2018 at 03:49:17PM -0700, Keith Busch wrote:
> System memory may have side caches to help improve access speed. While
> the system provided cache is transparent to the software accessing
> these memory ranges, applications can optimize their own access based
> on cache attributes.
>
> In preparation for such systems, provide a new API for the kernel to
> register these memory side caches under the memory node that provides it.
>
> The kernel's sysfs representation is modeled from the cpu cacheinfo
> attributes, as seen from /sys/devices/system/cpu/cpuX/cache/. Unlike CPU
> cacheinfo, though, a higher node's memory cache level is nearer to the
> CPU, while lower levels are closer to the backing memory. Also unlike
> CPU cache, the system handles flushing any dirty cached memory to the
> last level the memory on a power failure if the range is persistent.
>
> The exported attributes are the cache size, the line size, associativity,
> and write back policy.
>
> Signed-off-by: Keith Busch
> ---
> drivers/base/node.c | 117
> +++
> include/linux/node.h | 23 ++
> 2 files changed, 140 insertions(+)
>
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 232535761998..bb94f1d18115 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -60,6 +60,12 @@ static DEVICE_ATTR(cpumap, S_IRUGO, node_read_cpumask,
> NULL);
> static DEVICE_ATTR(cpulist, S_IRUGO, node_read_cpulist, NULL);
>
> #ifdef CONFIG_HMEM
> +struct node_cache_obj {
> + struct kobject kobj;
> + struct list_head node;
> + struct node_cache_attrs cache_attrs;
> +};
I know you all are off in the weeds designing some new crazy api for
this instead of this current proposal (sorry, I lost the thread, I'll
wait for the patches before commenting on it), but I do want to say one
thing here.
NEVER use a raw kobject as a child for a 'struct device' unless you
REALLY REALLY REALLY REALLY know what you are doing and have a VERY good
reason to do so.
Just use a 'struct device', otherwise you end up having to reinvent all
of the core logic that struct device provides you, like attribute
callbacks (which you had to create), and other good stuff like telling
userspace that a device has shown up so it knows to look at it.
That last one is key, a kobject is suddenly a "black hole" in sysfs as
far as userspace knows because it does not see them for the most part
(unless you are mucking around in the filesystem on your own, and
really, don't do that, use a library like the rest of the world unless
you really like reinventing everything, which, from your patchset it
feels like...)
Anyway, use 'struct device'. That's all.
greg k-h
Re: [PATCH 4/7] node: Add memory caching attributes
On Wed, Nov 14, 2018 at 03:49:17PM -0700, Keith Busch wrote: > System memory may have side caches to help improve access speed. While > the system provided cache is transparent to the software accessing > these memory ranges, applications can optimize their own access based > on cache attributes. > > In preparation for such systems, provide a new API for the kernel to > register these memory side caches under the memory node that provides it. > > The kernel's sysfs representation is modeled from the cpu cacheinfo > attributes, as seen from /sys/devices/system/cpu/cpuX/cache/. Unlike CPU > cacheinfo, though, a higher node's memory cache level is nearer to the > CPU, while lower levels are closer to the backing memory. Also unlike > CPU cache, the system handles flushing any dirty cached memory to the > last level the memory on a power failure if the range is persistent. > > The exported attributes are the cache size, the line size, associativity, > and write back policy. You also didn't document your new sysfs attributes/layout in a Documentation/ABI/ entry which is required for any sysfs change... thanks, greg k-h
Re: [PATCH 4/7] node: Add memory caching attributes
On Mon, Nov 26, 2018 at 11:06:19AM -0800, Greg Kroah-Hartman wrote:
> On Wed, Nov 14, 2018 at 03:49:17PM -0700, Keith Busch wrote:
> > System memory may have side caches to help improve access speed. While
> > the system provided cache is transparent to the software accessing
> > these memory ranges, applications can optimize their own access based
> > on cache attributes.
> >
> > In preparation for such systems, provide a new API for the kernel to
> > register these memory side caches under the memory node that provides it.
> >
> > The kernel's sysfs representation is modeled from the cpu cacheinfo
> > attributes, as seen from /sys/devices/system/cpu/cpuX/cache/. Unlike CPU
> > cacheinfo, though, a higher node's memory cache level is nearer to the
> > CPU, while lower levels are closer to the backing memory. Also unlike
> > CPU cache, the system handles flushing any dirty cached memory to the
> > last level the memory on a power failure if the range is persistent.
> >
> > The exported attributes are the cache size, the line size, associativity,
> > and write back policy.
> >
> > Signed-off-by: Keith Busch
> > ---
> > drivers/base/node.c | 117
> > +++
> > include/linux/node.h | 23 ++
> > 2 files changed, 140 insertions(+)
> >
> > diff --git a/drivers/base/node.c b/drivers/base/node.c
> > index 232535761998..bb94f1d18115 100644
> > --- a/drivers/base/node.c
> > +++ b/drivers/base/node.c
> > @@ -60,6 +60,12 @@ static DEVICE_ATTR(cpumap, S_IRUGO, node_read_cpumask,
> > NULL);
> > static DEVICE_ATTR(cpulist, S_IRUGO, node_read_cpulist, NULL);
> >
> > #ifdef CONFIG_HMEM
> > +struct node_cache_obj {
> > + struct kobject kobj;
> > + struct list_head node;
> > + struct node_cache_attrs cache_attrs;
> > +};
>
> I know you all are off in the weeds designing some new crazy api for
> this instead of this current proposal (sorry, I lost the thread, I'll
> wait for the patches before commenting on it), but I do want to say one
> thing here.
>
> NEVER use a raw kobject as a child for a 'struct device' unless you
> REALLY REALLY REALLY REALLY know what you are doing and have a VERY good
> reason to do so.
>
> Just use a 'struct device', otherwise you end up having to reinvent all
> of the core logic that struct device provides you, like attribute
> callbacks (which you had to create), and other good stuff like telling
> userspace that a device has shown up so it knows to look at it.
>
> That last one is key, a kobject is suddenly a "black hole" in sysfs as
> far as userspace knows because it does not see them for the most part
> (unless you are mucking around in the filesystem on your own, and
> really, don't do that, use a library like the rest of the world unless
> you really like reinventing everything, which, from your patchset it
> feels like...)
>
> Anyway, use 'struct device'. That's all.
>
> greg k-h
Okay, thank you for the advice. I prefer to reuse over reinvent. :)
I only used kobject because the power/ directory was automatically
created with 'struct device', but I now I see there are better ways to
suppress that.
Re: [PATCH 4/7] node: Add memory caching attributes
On 11/14/18 2:49 PM, Keith Busch wrote: > System memory may have side caches to help improve access speed. While > the system provided cache is transparent to the software accessing > these memory ranges, applications can optimize their own access based > on cache attributes. > > In preparation for such systems, provide a new API for the kernel to > register these memory side caches under the memory node that provides it. > > The kernel's sysfs representation is modeled from the cpu cacheinfo > attributes, as seen from /sys/devices/system/cpu/cpuX/cache/. Unlike CPU > cacheinfo, though, a higher node's memory cache level is nearer to the > CPU, while lower levels are closer to the backing memory. Also unlike > CPU cache, the system handles flushing any dirty cached memory to the > last level the memory on a power failure if the range is persistent. > > The exported attributes are the cache size, the line size, associativity, > and write back policy. Could you also include an example of the layout?
Re: [PATCH 4/7] node: Add memory caching attributes
On 11/15/2018 04:19 AM, Keith Busch wrote: > System memory may have side caches to help improve access speed. While > the system provided cache is transparent to the software accessing > these memory ranges, applications can optimize their own access based > on cache attributes. Cache is not a separate memory attribute. It impacts how the real attributes like bandwidth, latency e.g which are already captured in the previous patch. What is the purpose of adding this as a separate attribute ? Can you explain how this is going to help the user space apart from the hints it has already received with bandwidth, latency etc properties. > > In preparation for such systems, provide a new API for the kernel to > register these memory side caches under the memory node that provides it. Under target memory node interface /sys/devices/system/node/nodeY/target* ? > > The kernel's sysfs representation is modeled from the cpu cacheinfo > attributes, as seen from /sys/devices/system/cpu/cpuX/cache/. Unlike CPU > cacheinfo, though, a higher node's memory cache level is nearer to the > CPU, while lower levels are closer to the backing memory. Also unlike > CPU cache, the system handles flushing any dirty cached memory to the > last level the memory on a power failure if the range is persistent. Lets assume that a CPU has got four levels of caches L1, L2, L3, L4 before reaching memory. L4 is the backing cache for the memory and L1-L3 is from CPU till the system bus. Hence some of them will be represented as CPU caches and some of them will be represented as memory caches ? /sys/devices/system/cpu/cpuX/cache/ --> L1, L2, L3 /sys/devices/system/node/nodeY/target --> L4 L4 will be listed even if the node is memory only ?
