@cryptotaxi247 / netdata-1 / commits / 136547dd7

fix crash when the DRM file does not contain the right information (#19258)

* fix crash when the DRM file does not contain the right information * removed unecessary code * simplify the code and allow monitoring the clock speed of card that are sleeping

Costa Tsaousis committed Dec 20, 2024 at 21:11 UTC 136547dd7d4d855b5e43cb1db0a4e1d60301d091
1 file changed +38 -30
src/collectors/proc.plugin/sys_class_drm.c
+38 -30
@@ -646,29 +646,30 @@ cleanup:
646 static int read_clk_freq_file(procfile **p_ff, const char *const pathname, collected_number *num){
647 if(unlikely(!*p_ff)){
648 *p_ff = procfile_open(pathname, NULL, PROCFILE_FLAG_NO_ERROR_ON_FILE_IO);
649 - if(unlikely(!*p_ff)) return -2;
649 + if(unlikely(!*p_ff))
650 + return -1;
651 }
652
652 - if(unlikely(NULL == (*p_ff = procfile_readall(*p_ff)))) return -3;
653 + if(unlikely(NULL == (*p_ff = procfile_readall(*p_ff))))
654 + return -1;
655
656 for(size_t l = 0; l < procfile_lines(*p_ff) ; l++) {
657 char *str_with_units = NULL;
656 - if((*p_ff)->lines->lines[l].words >= 3 && !strcmp(procfile_lineword((*p_ff), l, 2), "*")) //format: X: collected_number *
658 +
659 + if(procfile_linewords(*p_ff, l) >= 3 && !strcmp(procfile_lineword((*p_ff), l, 2), "*")) //format: X: collected_number *
660 str_with_units = procfile_lineword((*p_ff), l, 1);
658 - else if ((*p_ff)->lines->lines[l].words == 2 && !strcmp(procfile_lineword((*p_ff), l, 1), "*")) //format: collected_number *
661 + else if (procfile_linewords(*p_ff, l) == 2 && !strcmp(procfile_lineword((*p_ff), l, 1), "*")) //format: collected_number *
662 str_with_units = procfile_lineword((*p_ff), l, 0);
663
664 if (str_with_units) {
662 - char *delim = strchr(str_with_units, 'M');
663 - char str_without_units[10];
664 - memcpy(str_without_units, str_with_units, delim - str_with_units);
665 - *num = str2ll(str_without_units, NULL);
665 + char *units = NULL;
666 + *num = str2ll(str_with_units, &units);
667 return 0;
668 }
669 }
670
670 - procfile_close((*p_ff));
671 - return -4;
671 + *num = 0; // the card is not active, so no speed reporting
672 + return 0;
673 }
674
675 static char *set_id(const char *const suf_1, const char *const suf_2, const char *const suf_3){
@@ -835,8 +836,8 @@ int do_sys_class_drm(int update_every, usec_t dt) {
836 int chart_prio = NETDATA_CHART_PRIO_DRM_AMDGPU;
837
838 if(unlikely(!drm_dir)) {
838 - char filename[FILENAME_MAX + 1];
839 - snprintfz(filename, FILENAME_MAX, "%s%s", netdata_configured_host_prefix, "/sys/class/drm");
839 + char filename[FILENAME_MAX];
840 + snprintfz(filename, sizeof(filename), "%s%s", netdata_configured_host_prefix, "/sys/class/drm");
841 const char *drm_dir_name = config_get(CONFIG_SECTION_PLUGIN_PROC_DRM, "directory to monitor", filename);
842 if(unlikely(NULL == (drm_dir = opendir(drm_dir_name)))){
843 collector_error("Cannot read directory '%s'", drm_dir_name);
@@ -849,23 +850,23 @@ int do_sys_class_drm(int update_every, usec_t dt) {
850 (de->d_name[0] == '.' && de->d_name[1] == '.' && de->d_name[2] == '\0'))) continue;
851
852 if(de->d_type == DT_LNK && !strncmp(de->d_name, "card", 4) && !strchr(de->d_name, '-')) {
852 - snprintfz(filename, FILENAME_MAX, "%s/%s/%s", drm_dir_name, de->d_name, "device/uevent");
853 + snprintfz(filename, sizeof(filename), "%s/%s/%s", drm_dir_name, de->d_name, "device/uevent");
854 if(check_card_is_amdgpu(filename)) continue;
855
856 /* Get static info */
857
858 struct card *const c = callocz(1, sizeof(struct card));
858 - snprintfz(filename, FILENAME_MAX, "%s/%s", drm_dir_name, de->d_name);
859 + snprintfz(filename, sizeof(filename), "%s/%s", drm_dir_name, de->d_name);
860 c->pathname = strdupz(filename);
861
861 - snprintfz(filename, FILENAME_MAX, "%s/%s", c->pathname, "device/device");
862 + snprintfz(filename, sizeof(filename), "%s/%s", c->pathname, "device/device");
863 if(read_single_base64_or_hex_number_file(filename, &c->id.asic_id)){
864 collector_error("Cannot read asic_id from '%s'", filename);
865 card_free(c);
866 continue;
867 }
868
868 - snprintfz(filename, FILENAME_MAX, "%s/%s", c->pathname, "device/revision");
869 + snprintfz(filename, sizeof(filename), "%s/%s", c->pathname, "device/revision");
870 if(read_single_base64_or_hex_number_file(filename, &c->id.pci_rev_id)){
871 collector_error("Cannot read pci_rev_id from '%s'", filename);
872 card_free(c);
@@ -883,10 +884,17 @@ int do_sys_class_drm(int update_every, usec_t dt) {
884
885
886 collected_number tmp_val;
886 - #define set_prop_pathname(prop_filename, prop_pathname, p_ff) do { \
887 - snprintfz(filename, FILENAME_MAX, "%s/%s", c->pathname, prop_filename); \
888 - if((p_ff && !read_clk_freq_file(p_ff, filename, &tmp_val)) || \
889 - !read_single_number_file(filename, (unsigned long long *) &tmp_val)) \
887 + #define set_prop_pathname_single_number_file(prop_filename, prop_pathname) do { \
888 + snprintfz(filename, sizeof(filename), "%s/%s", c->pathname, prop_filename); \
889 + if(!read_single_number_file(filename, (unsigned long long *) &tmp_val)) \
890 + prop_pathname = strdupz(filename); \
891 + else \
892 + collector_info("Cannot read file '%s'", filename); \
893 + } while(0)
894 +
895 + #define set_prop_pathname_clock_file(prop_filename, prop_pathname, p_ff) do { \
896 + snprintfz(filename, sizeof(filename), "%s/%s", c->pathname, prop_filename); \
897 + if(!read_clk_freq_file(p_ff, filename, &tmp_val)) \
898 prop_pathname = strdupz(filename); \
899 else \
900 collector_info("Cannot read file '%s'", filename); \
@@ -894,7 +902,7 @@ int do_sys_class_drm(int update_every, usec_t dt) {
902
903 /* Initialize GPU and VRAM utilization metrics */
904
897 - set_prop_pathname("device/gpu_busy_percent", c->pathname_util_gpu, NULL);
905 + set_prop_pathname_single_number_file("device/gpu_busy_percent", c->pathname_util_gpu);
906
907 if(c->pathname_util_gpu){
908 c->st_util_gpu = rrdset_create_localhost(
@@ -919,7 +927,7 @@ int do_sys_class_drm(int update_every, usec_t dt) {
927 add_do_rrd_x(c, do_rrd_util_gpu);
928 }
929
922 - set_prop_pathname("device/mem_busy_percent", c->pathname_util_mem, NULL);
930 + set_prop_pathname_single_number_file("device/mem_busy_percent", c->pathname_util_mem);
931
932 if(c->pathname_util_mem){
933 c->st_util_mem = rrdset_create_localhost(
@@ -947,7 +955,7 @@ int do_sys_class_drm(int update_every, usec_t dt) {
955
956 /* Initialize GPU and VRAM clock frequency metrics */
957
950 - set_prop_pathname("device/pp_dpm_sclk", c->pathname_clk_gpu, &c->ff_clk_gpu);
958 + set_prop_pathname_clock_file("device/pp_dpm_sclk", c->pathname_clk_gpu, &c->ff_clk_gpu);
959
960 if(c->pathname_clk_gpu){
961 c->st_clk_gpu = rrdset_create_localhost(
@@ -973,7 +981,7 @@ int do_sys_class_drm(int update_every, usec_t dt) {
981
982 }
983
976 - set_prop_pathname("device/pp_dpm_mclk", c->pathname_clk_mem, &c->ff_clk_mem);
984 + set_prop_pathname_clock_file("device/pp_dpm_mclk", c->pathname_clk_mem, &c->ff_clk_mem);
985
986 if(c->pathname_clk_mem){
987 c->st_clk_mem = rrdset_create_localhost(
@@ -1001,8 +1009,8 @@ int do_sys_class_drm(int update_every, usec_t dt) {
1009
1010 /* Initialize GPU memory usage metrics */
1011
1004 - set_prop_pathname("device/mem_info_vram_used", c->pathname_mem_used_vram, NULL);
1005 - set_prop_pathname("device/mem_info_vram_total", c->pathname_mem_total_vram, NULL);
1012 + set_prop_pathname_single_number_file("device/mem_info_vram_used", c->pathname_mem_used_vram);
1013 + set_prop_pathname_single_number_file("device/mem_info_vram_total", c->pathname_mem_total_vram);
1014 if(c->pathname_mem_total_vram) c->total_vram = tmp_val;
1015
1016 if(c->pathname_mem_used_vram && c->pathname_mem_total_vram){
@@ -1050,8 +1058,8 @@ int do_sys_class_drm(int update_every, usec_t dt) {
1058 add_do_rrd_x(c, do_rrd_vram);
1059 }
1060
1053 - set_prop_pathname("device/mem_info_vis_vram_used", c->pathname_mem_used_vis_vram, NULL);
1054 - set_prop_pathname("device/mem_info_vis_vram_total", c->pathname_mem_total_vis_vram, NULL);
1061 + set_prop_pathname_single_number_file("device/mem_info_vis_vram_used", c->pathname_mem_used_vis_vram);
1062 + set_prop_pathname_single_number_file("device/mem_info_vis_vram_total", c->pathname_mem_total_vis_vram);
1063 if(c->pathname_mem_total_vis_vram) c->total_vis_vram = tmp_val;
1064
1065 if(c->pathname_mem_used_vis_vram && c->pathname_mem_total_vis_vram){
@@ -1099,8 +1107,8 @@ int do_sys_class_drm(int update_every, usec_t dt) {
1107 add_do_rrd_x(c, do_rrd_vis_vram);
1108 }
1109
1102 - set_prop_pathname("device/mem_info_gtt_used", c->pathname_mem_used_gtt, NULL);
1103 - set_prop_pathname("device/mem_info_gtt_total", c->pathname_mem_total_gtt, NULL);
1110 + set_prop_pathname_single_number_file("device/mem_info_gtt_used", c->pathname_mem_used_gtt);
1111 + set_prop_pathname_single_number_file("device/mem_info_gtt_total", c->pathname_mem_total_gtt);
1112 if(c->pathname_mem_total_gtt) c->total_gtt = tmp_val;
1113
1114 if(c->pathname_mem_used_gtt && c->pathname_mem_total_gtt){