Re: [systemd-devel] [PATCH] systemd: add RDTCacheReservation= option to support CAT (Cache Allocation Technology)
On Fri, Jan 06, 2017 at 01:51:17PM -0200, Marcelo Tosatti wrote: > On Fri, Jan 06, 2017 at 05:26:36PM +0200, Mantas Mikulėnas wrote: > > On Fri, Jan 6, 2017 at 3:59 PM, Marcelo Tosatti wrote: > > > > > > > > > > > Cache Allocation Technology is a feature on selected recent Intel Xeon > > > processors which allows control over L3 cache allocation. > > > > > > Kernel support has been merged to the upstream kernel, via a filesystem > > > resctrlfs. > > > > > > On top of that, a userspace utility, resctrltool has been written > > > to facilitate writing applications and using the filesystem > > > interface (see the rationale at > > > http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1300792.html). > > > > > > This patch adds a new option to systemd, RDTCacheReservation, > > > to allow configuration of CAT via resctrltool. > > > > > > See the first hunk of the patch for a description of the option > > > > > > This really doesn't look pretty, neither the approach nor the > > implementation... > > Suggestions to improve the code or the approach are welcome. > > > Is the option actually so complex that calling resctrltool is the only way > > to adjust it? What about writing to the resctrlfs directly? > > You'll have to deal with the issues that resctrltool deals with, > namely: > > 1) Filesystem locking. > 2) Reading in every directory and the default > directory. > 3) Converting the reservation request to proper sizes. > 4) Converting: > type=both --> type=data/type=code > > type=data/type=code --> type=both > > 4) Finding free space for the reservation. > 5) Adjusting the default group reservation. > > Since this steps must be performed by every user of > CAT (including libvirt which plans to execute resctrltool > as well), it was decided its better to maintain this logic > in a centralized place. Errr, no, that's not correct - Libvirt is certainly not going to spawn some python program to do this. If there's no C library API for this, libvirt will simply implement all the logic itself. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd: add RDTCacheReservation= option to support CAT (Cache Allocation Technology)
On Fri, Jan 06, 2017 at 07:51:05PM +0100, Lennart Poettering wrote: > On Fri, 06.01.17 11:59, Marcelo Tosatti (mtosa...@redhat.com) wrote: > > > Cache Allocation Technology is a feature on selected recent Intel Xeon > > processors which allows control over L3 cache allocation. > > What precisely is the benefit of making this configurable? Can you > describe a basic usecase why I'd want to set this on some service? http://www.intel.com/content/www/us/en/communications/cache-allocation-technology-white-paper.html "On systems with multiple workloads running simultaneously, which heavily utilize the cache; system administrators can utilize CAT to improve performance. CAT can be used to better utilize the caches and as a result some applications are more likely to hit the cache which results in fewer accesses to system memory. This results in lower latency and greater performance." For example, with a job with large memory reaccess distances (where caching hardly brings any benefit), you can limit that job to reclaim a small portion of L3 cache (so you don't throw away data from other jobs from L3 cache). > > > Index: systemd/man/systemd.exec.xml > > === > > --- systemd.orig/man/systemd.exec.xml > > +++ systemd/man/systemd.exec.xml > > Hmm, this doesn't look like a git patch? We generally prefer > submissions as github PRs these days, btw. > > > @@ -233,6 +233,31 @@ > > > > > > > > +RDTCacheReservation= > > + > > +Creates a L3 CAT reservation in resctrlfs > > +for the executed processes. Takes a ";" separated list of > > +tuples (optionally triples) containing type,size and > > +optionally cacheid: > > if I read this, then I am puzzled what "RDT Cache" is, what "L3 CAT" > is. Can you explain? RDT: Resource Director Technology(RDT) CAT: Cache Allocation Technology (CAT), a subcomponent of RDT (which includes other things such as CMT: Cache Monitoring Technology). > > > +type=type,size=size,cacheid=id; > > +type=type,size=size,cacheid=id;... > > + > > +Where: > > + > > +type is one of: both, data, code. > > +size is size in kbytes. > > +cacheid (optional) is the cacheid for > > +the reservation. > > what is a "cacheid"? Cache IDs - On current generation systems there is one L3 cache per socket and L2 caches are generally just shared by the hyperthreads on a core, but this isn't an architectural requirement. We could have multiple separate L3 caches on a socket, multiple cores could share an L2 cache. So instead of using "socket" or "core" to define the set of logical cpus sharing a resource we use a "Cache ID". At a given cache level this will be a unique number across the whole system (but it isn't guaranteed to be a contiguous sequence, there may be gaps). To find the ID for each logical CPU look in /sys/devices/system/cpu/cpu*/cache/index*/id > > > + > > +Rules for the parameters: > > +* type=code must be paired with type=data entry. > > + > > +See the output of resctrltool for more details. > > + > > + > > + > > This syntax doesn't appear to be in line with how we'd do this in > systemd usually. Ideally, we try to hide the fact that our settings > syntax is one-dimensional as much as we can. Hence, a syntax like this > sounds more appropriate to me: > >RDTCacheReservation=4K code >RDTCacheReservation=16K data > > and so on... What you suggest is to essentially to split RDTCacheReservation=type=type,size=size,cacheid=id;type=type,size=size,cacheid=id Into RDTCacheReservation=type=type,size=size,cacheid=id RDTCacheReservation=type=type,size=size,cacheid=id (yes sure can get rid of the commas as well, looks better). > But then again, I have no idea what all of this actually really is, > hence I am not sure my proposed syntax makes much sense. > > sizes are generally parsed with parse_bytes() in systemd, so that K, > M, G suffixes are properly recognized... > > > +ExecContext *c = userdata; > > +int r; > > +int i; > > + > > +assert(bus); > > +assert(reply); > > +assert(c); > > + > > +r = sd_bus_message_open_container(reply, 'a', "s"); > > +if (r < 0) > > +return r; > > + > > +for (i = 0; i < c->argmidx; i++) { > > +r = sd_bus_message_append(reply, "s", c->argm[i]); > > +if (r < 0) > > +return r; > > +} > > indentation is borked. Please indent to multiples of 8ch. > > > +static char* convert_rdt_back(char *argm[], int argmidx) > > +{ > > Please follow coding style, place opening bracket on same name as > funciton name. > > > + int i, ret; > > + char *buf, *str; > > + int size = 0; > > + int printcomma = 0; > > + int printsem
Re: [systemd-devel] [PATCH] systemd: add RDTCacheReservation= option to support CAT (Cache Allocation Technology)
GOn Fri, Jan 06, 2017 at 07:54:48PM +0100, Lennart Poettering wrote: > On Fri, 06.01.17 13:51, Marcelo Tosatti (mtosa...@redhat.com) wrote: > > > > This really doesn't look pretty, neither the approach nor the > > > implementation... > > > > Suggestions to improve the code or the approach are welcome. > > > > > Is the option actually so complex that calling resctrltool is the only way > > > to adjust it? What about writing to the resctrlfs directly? > > > > You'll have to deal with the issues that resctrltool deals with, > > namely: > > > > 1) Filesystem locking. > > 2) Reading in every directory and the default > > directory. > > 3) Converting the reservation request to proper sizes. > > 4) Converting: > > type=both --> type=data/type=code > > > > type=data/type=code --> type=both > > > > 4) Finding free space for the reservation. > > 5) Adjusting the default group reservation. > > > > Since this steps must be performed by every user of > > CAT (including libvirt which plans to execute resctrltool > > as well), it was decided its better to maintain this logic > > in a centralized place. > > Centralized place is fine, but please do this via a proper C library, > not some tool to shell out... > > Hmm, if I see that correctly, then resctrltool is a python script? I > am particularly concerned about making systemd indirectly depend on > Python this way, in particular for something that isn't precisely a > core functionality of systemd. Yes its a Python script. Sure, will write a C library for resctrltool-0.2 and repost the patches. Thanks. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd: add RDTCacheReservation= option to support CAT (Cache Allocation Technology)
On Fri, 06.01.17 13:51, Marcelo Tosatti (mtosa...@redhat.com) wrote: > > This really doesn't look pretty, neither the approach nor the > > implementation... > > Suggestions to improve the code or the approach are welcome. > > > Is the option actually so complex that calling resctrltool is the only way > > to adjust it? What about writing to the resctrlfs directly? > > You'll have to deal with the issues that resctrltool deals with, > namely: > > 1) Filesystem locking. > 2) Reading in every directory and the default > directory. > 3) Converting the reservation request to proper sizes. > 4) Converting: > type=both --> type=data/type=code > > type=data/type=code --> type=both > > 4) Finding free space for the reservation. > 5) Adjusting the default group reservation. > > Since this steps must be performed by every user of > CAT (including libvirt which plans to execute resctrltool > as well), it was decided its better to maintain this logic > in a centralized place. Centralized place is fine, but please do this via a proper C library, not some tool to shell out... Hmm, if I see that correctly, then resctrltool is a python script? I am particularly concerned about making systemd indirectly depend on Python this way, in particular for something that isn't precisely a core functionality of systemd. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd: add RDTCacheReservation= option to support CAT (Cache Allocation Technology)
On Fri, 06.01.17 11:59, Marcelo Tosatti (mtosa...@redhat.com) wrote: > Cache Allocation Technology is a feature on selected recent Intel Xeon > processors which allows control over L3 cache allocation. What precisely is the benefit of making this configurable? Can you describe a basic usecase why I'd want to set this on some service? > Index: systemd/man/systemd.exec.xml > === > --- systemd.orig/man/systemd.exec.xml > +++ systemd/man/systemd.exec.xml Hmm, this doesn't look like a git patch? We generally prefer submissions as github PRs these days, btw. > @@ -233,6 +233,31 @@ > > > > +RDTCacheReservation= > + > +Creates a L3 CAT reservation in resctrlfs > +for the executed processes. Takes a ";" separated list of > +tuples (optionally triples) containing type,size and > +optionally cacheid: if I read this, then I am puzzled what "RDT Cache" is, what "L3 CAT" is. Can you explain? > +type=type,size=size,cacheid=id; > +type=type,size=size,cacheid=id;... > + > +Where: > + > +type is one of: both, data, code. > +size is size in kbytes. > +cacheid (optional) is the cacheid for > +the reservation. what is a "cacheid"? > + > +Rules for the parameters: > +* type=code must be paired with type=data entry. > + > +See the output of resctrltool for more details. > + > + > + This syntax doesn't appear to be in line with how we'd do this in systemd usually. Ideally, we try to hide the fact that our settings syntax is one-dimensional as much as we can. Hence, a syntax like this sounds more appropriate to me: RDTCacheReservation=4K code RDTCacheReservation=16K data and so on... But then again, I have no idea what all of this actually really is, hence I am not sure my proposed syntax makes much sense. sizes are generally parsed with parse_bytes() in systemd, so that K, M, G suffixes are properly recognized... > +ExecContext *c = userdata; > +int r; > +int i; > + > +assert(bus); > +assert(reply); > +assert(c); > + > +r = sd_bus_message_open_container(reply, 'a', "s"); > +if (r < 0) > +return r; > + > +for (i = 0; i < c->argmidx; i++) { > +r = sd_bus_message_append(reply, "s", c->argm[i]); > +if (r < 0) > +return r; > +} indentation is borked. Please indent to multiples of 8ch. > +static char* convert_rdt_back(char *argm[], int argmidx) > +{ Please follow coding style, place opening bracket on same name as funciton name. > + int i, ret; > + char *buf, *str; > + int size = 0; > + int printcomma = 0; > + int printsemicolon = 0; > + int localmode = 0; Please use C99 "bool" when you actually mean a boolean. > + > + for (i=0; i < argmidx; i++) { > + /* ',', ';', '=' */ > + size += strlen(argm[i]) + 3; > + } > + > + buf = malloc(size); > + if (buf == NULL) > + return NULL; > + memset(buf, 0, size); Use "buf = new0(char, size)" for allocations of this kind. > + > + str = buf; > + > + /* cache-id specified */ > + for (i=0; i < argmidx; i++) { > + if (strlen(argm[i]) == 10) { > + if (strcmp(argm[i], "--cache-id") == 0) > + localmode = 1; > + } > + } Please use "streq()" instead of strcmp() to compare strings for equality. > + > + for (i=0; i + char *cur = argm[i]; > + > + if (strlen(cur) == 0) > + return buf; > + > + if (strlen(cur) == 6) { > + if (strcmp(cur, "--type") == 0) { > + ret = sprintf(str, "type="); > + str = str + ret; > + printcomma = 1; > + continue; > + } > + > + if (strcmp(cur, "--size") == 0) { > + ret = sprintf(str, "size="); > + str = str + ret; > + if (localmode == 1) > + printcomma = 1; > + else > + printsemicolon = 1; > + > + continue; > + } > + } > + > + if (strlen(cur) == 10) { > + if (strcmp(cur, "--cache-id") == 0) { > + ret = sprintf(str, "cache-id="); > + str = str + ret; > + printsemicolon = 1; > + continue; > + } > + } > + > + ret = sprintf(str, "%s", cur); > + str = s
Re: [systemd-devel] [PATCH] systemd: add RDTCacheReservation= option to support CAT (Cache Allocation Technology)
On Fri, Jan 06, 2017 at 01:51:11PM -0200, Marcelo Tosatti wrote: > On Fri, Jan 06, 2017 at 05:26:36PM +0200, Mantas Mikulėnas wrote: > > On Fri, Jan 6, 2017 at 3:59 PM, Marcelo Tosatti wrote: > > > > > > > > > > > Cache Allocation Technology is a feature on selected recent Intel Xeon > > > processors which allows control over L3 cache allocation. > > > > > > Kernel support has been merged to the upstream kernel, via a filesystem > > > resctrlfs. > > > > > > On top of that, a userspace utility, resctrltool has been written > > > to facilitate writing applications and using the filesystem > > > interface (see the rationale at > > > http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1300792.html). > > > > > > This patch adds a new option to systemd, RDTCacheReservation, > > > to allow configuration of CAT via resctrltool. > > > > > > See the first hunk of the patch for a description of the option > > > > > > This really doesn't look pretty, neither the approach nor the > > implementation... > > Suggestions to improve the code or the approach are welcome. > > > Is the option actually so complex that calling resctrltool is the only way > > to adjust it? What about writing to the resctrlfs directly? > > You'll have to deal with the issues that resctrltool deals with, > namely: > > 1) Filesystem locking. > 2) Reading in every directory and the default > directory. > 3) Converting the reservation request to proper sizes. > 4) Converting: > type=both --> type=data/type=code > > type=data/type=code --> type=both > > 4) Finding free space for the reservation. > 5) Adjusting the default group reservation. > > Since this steps must be performed by every user of > CAT (including libvirt which plans to execute resctrltool > as well), it was decided its better to maintain this logic > in a centralized place. > > Why do you prefer writing to to resctrlfs directly? > > > Also, it would be nicer to submit the patches as GitHub pull requests > > instead. > > Sure can do that after the reviews... Thanks for your comments! > > > > > > > > > +static char* convert_rdt_back(char *argm[], int argmidx) > > > +{ > > > + int i, ret; > > > + char *buf, *str; > > > + int size = 0; > > > + int printcomma = 0; > > > + int printsemicolon = 0; > > > + int localmode = 0; > > > + > > > + for (i=0; i < argmidx; i++) { > > > + /* ',', ';', '=' */ > > > + size += strlen(argm[i]) + 3; > > > + } > > > + > > > + buf = malloc(size); > > > + if (buf == NULL) > > > + return NULL; > > > + memset(buf, 0, size); > > > + > > > + str = buf; > > > + > > > + /* cache-id specified */ > > > + for (i=0; i < argmidx; i++) { > > > + if (strlen(argm[i]) == 10) { > > > + if (strcmp(argm[i], "--cache-id") == 0) > > > + localmode = 1; > > > + } > > > + } > > > + > > > + for (i=0; i > > + char *cur = argm[i]; > > > + > > > + if (strlen(cur) == 0) > > > + return buf; > > > + > > > + if (strlen(cur) == 6) { > > > + if (strcmp(cur, "--type") == 0) { > > > + ret = sprintf(str, "type="); > > > + str = str + ret; > > > + printcomma = 1; > > > + continue; > > > + } > > > + etc. > > > + } > > > + > > > + return buf; > > > +} > > > + > > > > > > > Converting from foo;bar to argv[] and back is a bit horrible. > > Its a very simple function. > > > It seems to > > me that it would be *much* simpler and easier to maintain if you stored > > only actual parameters in the unit (e.g. as an array of > > {type,size,cache_id}) and only generated argv at the moment it was > > necessary, i.e. right before spawning resctrltool. After all, that's the > > only place you need it. *And* that way you wouldn't need the above function > > at all. > > Honestly: > > An array of {type,size,cache_id} > > VS > > A list of: > > "--type" "type" "--size" size "--cache-id" id > > Is essentially the same thing to me. > > And you would have to convert back to the > > "type=type,size=size,cache-id=cacheid" > > format anyway. > > So i don't see any actual advantage of your suggestion > (other than personal preference). > > But sure, i'll convert it to an array of > > {type,size,cache_id}. Well, i take that back. Its the same data, just stored in a different way. So truly this is personal preference and not a technical argument. Can you give me one technical advantage of storing the data as {type,size,cache_id} ? Disadvantages: 1) you have to parse "mb" "kb" "gb" (documentation lacks to mention that). 2) you have to perform the following convertions:
Re: [systemd-devel] [PATCH] systemd: add RDTCacheReservation= option to support CAT (Cache Allocation Technology)
On Fri, Jan 06, 2017 at 05:26:36PM +0200, Mantas Mikulėnas wrote: > On Fri, Jan 6, 2017 at 3:59 PM, Marcelo Tosatti wrote: > > > > > > > Cache Allocation Technology is a feature on selected recent Intel Xeon > > processors which allows control over L3 cache allocation. > > > > Kernel support has been merged to the upstream kernel, via a filesystem > > resctrlfs. > > > > On top of that, a userspace utility, resctrltool has been written > > to facilitate writing applications and using the filesystem > > interface (see the rationale at > > http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1300792.html). > > > > This patch adds a new option to systemd, RDTCacheReservation, > > to allow configuration of CAT via resctrltool. > > > > See the first hunk of the patch for a description of the option > > > This really doesn't look pretty, neither the approach nor the > implementation... Suggestions to improve the code or the approach are welcome. > Is the option actually so complex that calling resctrltool is the only way > to adjust it? What about writing to the resctrlfs directly? You'll have to deal with the issues that resctrltool deals with, namely: 1) Filesystem locking. 2) Reading in every directory and the default directory. 3) Converting the reservation request to proper sizes. 4) Converting: type=both --> type=data/type=code type=data/type=code --> type=both 4) Finding free space for the reservation. 5) Adjusting the default group reservation. Since this steps must be performed by every user of CAT (including libvirt which plans to execute resctrltool as well), it was decided its better to maintain this logic in a centralized place. Why do you prefer writing to to resctrlfs directly? > Also, it would be nicer to submit the patches as GitHub pull requests > instead. Sure can do that after the reviews... Thanks for your comments! > > > > > +static char* convert_rdt_back(char *argm[], int argmidx) > > +{ > > + int i, ret; > > + char *buf, *str; > > + int size = 0; > > + int printcomma = 0; > > + int printsemicolon = 0; > > + int localmode = 0; > > + > > + for (i=0; i < argmidx; i++) { > > + /* ',', ';', '=' */ > > + size += strlen(argm[i]) + 3; > > + } > > + > > + buf = malloc(size); > > + if (buf == NULL) > > + return NULL; > > + memset(buf, 0, size); > > + > > + str = buf; > > + > > + /* cache-id specified */ > > + for (i=0; i < argmidx; i++) { > > + if (strlen(argm[i]) == 10) { > > + if (strcmp(argm[i], "--cache-id") == 0) > > + localmode = 1; > > + } > > + } > > + > > + for (i=0; i > + char *cur = argm[i]; > > + > > + if (strlen(cur) == 0) > > + return buf; > > + > > + if (strlen(cur) == 6) { > > + if (strcmp(cur, "--type") == 0) { > > + ret = sprintf(str, "type="); > > + str = str + ret; > > + printcomma = 1; > > + continue; > > + } > > + etc. > > + } > > + > > + return buf; > > +} > > + > > > > Converting from foo;bar to argv[] and back is a bit horrible. Its a very simple function. > It seems to > me that it would be *much* simpler and easier to maintain if you stored > only actual parameters in the unit (e.g. as an array of > {type,size,cache_id}) and only generated argv at the moment it was > necessary, i.e. right before spawning resctrltool. After all, that's the > only place you need it. *And* that way you wouldn't need the above function > at all. Honestly: An array of {type,size,cache_id} VS A list of: "--type" "type" "--size" size "--cache-id" id Is essentially the same thing to me. And you would have to convert back to the "type=type,size=size,cache-id=cacheid" format anyway. So i don't see any actual advantage of your suggestion (other than personal preference). But sure, i'll convert it to an array of {type,size,cache_id}. > > Index: systemd/src/core/execute.c > > === > > --- systemd.orig/src/core/execute.c > > +++ systemd/src/core/execute.c > > @@ -36,6 +36,10 @@ > > #include > > #include > > #include > > +#include > > +#include > > +#include > > +#include > > > > #ifdef HAVE_PAM > > #include > > @@ -2201,6 +2205,161 @@ static int apply_working_directory(const > > return 0; > > } > > > > + > > +static int fork_exec_resctrltool(Unit *u, char *argv[]) > > +{ > > +int outpipe[2]; > > +int errpipe[2]; > > +pid_t cpid; > > + > > +pipe(outpipe); > > +pipe(errpipe); > > +cpid = fork(); > > + > > +if(cpid == 0) { > > +
Re: [systemd-devel] [PATCH] systemd: add RDTCacheReservation= option to support CAT (Cache Allocation Technology)
On Fri, Jan 6, 2017 at 3:59 PM, Marcelo Tosatti wrote: > > > Cache Allocation Technology is a feature on selected recent Intel Xeon > processors which allows control over L3 cache allocation. > > Kernel support has been merged to the upstream kernel, via a filesystem > resctrlfs. > > On top of that, a userspace utility, resctrltool has been written > to facilitate writing applications and using the filesystem > interface (see the rationale at > http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1300792.html). > > This patch adds a new option to systemd, RDTCacheReservation, > to allow configuration of CAT via resctrltool. > > See the first hunk of the patch for a description of the option This really doesn't look pretty, neither the approach nor the implementation... Is the option actually so complex that calling resctrltool is the only way to adjust it? What about writing to the resctrlfs directly? Also, it would be nicer to submit the patches as GitHub pull requests instead. > > +static char* convert_rdt_back(char *argm[], int argmidx) > +{ > + int i, ret; > + char *buf, *str; > + int size = 0; > + int printcomma = 0; > + int printsemicolon = 0; > + int localmode = 0; > + > + for (i=0; i < argmidx; i++) { > + /* ',', ';', '=' */ > + size += strlen(argm[i]) + 3; > + } > + > + buf = malloc(size); > + if (buf == NULL) > + return NULL; > + memset(buf, 0, size); > + > + str = buf; > + > + /* cache-id specified */ > + for (i=0; i < argmidx; i++) { > + if (strlen(argm[i]) == 10) { > + if (strcmp(argm[i], "--cache-id") == 0) > + localmode = 1; > + } > + } > + > + for (i=0; i + char *cur = argm[i]; > + > + if (strlen(cur) == 0) > + return buf; > + > + if (strlen(cur) == 6) { > + if (strcmp(cur, "--type") == 0) { > + ret = sprintf(str, "type="); > + str = str + ret; > + printcomma = 1; > + continue; > + } > + etc. > + } > + > + return buf; > +} > + > Converting from foo;bar to argv[] and back is a bit horrible. It seems to me that it would be *much* simpler and easier to maintain if you stored only actual parameters in the unit (e.g. as an array of {type,size,cache_id}) and only generated argv at the moment it was necessary, i.e. right before spawning resctrltool. After all, that's the only place you need it. *And* that way you wouldn't need the above function at all. > > Index: systemd/src/core/execute.c > === > --- systemd.orig/src/core/execute.c > +++ systemd/src/core/execute.c > @@ -36,6 +36,10 @@ > #include > #include > #include > +#include > +#include > +#include > +#include > > #ifdef HAVE_PAM > #include > @@ -2201,6 +2205,161 @@ static int apply_working_directory(const > return 0; > } > > + > +static int fork_exec_resctrltool(Unit *u, char *argv[]) > +{ > +int outpipe[2]; > +int errpipe[2]; > +pid_t cpid; > + > +pipe(outpipe); > +pipe(errpipe); > +cpid = fork(); > + > +if(cpid == 0) { > +int r; > + > +dup2(errpipe[1], STDERR_FILENO); > +dup2(outpipe[1], STDOUT_FILENO); > + > +r = execve(argv[0], argv, NULL); > +if (r == -1) { > +log_open(); > +log_unit_error_errno(u, r, "Failed to execve > resctrltool, ignoring: %m"); > +log_close(); > +return -errno; > +} > +} else { > +char buffer[100]; > +fd_set fdset; > +int count; > +int ret; > +int nfds; > +int status = 0; > +int retst; > + > +ret = waitpid(cpid, &status, 0); > +if (ret == -1) { > +log_open(); > +log_unit_error_errno(u, ret, "Failed to waitpid > resctrltool, ignoring: %m"); > +log_close(); > +return -errno; > +} > + > +if (!WIFEXITED(status)) { > +log_open(); > +log_unit_error_errno(u, ret, "resctrltool exits > abnormally, ignoring: %m"); > +log_close(); > +return -1; > +} else { > +retst = WEXITSTATUS(status); > + > +if (retst == 0) { > + return 0; > +} > +
[systemd-devel] [PATCH] systemd: add RDTCacheReservation= option to support CAT (Cache Allocation Technology)
Cache Allocation Technology is a feature on selected recent Intel Xeon processors which allows control over L3 cache allocation. Kernel support has been merged to the upstream kernel, via a filesystem resctrlfs. On top of that, a userspace utility, resctrltool has been written to facilitate writing applications and using the filesystem interface (see the rationale at http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1300792.html). This patch adds a new option to systemd, RDTCacheReservation, to allow configuration of CAT via resctrltool. See the first hunk of the patch for a description of the option. Signed-off-by: Marcelo Tosatti Index: systemd/man/systemd.exec.xml === --- systemd.orig/man/systemd.exec.xml +++ systemd/man/systemd.exec.xml @@ -233,6 +233,31 @@ +RDTCacheReservation= + +Creates a L3 CAT reservation in resctrlfs +for the executed processes. Takes a ";" separated list of +tuples (optionally triples) containing type,size and +optionally cacheid: +type=type,size=size,cacheid=id; +type=type,size=size,cacheid=id;... + +Where: + +type is one of: both, data, code. +size is size in kbytes. +cacheid (optional) is the cacheid for +the reservation. + +Rules for the parameters: +* type=code must be paired with type=data entry. + +See the output of resctrltool for more details. + + + + + IOSchedulingClass= Sets the I/O scheduling class for executed Index: systemd/src/basic/exit-status.c === --- systemd.orig/src/basic/exit-status.c +++ systemd/src/basic/exit-status.c @@ -62,6 +62,9 @@ const char* exit_status_to_string(int st case EXIT_OOM_ADJUST: return "OOM_ADJUST"; +case EXIT_RDT_CACHE_RESERVATION: +return "RDT_CACHE_RESERVATION"; + case EXIT_SIGNAL_MASK: return "SIGNAL_MASK"; Index: systemd/src/basic/exit-status.h === --- systemd.orig/src/basic/exit-status.h +++ systemd/src/basic/exit-status.h @@ -83,6 +83,7 @@ enum { EXIT_CHOWN, EXIT_SMACK_PROCESS_LABEL, EXIT_KEYRING, +EXIT_RDT_CACHE_RESERVATION, }; typedef enum ExitStatusLevel { Index: systemd/src/core/dbus-execute.c === --- systemd.orig/src/core/dbus-execute.c +++ systemd/src/core/dbus-execute.c @@ -119,6 +119,36 @@ static int property_get_oom_score_adjust return sd_bus_message_append(reply, "i", n); } +static int property_get_rdt_cache_reservation( +sd_bus *bus, +const char *path, +const char *interface, +const char *property, +sd_bus_message *reply, +void *userdata, +sd_bus_error *error) { + +ExecContext *c = userdata; +int r; +int i; + +assert(bus); +assert(reply); +assert(c); + +r = sd_bus_message_open_container(reply, 'a', "s"); +if (r < 0) +return r; + +for (i = 0; i < c->argmidx; i++) { +r = sd_bus_message_append(reply, "s", c->argm[i]); +if (r < 0) +return r; +} + +return sd_bus_message_close_container(reply); +} + static int property_get_nice( sd_bus *bus, const char *path, @@ -759,6 +789,7 @@ const sd_bus_vtable bus_exec_vtable[] = SD_BUS_PROPERTY("WorkingDirectory", "s", property_get_working_directory, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RootDirectory", "s", NULL, offsetof(ExecContext, root_directory), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("OOMScoreAdjust", "i", property_get_oom_score_adjust, 0, SD_BUS_VTABLE_PROPERTY_CONST), +SD_BUS_PROPERTY("RDTCacheReservation", "as", property_get_rdt_cache_reservation, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Nice", "i", property_get_nice, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("IOScheduling", "i", property_get_ioprio, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("CPUSchedulingPolicy", "i", property_get_cpu_sched_policy, 0, SD_BUS_VTABLE_PROPERTY_CONST), @@ -921,6 +952,90 @@ int bus_property_get_exec_command_list( return sd_bus_message_close_container(reply); } +static char* convert_rdt_back(char *argm[], int argmidx) +{ + int i, ret; + char *buf, *str; + int size = 0; + int printcomma = 0; + int printsemicolon = 0; + int localmode = 0; + + for (i=0; i < argmidx; i++) { + /* ',', '