Re: [PATCH 06/10] util: Introduce resctrl monitor for CMT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 08/27/2018 07:23 AM, Wang Huaqiang wrote:
> 'virResctrlAllocMon' denotes a resctrl monitor reporting the resource
> consumption information.
> 
> This patch introduced the interfaces for resctrl monitor.
> 
> Relationship of 'resctrl allocation' and 'resctrl monitor':
> 1. resctrl monitor monitors resources (cache or memory bandwidth) of
> particular allocation.
> 2. resctrl allocation may refer to the 'default' allocation if no
> dedicated resource 'control' applied to it. The 'default' allocation
> enjoys remaining resource that not allocated.
> 3. resctrl monitor belongs to 'default' allocation if no 'cachetune'
> specified in XML file.
> 4. one resctrl allocation may have several monitors. It is also
> permitted that there is no resctrl monitor associated with an
> allocation.
> 
> Key data structures:
> 
> +    struct _virResctrlAllocMon {
> +        char *id;
> +        char *path;
> +    };
> 
>     struct _virResctrlAlloc {
>       virObject parent;
> 
>       @@ -276,6 +289,12 @@ struct _virResctrlAlloc {
>                    virResctrlAllocMemBWPtr mem_bw;
> +                   virResctrlAllocMonPtr *monitors;
> +                   size_t nmonitors;
>       }
> 
> Signed-off-by: Wang Huaqiang <huaqiang.wang@xxxxxxxxx>
> ---
>  src/libvirt_private.syms |   6 +
>  src/util/virresctrl.c    | 361 ++++++++++++++++++++++++++++++++++++++++++++++-
>  src/util/virresctrl.h    |  31 ++++
>  3 files changed, 394 insertions(+), 4 deletions(-)
> 

Similar to the previous patch - there's a bit too much going on for just
one patch here.

Introducing "default_group" and "monitors".  This needs some separation.

I am not going to look at this patch.  I think you really need to
describe this default_group concept quite a bit more as you'd be totally
changing the meaning of alloc->path by "removing" everything after the
SYSFS_RESCTRL_PATH. What's the purpose of alloc->path then in this
environment?

John

> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 47ea35f..1439327 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2645,12 +2645,17 @@ virCacheKernelTypeFromString;
>  virCacheKernelTypeToString;
>  virCacheTypeFromString;
>  virCacheTypeToString;
> +virResctrlAllocAddMonitorPID;
>  virResctrlAllocAddPID;
>  virResctrlAllocCreate;
> +virResctrlAllocCreateMonitor;
> +virResctrlAllocDeleteMonitor;
> +virResctrlAllocDetermineMonitorPath;
>  virResctrlAllocDeterminePath;
>  virResctrlAllocForeachCache;
>  virResctrlAllocForeachMemory;
>  virResctrlAllocFormat;
> +virResctrlAllocGetCacheOccupancy;
>  virResctrlAllocGetID;
>  virResctrlAllocGetUnused;
>  virResctrlAllocNew;
> @@ -2658,6 +2663,7 @@ virResctrlAllocRemove;
>  virResctrlAllocSetCacheSize;
>  virResctrlAllocSetID;
>  virResctrlAllocSetMemoryBandwidth;
> +virResctrlAllocSetMonitor;
>  virResctrlInfoGetCache;
>  virResctrlInfoNew;
>  
> diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
> index b3bae6e..7215a47 100644
> --- a/src/util/virresctrl.c
> +++ b/src/util/virresctrl.c
> @@ -257,6 +257,19 @@ struct _virResctrlAllocMemBW {
>      size_t nbandwidths;
>  };
>  
> +
> +typedef struct _virResctrlAllocMon virResctrlAllocMon;
> +typedef virResctrlAllocMon *virResctrlAllocMonPtr;
> +/* virResctrlAllocMon denotes a resctrl monitoring group reporting the resource
> + * consumption information for resource of either cache or memory
> + * bandwidth. */
> +struct _virResctrlAllocMon {
> +    /* monitoring group identifier, should be unique in scope of allocation */
> +    char *id;
> +    /* directory path under /sys/fs/resctrl*/
> +    char *path;
> +};
> +
>  struct _virResctrlAlloc {
>      virObject parent;
>  
> @@ -265,11 +278,21 @@ struct _virResctrlAlloc {
>  
>      virResctrlAllocMemBWPtr mem_bw;
>  
> +    /* monintoring groups associated with current resource allocation
> +     * it might report resource consumption information at a finer
> +     * granularity */
> +    virResctrlAllocMonPtr *monitors;
> +    size_t nmonitors;
> +
>      /* The identifier (any unique string for now) */
>      char *id;
>      /* libvirt-generated path in /sys/fs/resctrl for this particular
>       * allocation */
>      char *path;
> +    /* is this a default resctrl group?
> +     * true : default group, directory path equals '/sys/fs/resctrl'
> +     * false: non-default group */
> +    bool default_group;
>  };
>  
>  
> @@ -315,6 +338,13 @@ virResctrlAllocDispose(void *obj)
>          VIR_FREE(alloc->mem_bw);
>      }
>  
> +    for (i = 0; i < alloc->nmonitors; i++) {
> +        virResctrlAllocMonPtr monitor = alloc->monitors[i];
> +        VIR_FREE(monitor->id);
> +        VIR_FREE(monitor->path);
> +        VIR_FREE(monitor);
> +    }
> +    VIR_FREE(alloc->monitors);
>      VIR_FREE(alloc->id);
>      VIR_FREE(alloc->path);
>      VIR_FREE(alloc->levels);
> @@ -805,6 +835,7 @@ virResctrlInfoGetCache(virResctrlInfoPtr resctrl,
>          memcpy((*controls)[*ncontrols - 1], &i_type->control, sizeof(i_type->control));
>      }
>  
> +    cachemon->nfeatures = 0;
>      cachemon->max_allocation = 0;
>  
>      if (resctrl->monitor_info) {
> @@ -817,7 +848,7 @@ virResctrlInfoGetCache(virResctrlInfoPtr resctrl,
>              if (STREQLEN(info->features[i], "llc_", strlen("llc_"))) {
>                  if (virStringListAdd(&cachemon->features,
>                                       info->features[i]) < 0)
> -                     goto error;
> +                    goto error;
>                  cachemon->nfeatures++;
>              }
>          }
> @@ -841,10 +872,19 @@ virResctrlInfoGetCache(virResctrlInfoPtr resctrl,
>  virResctrlAllocPtr
>  virResctrlAllocNew(void)
>  {
> +    virResctrlAllocPtr ret = NULL;
> +
>      if (virResctrlInitialize() < 0)
>          return NULL;
>  
> -    return virObjectNew(virResctrlAllocClass);
> +    ret = virObjectNew(virResctrlAllocClass);
> +    if (!ret)
> +        return NULL;
> +
> +    /* By default, a resource group is a default group */
> +    ret->default_group = true;
> +
> +    return ret;
>  }
>  
>  
> @@ -861,6 +901,9 @@ virResctrlAllocIsEmpty(virResctrlAllocPtr alloc)
>      if (alloc->mem_bw)
>          return false;
>  
> +    if (alloc->nmonitors)
> +        return false;
> +
>      for (i = 0; i < alloc->nlevels; i++) {
>          virResctrlAllocPerLevelPtr a_level = alloc->levels[i];
>  
> @@ -2124,10 +2167,18 @@ int
>  virResctrlAllocDeterminePath(virResctrlAllocPtr alloc,
>                               const char *machinename)
>  {
> -    return virResctrlDeterminePath(alloc->id, NULL, NULL,
> -                                   machinename, &alloc->path);
> +    if (alloc->default_group) {
> +        if (VIR_STRDUP(alloc->path, SYSFS_RESCTRL_PATH) < 0)
> +            return -1;
> +        return 0;
> +    } else {
> +
> +        return virResctrlDeterminePath(alloc->id, NULL, NULL,
> +                                       machinename, &alloc->path);
> +    }
>  }
>  
> +
>  static int
>  virResctrlCreateGroup(virResctrlInfoPtr resctrl,
>                        char *path)
> @@ -2169,6 +2220,27 @@ virResctrlCreateGroup(virResctrlInfoPtr resctrl,
>      return ret;
>  }
>  
> + /* In case of no explicit requirement for allocating cache and memory
> +  * bandwidth, set 'alloc->default' to 'true', then the monitoring
> +  * group will be created under '/sys/fs/resctrl/mon_groups' in later
> +  * invocation of virResctrlAllocCreate.
> +  * Otherwise, set 'alloc->default' to false, create a new directory
> +  * under '/sys/fs/resctrl/'. This is will cost a hardware 'COSID'.*/
> +static int
> +virResctrlAllocCheckDefault(virResctrlAllocPtr alloc)
> +{
> +    bool default_group = true;
> +    if (!alloc)
> +        return -1;
> +
> +    if (alloc->nlevels)
> +        default_group = false;
> +    if (alloc->mem_bw && alloc->mem_bw->nbandwidths)
> +        default_group = false;
> +
> +    alloc->default_group = default_group;
> +    return 0;
> +}
>  
>  /* This checks if the directory for the alloc exists.  If not it tries to create
>   * it and apply appropriate alloc settings. */
> @@ -2185,6 +2257,8 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl,
>      if (!alloc)
>          return 0;
>  
> +    virResctrlAllocCheckDefault(alloc);
> +
>      if (virResctrlAllocDeterminePath(alloc, machinename) < 0)
>          return -1;
>  
> @@ -2275,10 +2349,289 @@ virResctrlAllocRemove(virResctrlAllocPtr alloc)
>          return 0;
>  
>      VIR_DEBUG("Removing resctrl allocation %s", alloc->path);
> +
> +    while (alloc->nmonitors > 0) {
> +        ret = virResctrlAllocDeleteMonitor(alloc, alloc->monitors[0]->id);
> +        if (ret < 0)
> +            goto cleanup;
> +    }
> +
>      if (rmdir(alloc->path) != 0 && errno != ENOENT) {
>          ret = -errno;
>          VIR_ERROR(_("Unable to remove %s (%d)"), alloc->path, errno);
>      }
>  
> +    ret = 0;
> + cleanup:
> +    return ret;
> +}
> +
> +
> +static int
> +virResctrlAllocGetMonitor(virResctrlAllocPtr alloc,
> +                          const char *id,
> +                          virResctrlAllocMonPtr *monitor,
> +                          size_t *pos)
> +{
> +    size_t i = 0;
> +
> +    if (!alloc || !id)
> +        return -1;
> +
> +    for (i = 0; i < alloc->nmonitors; i++) {
> +        if (alloc->monitors[i]->id &&
> +            STREQ(id, (alloc->monitors[i])->id)) {
> +            if (monitor)
> +                *monitor = alloc->monitors[i];
> +            if (pos)
> +                *pos = i;
> +            return 0;
> +        }
> +    }
> +
> +    return -1;
> +}
> +
> +
> +int
> +virResctrlAllocDetermineMonitorPath(virResctrlAllocPtr alloc,
> +                                    const char *id,
> +                                    const char *machinename)
> +{
> +    virResctrlAllocMonPtr monitor = NULL;
> +
> +    if (virResctrlAllocGetMonitor(alloc, id, &monitor, NULL) < 0)
> +        return -1;
> +
> +
> +    return virResctrlDeterminePath(monitor->id,
> +                                   alloc->path,
> +                                   "mon_groups",
> +                                   machinename,
> +                                   &monitor->path);
> +}
> +
> +
> +int
> +virResctrlAllocAddMonitorPID(virResctrlAllocPtr alloc,
> +                             const char *id,
> +                             pid_t pid)
> +{
> +    virResctrlAllocMonPtr monitor = NULL;
> +
> +    if (virResctrlAllocGetMonitor(alloc, id, &monitor, NULL) < 0)
> +        return -1;
> +
> +    return virResctrlAddPID(monitor->path, pid);
> +}
> +
> +
> +int
> +virResctrlAllocSetMonitor(virResctrlAllocPtr alloc,
> +                          const char *id)
> +{
> +    virResctrlAllocMonPtr monitor = NULL;
> +
> +    if (!alloc || !id)
> +        return - 1;
> +
> +    if (virResctrlAllocGetMonitor(alloc, id, &monitor, NULL) < 0) {
> +        if (VIR_ALLOC(monitor) < 0)
> +            return -1;
> +    }
> +
> +    if (VIR_STRDUP(monitor->id, (char*)id) < 0)
> +        return -1;
> +
> +    if (VIR_APPEND_ELEMENT(alloc->monitors, alloc->nmonitors, monitor) < 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +int
> +virResctrlAllocCreateMonitor(virResctrlInfoPtr resctrl,
> +                             virResctrlAllocPtr alloc,
> +                             const char *machinename,
> +                             const char *id)
> +{
> +    virResctrlAllocMonPtr monitor = NULL;
> +
> +    if (virResctrlAllocGetMonitor(alloc, id, &monitor, NULL) < 0)
> +        return - 1;
> +
> +    if (virResctrlAllocDetermineMonitorPath(alloc, id, machinename) < 0)
> +        return -1;
> +
> +    VIR_DEBUG("Creating resctrl monitor %s", monitor->path);
> +    if (virResctrlCreateGroup(resctrl, monitor->path) < 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +
> +int
> +virResctrlAllocDeleteMonitor(virResctrlAllocPtr alloc,
> +                             const char *id)
> +{
> +    int ret = 0;
> +
> +    virResctrlAllocMonPtr monitor = NULL;
> +    size_t pos = 0;
> +
> +    if (virResctrlAllocGetMonitor(alloc, id, &monitor, &pos) < 0)
> +        return -1;
> +
> +    VIR_DELETE_ELEMENT(alloc->monitors, pos, alloc->nmonitors);
> +
> +    VIR_DEBUG("Deleting resctrl monitor %s  ", monitor->path);
> +    if (rmdir(monitor->path) != 0 && errno != ENOENT) {
> +        ret = -errno;
> +        VIR_ERROR(_("Unable to remove %s (%d)"), monitor->path, errno);
> +    }
> +
> +    VIR_FREE(monitor->id);
> +    VIR_FREE(monitor->path);
> +    VIR_FREE(monitor);
> +    return ret;
> +}
> +
> +
> +static int
> +virResctrlAllocGetStatistic(virResctrlAllocPtr alloc,
> +                            const char *id,
> +                            const char *resfile,
> +                            unsigned int *nnodes,
> +                            unsigned int **nodeids,
> +                            unsigned int **nodevals)
> +{
> +    DIR *dirp = NULL;
> +    int ret = -1;
> +    int rv = -1;
> +    struct dirent *ent = NULL;
> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    char *mondatapath = NULL;
> +    size_t ntmpid = 0;
> +    size_t ntmpval = 0;
> +    virResctrlAllocMonPtr monitor = NULL;
> +
> +    if (!nnodes || !nodeids || !nodevals)
> +        return -1;
> +
> +    if (virResctrlAllocGetMonitor(alloc, id, &monitor, NULL) < 0)
> +        goto cleanup;
> +
> +    if (!monitor || !monitor->path)
> +        goto cleanup;
> +
> +    rv = virDirOpenIfExists(&dirp, monitor->path);
> +    if (rv <= 0)
> +        goto cleanup;
> +
> +    *nnodes = 0;
> +
> +    virBufferAsprintf(&buf, "%s/mon_data", monitor->path);
> +
> +    mondatapath = virBufferContentAndReset(&buf);
> +    if (!mondatapath)
> +        goto cleanup;
> +
> +    if (virDirOpen(&dirp, mondatapath) < 0)
> +        goto cleanup;
> +
> +    while ((rv = virDirRead(dirp, &ent, mondatapath)) > 0) {
> +        char *pstrid = NULL;
> +        size_t i = 0;
> +        unsigned int len = 0;
> +        unsigned int counter = 0;
> +        unsigned int cacheid = 0;
> +        unsigned int cur_cacheid = 0;
> +        unsigned int val = 0;
> +        int tmpnodeid = 0;
> +        int tmpnodeval = 0;
> +
> +        if (ent->d_type != DT_DIR)
> +            continue;
> +
> +        /* mon_L3(|CODE|DATA)_xx, xx is cache id */
> +        if (STRNEQLEN(ent->d_name, "mon_L", 5))
> +            continue;
> +
> +        len = strlen(ent->d_name);
> +        pstrid = ent->d_name;
> +        /* locating the cache id string: 'xx' */
> +        for (i = 0; i < len; i++) {
> +            if (*(pstrid + i) == '_')
> +                counter ++;
> +            if (counter == 2)
> +                break;
> +        }
> +        i++;
> +
> +        if (i >= len)
> +            goto cleanup;
> +
> +        if (virStrToLong_uip(pstrid + i, NULL, 0, &cacheid) < 0) {
> +            VIR_DEBUG("Cannot parse id from folder '%s'", ent->d_name);
> +            goto cleanup;
> +        }
> +
> +        rv = virFileReadValueUint(&val,
> +                                  "%s/%s/%s",
> +                                  mondatapath, ent->d_name, resfile);
> +
> +        if (rv == -2) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("file %s/%s/%s does not exist"),
> +                           mondatapath, ent->d_name, resfile);
> +            goto cleanup;
> +        } else {
> +            if (rv < 0)
> +                goto cleanup;
> +        }
> +
> +        /* The ultimate caller will be responiblefor free memory of
> +         * 'nodeids' an 'nodevals' */
> +        if (VIR_APPEND_ELEMENT(*nodeids, ntmpid, cacheid) < 0)
> +            goto cleanup;
> +        if (VIR_APPEND_ELEMENT(*nodevals, ntmpval, val) < 0)
> +            goto cleanup;
> +
> +        cur_cacheid = ntmpval - 1;
> +        /* sort the cache information in caach bank id's ascending order */
> +        for (i = 0; i < cur_cacheid; i++) {
> +            if ((*nodeids)[cur_cacheid] < (*nodeids)[i]) {
> +                tmpnodeid  = (*nodeids)[cur_cacheid];
> +                tmpnodeval = (*nodevals)[cur_cacheid];
> +                (*nodeids)[cur_cacheid]  = (*nodeids)[i];
> +                (*nodevals)[cur_cacheid] = (*nodevals)[i];
> +                (*nodeids)[i]  = tmpnodeid;
> +                (*nodevals)[i] = tmpnodeval;
> +            }
> +        }
> +    }
> +
> +    (*nnodes) = ntmpval;
> +    ret = 0;
> + cleanup:
> +    VIR_FREE(mondatapath);
> +    VIR_DIR_CLOSE(dirp);
> +    return ret;
> +}
> +
> +
> +int
> +virResctrlAllocGetCacheOccupancy(virResctrlAllocPtr alloc,
> +                                 const char *id,
> +                                 unsigned int *nbank,
> +                                 unsigned int **bankids,
> +                                 unsigned int **bankcaches)
> +{
> +    int ret = - 1;
> +
> +    ret = virResctrlAllocGetStatistic(alloc, id, "llc_occupancy",
> +                                      nbank, bankids, bankcaches);
> +
>      return ret;
>  }
> diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
> index 51bb68b..0f63997 100644
> --- a/src/util/virresctrl.h
> +++ b/src/util/virresctrl.h
> @@ -160,4 +160,35 @@ virResctrlAllocAddPID(virResctrlAllocPtr alloc,
>  int
>  virResctrlAllocRemove(virResctrlAllocPtr alloc);
>  
> +int
> +virResctrlAllocDetermineMonitorPath(virResctrlAllocPtr alloc,
> +                                    const char *id,
> +                                    const char *machinename);
> +
> +int
> +virResctrlAllocAddMonitorPID(virResctrlAllocPtr alloc,
> +                             const char *id,
> +                             pid_t pid);
> +
> +int
> +virResctrlAllocSetMonitor(virResctrlAllocPtr alloc,
> +                          const char *id);
> +
> +int
> +virResctrlAllocCreateMonitor(virResctrlInfoPtr resctrl,
> +                             virResctrlAllocPtr alloc,
> +                             const char *machinename,
> +                             const char *id);
> +
> +int
> +virResctrlAllocDeleteMonitor(virResctrlAllocPtr alloc,
> +                             const char *id);
> +
> +int
> +virResctrlAllocGetCacheOccupancy(virResctrlAllocPtr alloc,
> +                                 const char *id,
> +                                 unsigned int *nbank,
> +                                 unsigned int **bankids,
> +                                 unsigned int **bankcaches);
> +
>  #endif /*  __VIR_RESCTRL_H__ */
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux