[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v12 06/11] numa: Extend CLI to provide memory latency and ban
From: |
Igor Mammedov |
Subject: |
Re: [PATCH v12 06/11] numa: Extend CLI to provide memory latency and bandwidth information |
Date: |
Fri, 11 Oct 2019 15:56:47 +0200 |
On Wed, 9 Oct 2019 14:39:46 +0800
Tao Xu <address@hidden> wrote:
> On 10/2/2019 11:16 PM, Igor Mammedov wrote:
> > On Fri, 20 Sep 2019 15:43:44 +0800
> > Tao Xu <address@hidden> wrote:
> >
> [...]
> >> +struct HMAT_LB_Info {
> >> + /* Indicates it's memory or the specified level memory side cache. */
> >> + uint8_t hierarchy;
> >> +
> >> + /* Present the type of data, access/read/write latency or bandwidth.
> >> */
> >> + uint8_t data_type;
> >> +
> >> + /* Array to store the latencies */
> > specify units it's stored in
> >
> >> + uint64_t *latency;
> >> +
> >> + /* Array to store the bandwidthes */
> > ditto
> >
> >> + uint64_t *bandwidth;
> > btw:
> >
> > what was the reason for picking uint64_t for storing above values?
> >
> > it seems in this patch you dumb down bandwidth to MB/s above but
> > store latency as is.
>
> Because I want to store the bandwidth or latency value (minimum unit)
> that user input. In HMAT, the minimum unit of bandwidth is MB/s, but in
> QAPI, the minimum unit of size is Byte. So I convert size into MB/s and
> time unit is "ps", need not convert.
Just be consistent and store (user input) raw values for both fields
(i.e. B/s PS/s) and post-process them later to uint16_t.
> > and then in 9/11 build_hmat_lb you divide that on 'base' units,
> > where are guaranties that value stored here will fit into 2 bytes
> > used in HMAT to store it in the table?
> >
> For HMAT spec, for a matrix of bandwidth or latency, there is only one
> base (in order to save ACPI tables space). We need to extract base for a
> matrix, but user input bandwidth or latency line by line. So after all
> data input, we can extract the base (as in 9/11).
>
> There is another benefit. If user input different but similar units,
> such as "10ns" and "100ps", we can also store them. Only If user input
> big gap units, such as "1ps" and "1000ms". we can't store them and raise
> error.
No disagreement here,
but I suggest to move verification and base calculation from 09/11
into a separate patch (right after this one) and doing it at
numa_complete_configuration() time.
To store calculated base you can add a common_base field to
sub-table structure (HMAT_LB_Info) and use it when building ACPI
table without extra calculations.
> > if this structure should store values in terms on HMAT table it should
> > probably use uint16_t and check that user provided value won't overflow
> > at the time of CLI parsing.
> >
>