@cryptotaxi247 / netdata-1 / commits / 0cdef2a88

diskspace: reworked the cleanup to fix race conditions (#16786)

reworked the cleanup to fix race conditions

Costa Tsaousis committed Jan 14, 2024 at 15:58 UTC 0cdef2a8840f97e56cd2dcc22c45a476cd32e966
1 file changed +89 -92
collectors/diskspace.plugin/plugin_diskspace.c
+89 -92
@@ -40,11 +40,10 @@ static inline void mountinfo_reload(int force) {
40 struct mount_point_metadata {
41 int do_space;
42 int do_inodes;
43 - int shown_error;
44 - int updated;
45 - int slow;
43
47 - bool function_ready;
44 + bool shown_error;
45 + bool updated;
46 + bool slow;
47
48 STRING *filesystem;
49 STRING *mountroot;
@@ -68,48 +67,44 @@ static DICTIONARY *dict_mountpoints = NULL;
67
68 #define rrdset_obsolete_and_pointer_null(st) do { if(st) { rrdset_is_obsolete___safe_from_collector_thread(st); (st) = NULL; } } while(st)
69
71 -int mount_point_cleanup(const char *name, void *entry, int slow) {
72 - (void)name;
73 -
74 - struct mount_point_metadata *mp = (struct mount_point_metadata *)entry;
75 - if(!mp) return 0;
76 -
77 - if (slow != mp->slow)
78 - return 0;
70 +static void mount_points_cleanup(bool slow) {
71 + struct mount_point_metadata *mp;
72 + dfe_start_write(dict_mountpoints, mp) {
73 + if(mp->slow != slow) continue;
74
80 - if(likely(mp->updated)) {
81 - mp->updated = 0;
82 - return 0;
75 + if(mp->updated)
76 + mp->updated = false;
77 + else if(cleanup_mount_points)
78 + dictionary_del(dict_mountpoints, mp_dfe.name);
79 }
80 + dfe_done(mp);
81
85 - if(likely(cleanup_mount_points && mp->collected)) {
86 - mp->function_ready = false;
87 - mp->collected = 0;
88 - mp->updated = 0;
89 - mp->shown_error = 0;
82 + dictionary_garbage_collect(dict_mountpoints);
83 +}
84
91 - string_freez(mp->filesystem);
92 - string_freez(mp->mountroot);
85 +void mountpoint_delete_cb(const DICTIONARY_ITEM *item __maybe_unused, void *entry, void *data __maybe_unused) {
86 + struct mount_point_metadata *mp = (struct mount_point_metadata *)entry;
87
94 - rrdset_obsolete_and_pointer_null(mp->st_space);
95 - rrdset_obsolete_and_pointer_null(mp->st_inodes);
88 + mp->collected = 0;
89 + mp->updated = false;
90 + mp->shown_error = false;
91
97 - mp->rd_space_avail = NULL;
98 - mp->rd_space_used = NULL;
99 - mp->rd_space_reserved = NULL;
92 + string_freez(mp->filesystem);
93 + mp->filesystem = NULL;
94
101 - mp->rd_inodes_avail = NULL;
102 - mp->rd_inodes_used = NULL;
103 - mp->rd_inodes_reserved = NULL;
104 - }
95 + string_freez(mp->mountroot);
96 + mp->mountroot = NULL;
97
106 - return 0;
107 -}
98 + rrdset_obsolete_and_pointer_null(mp->st_space);
99 + rrdset_obsolete_and_pointer_null(mp->st_inodes);
100
109 -int mount_point_cleanup_cb(const DICTIONARY_ITEM *item, void *entry, void *data __maybe_unused) {
110 - const char *name = dictionary_acquired_item_name(item);
101 + mp->rd_space_avail = NULL;
102 + mp->rd_space_used = NULL;
103 + mp->rd_space_reserved = NULL;
104
112 - return mount_point_cleanup(name, (struct mount_point_metadata *)entry, 0);
105 + mp->rd_inodes_avail = NULL;
106 + mp->rd_inodes_used = NULL;
107 + mp->rd_inodes_reserved = NULL;
108 }
109
110 // a copy of basic mountinfo fields
@@ -297,8 +292,6 @@ static void calculate_values_and_show_charts(
292 rendered++;
293 }
294
300 - m->function_ready = rendered > 0;
301 -
295 if(likely(rendered))
296 m->collected++;
297 }
@@ -341,11 +334,12 @@ static inline void do_disk_space_stats(struct mountinfo *mi, int update_every) {
334 true);
335
336 dict_mountpoints = dictionary_create_advanced(DICT_OPTION_NONE, &dictionary_stats_category_collectors, 0);
337 + dictionary_register_delete_callback(dict_mountpoints, mountpoint_delete_cb, NULL);
338 }
339
346 - struct mount_point_metadata *m = dictionary_get(dict_mountpoints, mi->mount_point);
347 - if(unlikely(!m)) {
348 - int slow = 0;
340 + const DICTIONARY_ITEM *item = dictionary_get_and_acquire_item(dict_mountpoints, mi->mount_point);
341 + if(unlikely(!item)) {
342 + bool slow = false;
343
344 int def_space = config_get_boolean_ondemand(CONFIG_SECTION_DISKSPACE, "space usage for all disks", CONFIG_BOOLEAN_AUTO);
345 int def_inodes = config_get_boolean_ondemand(CONFIG_SECTION_DISKSPACE, "inodes usage for all disks", CONFIG_BOOLEAN_AUTO);
@@ -393,7 +387,7 @@ static inline void do_disk_space_stats(struct mountinfo *mi, int update_every) {
387 }
388
389 if ((now_monotonic_high_precision_usec() - start_time) > slow_timeout)
396 - slow = 1;
390 + slow = true;
391 }
392
393 char var_name[4096 + 1];
@@ -408,55 +402,55 @@ static inline void do_disk_space_stats(struct mountinfo *mi, int update_every) {
402 do_inodes = config_get_boolean_ondemand(var_name, "inodes usage", def_inodes);
403
404 struct mount_point_metadata mp = {
411 - .do_space = do_space,
412 - .do_inodes = do_inodes,
413 - .shown_error = 0,
414 - .updated = 0,
415 - .slow = 0,
416 -
417 - .collected = 0,
418 -
419 - .st_space = NULL,
420 - .rd_space_avail = NULL,
421 - .rd_space_used = NULL,
422 - .rd_space_reserved = NULL,
423 -
424 - .st_inodes = NULL,
425 - .rd_inodes_avail = NULL,
426 - .rd_inodes_used = NULL,
427 - .rd_inodes_reserved = NULL
405 + .do_space = do_space,
406 + .do_inodes = do_inodes,
407 + .shown_error = false,
408 + .updated = false,
409 + .slow = slow,
410 +
411 + .collected = 0,
412 + .filesystem = string_strdupz(mi->filesystem),
413 + .mountroot = string_strdupz(mi->root),
414 + .chart_labels = rrdlabels_create(),
415 +
416 + .st_space = NULL,
417 + .rd_space_avail = NULL,
418 + .rd_space_used = NULL,
419 + .rd_space_reserved = NULL,
420 +
421 + .st_inodes = NULL,
422 + .rd_inodes_avail = NULL,
423 + .rd_inodes_used = NULL,
424 + .rd_inodes_reserved = NULL
425 };
426
430 - mp.filesystem = string_strdupz(mi->filesystem);
431 - mp.mountroot = string_strdupz(mi->root);
432 -
433 - mp.chart_labels = rrdlabels_create();
427 rrdlabels_add(mp.chart_labels, "mount_point", mi->mount_point, RRDLABEL_SRC_AUTO);
428 rrdlabels_add(mp.chart_labels, "filesystem", mi->filesystem, RRDLABEL_SRC_AUTO);
429 rrdlabels_add(mp.chart_labels, "mount_root", mi->root, RRDLABEL_SRC_AUTO);
430
438 - m = dictionary_set(dict_mountpoints, mi->mount_point, &mp, sizeof(struct mount_point_metadata));
439 -
440 - m->slow = slow;
431 + item = dictionary_set_and_acquire_item(dict_mountpoints, mi->mount_point, &mp, sizeof(struct mount_point_metadata));
432 }
433
434 + struct mount_point_metadata *m = dictionary_acquired_item_value(item);
435 if (m->slow) {
436 add_basic_mountinfo(&slow_mountinfo_tmp_root, mi);
445 - return;
437 + goto cleanup;
438 }
439
448 - m->updated = 1;
440 + m->updated = true;
441
450 - if(unlikely(m->do_space == CONFIG_BOOLEAN_NO && m->do_inodes == CONFIG_BOOLEAN_NO))
451 - return;
442 + if(unlikely(m->do_space == CONFIG_BOOLEAN_NO && m->do_inodes == CONFIG_BOOLEAN_NO)) {
443 + goto cleanup;
444 + }
445
446 if (unlikely(
447 mi->flags & MOUNTINFO_READONLY &&
448 !(mi->flags & MOUNTINFO_IS_IN_SYSD_PROTECTED_LIST) &&
449 !m->collected &&
450 m->do_space != CONFIG_BOOLEAN_YES &&
458 - m->do_inodes != CONFIG_BOOLEAN_YES))
459 - return;
451 + m->do_inodes != CONFIG_BOOLEAN_YES)) {
452 + goto cleanup;
453 + }
454
455 usec_t start_time = now_monotonic_high_precision_usec();
456 struct statvfs buff_statvfs;
@@ -469,15 +463,15 @@ static inline void do_disk_space_stats(struct mountinfo *mi, int update_every) {
463 , mi->filesystem?mi->filesystem:""
464 , mi->root?mi->root:""
465 );
472 - m->shown_error = 1;
466 + m->shown_error = true;
467 }
474 - return;
468 + goto cleanup;
469 }
470
471 if ((now_monotonic_high_precision_usec() - start_time) > slow_timeout)
478 - m->slow = 1;
472 + m->slow = true;
473
480 - m->shown_error = 0;
474 + m->shown_error = false;
475
476 struct basic_mountinfo bmi;
477 bmi.mount_point = mi->mount_point;
@@ -486,12 +480,17 @@ static inline void do_disk_space_stats(struct mountinfo *mi, int update_every) {
480 bmi.root = mi->root;
481
482 calculate_values_and_show_charts(&bmi, m, &buff_statvfs, update_every);
483 +
484 +cleanup:
485 + dictionary_acquired_item_release(dict_mountpoints, item);
486 }
487
488 static inline void do_slow_disk_space_stats(struct basic_mountinfo *mi, int update_every) {
492 - struct mount_point_metadata *m = dictionary_get(dict_mountpoints, mi->mount_point);
489 + const DICTIONARY_ITEM *item = dictionary_get_and_acquire_item(dict_mountpoints, mi->mount_point);
490 + if(!item) return;
491
494 - m->updated = 1;
492 + struct mount_point_metadata *m = dictionary_acquired_item_value(item);
493 + m->updated = true;
494
495 struct statvfs buff_statvfs;
496 if (statvfs(mi->mount_point, &buff_statvfs) < 0) {
@@ -502,13 +501,16 @@ static inline void do_slow_disk_space_stats(struct basic_mountinfo *mi, int upda
501 , mi->filesystem?mi->filesystem:""
502 , mi->root?mi->root:""
503 );
505 - m->shown_error = 1;
504 + m->shown_error = true;
505 }
507 - return;
506 + goto cleanup;
507 }
509 - m->shown_error = 0;
508 + m->shown_error = false;
509
510 calculate_values_and_show_charts(mi, m, &buff_statvfs, update_every);
511 +
512 +cleanup:
513 + dictionary_acquired_item_release(dict_mountpoints, item);
514 }
515
516 static void diskspace_slow_worker_cleanup(void *ptr)
@@ -584,13 +586,9 @@ void *diskspace_slow_worker(void *ptr)
586
587 if(unlikely(!service_running(SERVICE_COLLECTORS))) break;
588
587 - worker_is_busy(WORKER_JOB_SLOW_CLEANUP);
588 -
589 - for(bmi = slow_mountinfo_root; bmi; bmi = bmi->next) {
590 - struct mount_point_metadata *m = dictionary_get(dict_mountpoints, bmi->mount_point);
591 -
592 - if (m)
593 - mount_point_cleanup(bmi->mount_point, m, 1);
589 + if(dict_mountpoints) {
590 + worker_is_busy(WORKER_JOB_SLOW_CLEANUP);
591 + mount_points_cleanup(true);
592 }
593
594 usec_t dt = now_monotonic_high_precision_usec() - start_time;
@@ -661,8 +659,8 @@ int diskspace_function_mount_points(BUFFER *wb, const char *function __maybe_unu
659 double max_inodes_reserved = 0.0;
660
661 struct mount_point_metadata *mp;
664 - dfe_start_write(dict_mountpoints, mp) {
665 - if (!mp->function_ready)
662 + dfe_start_read(dict_mountpoints, mp) {
663 + if (!mp->collected)
664 continue;
665
666 buffer_json_add_array_item_array(wb);
@@ -923,9 +921,8 @@ void *diskspace_main(void *ptr) {
921
922 if(dict_mountpoints) {
923 worker_is_busy(WORKER_JOB_CLEANUP);
926 - dictionary_walkthrough_read(dict_mountpoints, mount_point_cleanup_cb, NULL);
924 + mount_points_cleanup(false);
925 }
928 -
926 }
927 worker_unregister();
928