[PATCH] numa_conf: Properly check for caches in virDomainNumaDefValidate()

Laine Stump laine at redhat.com
Tue Aug 18 18:57:05 UTC 2020


On 8/18/20 6:55 AM, Michal Privoznik wrote:
> When adding support for HMAT, in f0611fe8830 I've introduced a
> check which aims to validate /domain/cpu/numa/interconnects. As a
> part of that, there is a loop which checks whether all <latency/>
> with @cache attribute refer to an existing cache level. For
> instance:
>
>    <cpu mode='host-model' check='partial'>
>      <numa>
>        <cell id='0' cpus='0-5' memory='512000' unit='KiB' discard='yes'>
>          <cache level='1' associativity='direct' policy='writeback'>
>            <size value='8' unit='KiB'/>
>            <line value='5' unit='B'/>
>          </cache>
>        </cell>
>        <interconnects>
>          <latency initiator='0' target='0' cache='1' type='access' value='5'/>
>          <bandwidth initiator='0' target='0' type='access' value='204800' unit='KiB'/>
>        </interconnects>
>      </numa>
>    </cpu>
>
> This XML defines that accessing L1 cache of node #0 from node #0
> has latency of 5ns.
>
> However, the loop was not written properly. Well, the check in
> it, as it was always checking for the first cache in the target
> node and not the rest. Therefore, the following example errors
> out:
>
>    <cpu mode='host-model' check='partial'>
>      <numa>
>        <cell id='0' cpus='0-5' memory='512000' unit='KiB' discard='yes'>
>          <cache level='3' associativity='direct' policy='writeback'>
>            <size value='10' unit='KiB'/>
>            <line value='8' unit='B'/>
>          </cache>
>          <cache level='1' associativity='direct' policy='writeback'>
>            <size value='8' unit='KiB'/>
>            <line value='5' unit='B'/>
>          </cache>
>        </cell>
>        <interconnects>
>          <latency initiator='0' target='0' cache='1' type='access' value='5'/>
>          <bandwidth initiator='0' target='0' type='access' value='204800' unit='KiB'/>
>        </interconnects>
>      </numa>
>    </cpu>
>
> This errors out even though it is a valid configuration. The L1
> cache under node #0 is still present.
>
> Fixes: f0611fe8830
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>
> Is this trivial enough to be pushed as such? ;-)


It looks pretty trivial to me, but just in case you want it:


Reviewed-by: Laine Stump <laine at redhat.com>


>
>   src/conf/numa_conf.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
> index 50d57ba8f6..9305e125b7 100644
> --- a/src/conf/numa_conf.c
> +++ b/src/conf/numa_conf.c
> @@ -1421,7 +1421,7 @@ virDomainNumaDefValidate(const virDomainNuma *def)
>   
>           if (l->cache > 0) {
>               for (j = 0; j < def->mem_nodes[l->target].ncaches; j++) {
> -                const virDomainNumaCache *cache = def->mem_nodes[l->target].caches;
> +                const virDomainNumaCache *cache = &def->mem_nodes[l->target].caches[j];
>   
>                   if (l->cache == cache->level)
>                       break;





More information about the libvir-list mailing list