| Message ID | 20230626023731.115783-1-carlosrodrifernandez@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libbpf: provide num present cpus | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 6/25/23 7:34 PM, Carlos Rodriguez-Fernandez wrote: > It allows tools to iterate over CPUs present > in the system that are actually running processes, > which can be less than the number of possible CPUs. Please add more context here. There exists a bcc issue https://github.com/iovisor/bcc/issues/4651 which shows the context for this patch set. Basically it is to address bcc/libbpf-tools/cpufreq failure in case that some cpus are not present. Carlos, you need to show what can be done in tool itself to resolve the issue vs. to use the API from what is proposed here to resolve the issue. So it would become clear how the new API might help or not. > > Signed-off-by: Carlos Rodriguez-Fernandez <carlosrodrifernandez@gmail.com> > --- > src/libbpf.c | 32 +++++++++++++++++++++++++++----- > src/libbpf.h | 8 +++++--- > 2 files changed, 32 insertions(+), 8 deletions(-) > > diff --git a/src/libbpf.c b/src/libbpf.c > index 214f828..e42d252 100644 > --- a/src/libbpf.c > +++ b/src/libbpf.c > @@ -12615,14 +12615,26 @@ int parse_cpu_mask_file(const char *fcpu, bool **mask, int *mask_sz) > return parse_cpu_mask_str(buf, mask, mask_sz); > } > > -int libbpf_num_possible_cpus(void) > +typedef enum {POSSIBLE=0, PRESENT, NUM_TYPES } CPU_TOPOLOGY_SYSFS_TYPE; > + > +static const char *cpu_topology_sysfs_path_by_type(const CPU_TOPOLOGY_SYSFS_TYPE type) { > + const static char *possible_sysfs_path = "/sys/devices/system/cpu/possible"; > + const static char *present_sysfs_path = "/sys/devices/system/cpu/present"; > + switch(type) { > + case POSSIBLE: return possible_sysfs_path; > + case PRESENT: return present_sysfs_path; > + default: return possible_sysfs_path; > + } > +} > + > +int libbpf_num_cpus_by_topology_sysfs_type(const CPU_TOPOLOGY_SYSFS_TYPE type) > { > - static const char *fcpu = "/sys/devices/system/cpu/possible"; > - static int cpus; > + const char *fcpu = cpu_topology_sysfs_path_by_type(type); > + static int cpus[NUM_TYPES]; > int err, n, i, tmp_cpus; > bool *mask; > > - tmp_cpus = READ_ONCE(cpus); > + tmp_cpus = READ_ONCE(cpus[type]); > if (tmp_cpus > 0) > return tmp_cpus; > > @@ -12637,10 +12649,20 @@ int libbpf_num_possible_cpus(void) > } > free(mask); > > - WRITE_ONCE(cpus, tmp_cpus); > + WRITE_ONCE(cpus[type], tmp_cpus); > return tmp_cpus; > } > > +int libbpf_num_possible_cpus(void) > +{ > + return libbpf_num_cpus_by_topology_sysfs_type(POSSIBLE); > +} > + > +int libbpf_num_present_cpus(void) > +{ > + return libbpf_num_cpus_by_topology_sysfs_type(PRESENT); > +} > + > static int populate_skeleton_maps(const struct bpf_object *obj, > struct bpf_map_skeleton *maps, > size_t map_cnt) > diff --git a/src/libbpf.h b/src/libbpf.h > index 754da73..a34152c 100644 > --- a/src/libbpf.h > +++ b/src/libbpf.h > @@ -1433,9 +1433,10 @@ LIBBPF_API int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, > enum bpf_func_id helper_id, const void *opts); > > /** > - * @brief **libbpf_num_possible_cpus()** is a helper function to get the > - * number of possible CPUs that the host kernel supports and expects. > - * @return number of possible CPUs; or error code on failure > + * @brief **libbpf_num_possible_cpus()**, and **libbpf_num_present_cpus()** > + * are helper functions to get the number of possible, and present CPUs respectivelly. > + * See for more information: https://www.kernel.org/doc/html/latest/admin-guide/cputopology.html > + * @return number of CPUs; or error code on failure > * > * Example usage: > * > @@ -1447,6 +1448,7 @@ LIBBPF_API int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, > * bpf_map_lookup_elem(per_cpu_map_fd, key, values); > */ > LIBBPF_API int libbpf_num_possible_cpus(void); > +LIBBPF_API int libbpf_num_present_cpus(void); > > struct bpf_map_skeleton { > const char *name;
Hi Yonghong, Thank you for looking into this. I replied on the bcc issue. I think your approach of modifying the tool itself should work. Even though there could be some value adding a helper function for "present", it is not enough because it doesn't give you the online CPUs as you said in the bcc issue. A function returning the online CPUs list in a bit array or equivalent could be a valuable function to avoid depending on avoidable errors. I apologize not giving enough context in these emails. I sent an email earlier explaining the context (see the email "Possible vs present number of cpus and propose for a new API function"), but then I realized I was sending the patch wrong, so I used the git send-email option. Sorry for the confusion. Regards, Carlos. On 6/26/23 10:02, Yonghong Song wrote: > > > On 6/25/23 7:34 PM, Carlos Rodriguez-Fernandez wrote: >> It allows tools to iterate over CPUs present >> in the system that are actually running processes, >> which can be less than the number of possible CPUs. > > Please add more context here. There exists a bcc issue > https://github.com/iovisor/bcc/issues/4651 > which shows the context for this patch set. > Basically it is to address bcc/libbpf-tools/cpufreq > failure in case that some cpus are not present. > > Carlos, you need to show what can be done > in tool itself to resolve the issue vs. > to use the API from what is proposed here > to resolve the issue. So it would become > clear how the new API might help or not. > >> >> Signed-off-by: Carlos Rodriguez-Fernandez >> <carlosrodrifernandez@gmail.com> >> --- >> src/libbpf.c | 32 +++++++++++++++++++++++++++----- >> src/libbpf.h | 8 +++++--- >> 2 files changed, 32 insertions(+), 8 deletions(-) >> >> diff --git a/src/libbpf.c b/src/libbpf.c >> index 214f828..e42d252 100644 >> --- a/src/libbpf.c >> +++ b/src/libbpf.c >> @@ -12615,14 +12615,26 @@ int parse_cpu_mask_file(const char *fcpu, >> bool **mask, int *mask_sz) >> return parse_cpu_mask_str(buf, mask, mask_sz); >> } >> -int libbpf_num_possible_cpus(void) >> +typedef enum {POSSIBLE=0, PRESENT, NUM_TYPES } CPU_TOPOLOGY_SYSFS_TYPE; >> + >> +static const char *cpu_topology_sysfs_path_by_type(const >> CPU_TOPOLOGY_SYSFS_TYPE type) { >> + const static char *possible_sysfs_path = >> "/sys/devices/system/cpu/possible"; >> + const static char *present_sysfs_path = >> "/sys/devices/system/cpu/present"; >> + switch(type) { >> + case POSSIBLE: return possible_sysfs_path; >> + case PRESENT: return present_sysfs_path; >> + default: return possible_sysfs_path; >> + } >> +} >> + >> +int libbpf_num_cpus_by_topology_sysfs_type(const >> CPU_TOPOLOGY_SYSFS_TYPE type) >> { >> - static const char *fcpu = "/sys/devices/system/cpu/possible"; >> - static int cpus; >> + const char *fcpu = cpu_topology_sysfs_path_by_type(type); >> + static int cpus[NUM_TYPES]; >> int err, n, i, tmp_cpus; >> bool *mask; >> - tmp_cpus = READ_ONCE(cpus); >> + tmp_cpus = READ_ONCE(cpus[type]); >> if (tmp_cpus > 0) >> return tmp_cpus; >> @@ -12637,10 +12649,20 @@ int libbpf_num_possible_cpus(void) >> } >> free(mask); >> - WRITE_ONCE(cpus, tmp_cpus); >> + WRITE_ONCE(cpus[type], tmp_cpus); >> return tmp_cpus; >> } >> +int libbpf_num_possible_cpus(void) >> +{ >> + return libbpf_num_cpus_by_topology_sysfs_type(POSSIBLE); >> +} >> + >> +int libbpf_num_present_cpus(void) >> +{ >> + return libbpf_num_cpus_by_topology_sysfs_type(PRESENT); >> +} >> + >> static int populate_skeleton_maps(const struct bpf_object *obj, >> struct bpf_map_skeleton *maps, >> size_t map_cnt) >> diff --git a/src/libbpf.h b/src/libbpf.h >> index 754da73..a34152c 100644 >> --- a/src/libbpf.h >> +++ b/src/libbpf.h >> @@ -1433,9 +1433,10 @@ LIBBPF_API int libbpf_probe_bpf_helper(enum >> bpf_prog_type prog_type, >> enum bpf_func_id helper_id, const void *opts); >> /** >> - * @brief **libbpf_num_possible_cpus()** is a helper function to get the >> - * number of possible CPUs that the host kernel supports and expects. >> - * @return number of possible CPUs; or error code on failure >> + * @brief **libbpf_num_possible_cpus()**, and >> **libbpf_num_present_cpus()** >> + * are helper functions to get the number of possible, and present >> CPUs respectivelly. >> + * See for more information: >> https://www.kernel.org/doc/html/latest/admin-guide/cputopology.html >> + * @return number of CPUs; or error code on failure >> * >> * Example usage: >> * >> @@ -1447,6 +1448,7 @@ LIBBPF_API int libbpf_probe_bpf_helper(enum >> bpf_prog_type prog_type, >> * bpf_map_lookup_elem(per_cpu_map_fd, key, values); >> */ >> LIBBPF_API int libbpf_num_possible_cpus(void); >> +LIBBPF_API int libbpf_num_present_cpus(void); >> struct bpf_map_skeleton { >> const char *name;
diff --git a/src/libbpf.c b/src/libbpf.c index 214f828..e42d252 100644 --- a/src/libbpf.c +++ b/src/libbpf.c @@ -12615,14 +12615,26 @@ int parse_cpu_mask_file(const char *fcpu, bool **mask, int *mask_sz) return parse_cpu_mask_str(buf, mask, mask_sz); } -int libbpf_num_possible_cpus(void) +typedef enum {POSSIBLE=0, PRESENT, NUM_TYPES } CPU_TOPOLOGY_SYSFS_TYPE; + +static const char *cpu_topology_sysfs_path_by_type(const CPU_TOPOLOGY_SYSFS_TYPE type) { + const static char *possible_sysfs_path = "/sys/devices/system/cpu/possible"; + const static char *present_sysfs_path = "/sys/devices/system/cpu/present"; + switch(type) { + case POSSIBLE: return possible_sysfs_path; + case PRESENT: return present_sysfs_path; + default: return possible_sysfs_path; + } +} + +int libbpf_num_cpus_by_topology_sysfs_type(const CPU_TOPOLOGY_SYSFS_TYPE type) { - static const char *fcpu = "/sys/devices/system/cpu/possible"; - static int cpus; + const char *fcpu = cpu_topology_sysfs_path_by_type(type); + static int cpus[NUM_TYPES]; int err, n, i, tmp_cpus; bool *mask; - tmp_cpus = READ_ONCE(cpus); + tmp_cpus = READ_ONCE(cpus[type]); if (tmp_cpus > 0) return tmp_cpus; @@ -12637,10 +12649,20 @@ int libbpf_num_possible_cpus(void) } free(mask); - WRITE_ONCE(cpus, tmp_cpus); + WRITE_ONCE(cpus[type], tmp_cpus); return tmp_cpus; } +int libbpf_num_possible_cpus(void) +{ + return libbpf_num_cpus_by_topology_sysfs_type(POSSIBLE); +} + +int libbpf_num_present_cpus(void) +{ + return libbpf_num_cpus_by_topology_sysfs_type(PRESENT); +} + static int populate_skeleton_maps(const struct bpf_object *obj, struct bpf_map_skeleton *maps, size_t map_cnt) diff --git a/src/libbpf.h b/src/libbpf.h index 754da73..a34152c 100644 --- a/src/libbpf.h +++ b/src/libbpf.h @@ -1433,9 +1433,10 @@ LIBBPF_API int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, enum bpf_func_id helper_id, const void *opts); /** - * @brief **libbpf_num_possible_cpus()** is a helper function to get the - * number of possible CPUs that the host kernel supports and expects. - * @return number of possible CPUs; or error code on failure + * @brief **libbpf_num_possible_cpus()**, and **libbpf_num_present_cpus()** + * are helper functions to get the number of possible, and present CPUs respectivelly. + * See for more information: https://www.kernel.org/doc/html/latest/admin-guide/cputopology.html + * @return number of CPUs; or error code on failure * * Example usage: * @@ -1447,6 +1448,7 @@ LIBBPF_API int libbpf_probe_bpf_helper(enum bpf_prog_type prog_type, * bpf_map_lookup_elem(per_cpu_map_fd, key, values); */ LIBBPF_API int libbpf_num_possible_cpus(void); +LIBBPF_API int libbpf_num_present_cpus(void); struct bpf_map_skeleton { const char *name;
It allows tools to iterate over CPUs present in the system that are actually running processes, which can be less than the number of possible CPUs. Signed-off-by: Carlos Rodriguez-Fernandez <carlosrodrifernandez@gmail.com> --- src/libbpf.c | 32 +++++++++++++++++++++++++++----- src/libbpf.h | 8 +++++--- 2 files changed, 32 insertions(+), 8 deletions(-)