[Mesa-dev] [PATCH] gallium/hud: Sensor extension is segfaulting.
Steven Toth
stoth at kernellabs.com
Thu Oct 13 17:29:15 UTC 2016
Round two of the patchset, incorporating feedback
from nhaehnle at gmail.com
The fixes in this set address bugfix #68169, HUD crashing
when testing with unigine (heaven).
The bug also manifested itself as a lack of data in
HUD charts when multiple instanced were created and
destroyed in a specific order, a use case not witnessed
with glxgears.
These patches:
1. mutex protect the internal object lists.
2. reference count object access for destruction purposes.
Patchset tested with glxgears/demo and unigine.
Signed-off-by: Steven Toth <stoth at kernellabs.com>
---
src/gallium/auxiliary/hud/hud_cpufreq.c | 33 ++++++++++++++++++----
src/gallium/auxiliary/hud/hud_diskstat.c | 39 ++++++++++++++++++++++----
src/gallium/auxiliary/hud/hud_nic.c | 37 +++++++++++++++++++++----
src/gallium/auxiliary/hud/hud_sensors_temp.c | 41 ++++++++++++++++++++++------
4 files changed, 125 insertions(+), 25 deletions(-)
diff --git a/src/gallium/auxiliary/hud/hud_cpufreq.c b/src/gallium/auxiliary/hud/hud_cpufreq.c
index 4501bbb..e193568 100644
--- a/src/gallium/auxiliary/hud/hud_cpufreq.c
+++ b/src/gallium/auxiliary/hud/hud_cpufreq.c
@@ -37,6 +37,8 @@
#include "util/list.h"
#include "os/os_time.h"
#include "util/u_memory.h"
+#include "util/u_inlines.h"
+#include "os/os_thread.h"
#include <stdio.h>
#include <unistd.h>
#include <dirent.h>
@@ -57,21 +59,28 @@ struct cpufreq_info
char sysfs_filename[128];
uint64_t KHz;
uint64_t last_time;
+ struct pipe_reference refcnt;
};
static int gcpufreq_count = 0;
static struct list_head gcpufreq_list;
+static pipe_mutex gcpufreq_mutex;
static struct cpufreq_info *
find_cfi_by_index(int cpu_index, int mode)
{
+ struct cpufreq_info *ret = 0;
+ pipe_mutex_lock(gcpufreq_mutex);
list_for_each_entry(struct cpufreq_info, cfi, &gcpufreq_list, list) {
if (cfi->mode != mode)
continue;
- if (cfi->cpu_index == cpu_index)
- return cfi;
+ if (cfi->cpu_index == cpu_index) {
+ ret = cfi;
+ break;
+ }
}
- return 0;
+ pipe_mutex_unlock(gcpufreq_mutex);
+ return ret;
}
static int
@@ -116,8 +125,14 @@ static void
free_query_data(void *p)
{
struct cpufreq_info *cfi = (struct cpufreq_info *)p;
- list_del(&cfi->list);
- FREE(cfi);
+ /* atomic dec */
+ if (pipe_reference(&cfi->refcnt, NULL)) {
+ pipe_mutex_lock(gcpufreq_mutex);
+ list_del(&cfi->list);
+ gcpufreq_count--;
+ pipe_mutex_unlock(gcpufreq_mutex);
+ FREE(cfi);
+ }
}
/**
@@ -159,6 +174,7 @@ hud_cpufreq_graph_install(struct hud_pane *pane, int cpu_index,
return;
}
+ pipe_reference(NULL, &cfi->refcnt); /* atomic inc */
gr->query_data = cfi;
gr->query_new_value = query_cfi_load;
@@ -180,8 +196,12 @@ add_object(const char *name, const char *fn, int objmode, int cpu_index)
strcpy(cfi->sysfs_filename, fn);
cfi->mode = objmode;
cfi->cpu_index = cpu_index;
+ pipe_reference_init(&cfi->refcnt, 1);
+
+ pipe_mutex_lock(gcpufreq_mutex);
list_addtail(&cfi->list, &gcpufreq_list);
gcpufreq_count++;
+ pipe_mutex_unlock(gcpufreq_mutex);
}
/**
@@ -205,6 +225,7 @@ hud_get_num_cpufreq(bool displayhelp)
/* Scan /sys/devices.../cpu, for every object type we support, create
* and persist an object to represent its different metrics.
*/
+ pipe_mutex_init(gcpufreq_mutex);
list_inithead(&gcpufreq_list);
DIR *dir = opendir("/sys/devices/system/cpu");
if (!dir)
@@ -240,6 +261,7 @@ hud_get_num_cpufreq(bool displayhelp)
}
if (displayhelp) {
+ pipe_mutex_lock(gcpufreq_mutex);
list_for_each_entry(struct cpufreq_info, cfi, &gcpufreq_list, list) {
char line[128];
snprintf(line, sizeof(line), " cpufreq-%s-%s",
@@ -249,6 +271,7 @@ hud_get_num_cpufreq(bool displayhelp)
puts(line);
}
+ pipe_mutex_unlock(gcpufreq_mutex);
}
return gcpufreq_count;
diff --git a/src/gallium/auxiliary/hud/hud_diskstat.c b/src/gallium/auxiliary/hud/hud_diskstat.c
index b248baf..e1748c5 100644
--- a/src/gallium/auxiliary/hud/hud_diskstat.c
+++ b/src/gallium/auxiliary/hud/hud_diskstat.c
@@ -35,7 +35,9 @@
#include "hud/hud_private.h"
#include "util/list.h"
#include "os/os_time.h"
+#include "os/os_thread.h"
#include "util/u_memory.h"
+#include "util/u_inlines.h"
#include <stdio.h>
#include <unistd.h>
#include <dirent.h>
@@ -73,6 +75,7 @@ struct diskstat_info
char sysfs_filename[128];
uint64_t last_time;
struct stat_s last_stat;
+ struct pipe_reference refcnt;
};
/* TODO: We don't handle dynamic block device / partition
@@ -81,17 +84,23 @@ struct diskstat_info
*/
static int gdiskstat_count = 0;
static struct list_head gdiskstat_list;
+static pipe_mutex gdiskstat_mutex;
static struct diskstat_info *
find_dsi_by_name(const char *n, int mode)
{
+ struct diskstat_info *ret = 0;
+ pipe_mutex_lock(gdiskstat_mutex);
list_for_each_entry(struct diskstat_info, dsi, &gdiskstat_list, list) {
if (dsi->mode != mode)
continue;
- if (strcasecmp(dsi->name, n) == 0)
- return dsi;
+ if (strcasecmp(dsi->name, n) == 0) {
+ ret = dsi;
+ break;
+ }
}
- return 0;
+ pipe_mutex_unlock(gdiskstat_mutex);
+ return ret;
}
static int
@@ -165,9 +174,16 @@ query_dsi_load(struct hud_graph *gr)
static void
free_query_data(void *p)
{
- struct diskstat_info *nic = (struct diskstat_info *) p;
- list_del(&nic->list);
- FREE(nic);
+ struct diskstat_info *dsi = (struct diskstat_info *) p;
+
+ /* atomic dec */
+ if (pipe_reference(&dsi->refcnt, NULL)) {
+ pipe_mutex_lock(gdiskstat_mutex);
+ list_del(&dsi->list);
+ gdiskstat_count--;
+ pipe_mutex_unlock(gdiskstat_mutex);
+ FREE(dsi);
+ }
}
/**
@@ -205,6 +221,7 @@ hud_diskstat_graph_install(struct hud_pane *pane, const char *dev_name,
else
return;
+ pipe_reference(NULL, &dsi->refcnt); /* atomic inc */
gr->query_data = dsi;
gr->query_new_value = query_dsi_load;
@@ -226,8 +243,12 @@ add_object_part(const char *basename, const char *name, int objmode)
snprintf(dsi->sysfs_filename, sizeof(dsi->sysfs_filename), "%s/%s/stat",
basename, name);
dsi->mode = objmode;
+ pipe_reference_init(&dsi->refcnt, 1);
+
+ pipe_mutex_lock(gdiskstat_mutex);
list_addtail(&dsi->list, &gdiskstat_list);
gdiskstat_count++;
+ pipe_mutex_unlock(gdiskstat_mutex);
}
static void
@@ -239,8 +260,11 @@ add_object(const char *basename, const char *name, int objmode)
snprintf(dsi->sysfs_filename, sizeof(dsi->sysfs_filename), "%s/stat",
basename);
dsi->mode = objmode;
+
+ pipe_mutex_lock(gdiskstat_mutex);
list_addtail(&dsi->list, &gdiskstat_list);
gdiskstat_count++;
+ pipe_mutex_unlock(gdiskstat_mutex);
}
/**
@@ -263,6 +287,7 @@ hud_get_num_disks(bool displayhelp)
/* Scan /sys/block, for every object type we support, create and
* persist an object to represent its different statistics.
*/
+ pipe_mutex_init(gdiskstat_mutex);
list_inithead(&gdiskstat_list);
DIR *dir = opendir("/sys/block/");
if (!dir)
@@ -313,6 +338,7 @@ hud_get_num_disks(bool displayhelp)
}
if (displayhelp) {
+ pipe_mutex_lock(gdiskstat_mutex);
list_for_each_entry(struct diskstat_info, dsi, &gdiskstat_list, list) {
char line[32];
snprintf(line, sizeof(line), " diskstat-%s-%s",
@@ -321,6 +347,7 @@ hud_get_num_disks(bool displayhelp)
puts(line);
}
+ pipe_mutex_unlock(gdiskstat_mutex);
}
return gdiskstat_count;
diff --git a/src/gallium/auxiliary/hud/hud_nic.c b/src/gallium/auxiliary/hud/hud_nic.c
index fb6b8c0..7337164 100644
--- a/src/gallium/auxiliary/hud/hud_nic.c
+++ b/src/gallium/auxiliary/hud/hud_nic.c
@@ -36,6 +36,8 @@
#include "util/list.h"
#include "os/os_time.h"
#include "util/u_memory.h"
+#include "util/u_inlines.h"
+#include "os/os_thread.h"
#include <stdio.h>
#include <unistd.h>
#include <dirent.h>
@@ -59,6 +61,7 @@ struct nic_info
char throughput_filename[128];
uint64_t last_time;
uint64_t last_nic_bytes;
+ struct pipe_reference refcnt;
};
/* TODO: We don't handle dynamic NIC arrival or removal.
@@ -66,18 +69,23 @@ struct nic_info
*/
static int gnic_count = 0;
static struct list_head gnic_list;
+static pipe_mutex gnic_mutex;
static struct nic_info *
find_nic_by_name(const char *n, int mode)
{
+ struct nic_info *ret = 0;
+ pipe_mutex_lock(gnic_mutex);
list_for_each_entry(struct nic_info, nic, &gnic_list, list) {
if (nic->mode != mode)
continue;
-
- if (strcasecmp(nic->name, n) == 0)
- return nic;
+ if (strcasecmp(nic->name, n) == 0) {
+ ret = nic;
+ break;
+ }
}
- return 0;
+ pipe_mutex_unlock(gnic_mutex);
+ return ret;
}
static int
@@ -238,8 +246,14 @@ static void
free_query_data(void *p)
{
struct nic_info *nic = (struct nic_info *) p;
- list_del(&nic->list);
- FREE(nic);
+ /* atomic dec */
+ if (pipe_reference(&nic->refcnt, NULL)) {
+ pipe_mutex_lock(gnic_mutex);
+ list_del(&nic->list);
+ gnic_count--;
+ pipe_mutex_unlock(gnic_mutex);
+ FREE(nic);
+ }
}
/**
@@ -281,6 +295,7 @@ hud_nic_graph_install(struct hud_pane *pane, const char *nic_name,
else
return;
+ pipe_reference(NULL, &nic->refcnt); /* atomic inc */
gr->query_data = nic;
gr->query_new_value = query_nic_load;
@@ -348,6 +363,7 @@ hud_get_num_nics(bool displayhelp)
/* Scan /sys/block, for every object type we support, create and
* persist an object to represent its different statistics.
*/
+ pipe_mutex_init(gnic_mutex);
list_inithead(&gnic_list);
DIR *dir = opendir("/sys/class/net/");
if (!dir)
@@ -372,6 +388,7 @@ hud_get_num_nics(bool displayhelp)
/* Add the RX object */
nic = CALLOC_STRUCT(nic_info);
+ pipe_reference_init(&nic->refcnt, 1);
strcpy(nic->name, dp->d_name);
snprintf(nic->throughput_filename, sizeof(nic->throughput_filename),
"%s/statistics/rx_bytes", basename);
@@ -379,8 +396,10 @@ hud_get_num_nics(bool displayhelp)
nic->is_wireless = is_wireless;
query_nic_bitrate(nic, basename);
+ pipe_mutex_lock(gnic_mutex);
list_addtail(&nic->list, &gnic_list);
gnic_count++;
+ pipe_mutex_unlock(gnic_mutex);
/* Add the TX object */
nic = CALLOC_STRUCT(nic_info);
@@ -393,8 +412,10 @@ hud_get_num_nics(bool displayhelp)
query_nic_bitrate(nic, basename);
+ pipe_mutex_lock(gnic_mutex);
list_addtail(&nic->list, &gnic_list);
gnic_count++;
+ pipe_mutex_unlock(gnic_mutex);
if (nic->is_wireless) {
/* RSSI Support */
@@ -407,12 +428,15 @@ hud_get_num_nics(bool displayhelp)
query_nic_bitrate(nic, basename);
+ pipe_mutex_lock(gnic_mutex);
list_addtail(&nic->list, &gnic_list);
gnic_count++;
+ pipe_mutex_unlock(gnic_mutex);
}
}
+ pipe_mutex_lock(gnic_mutex);
list_for_each_entry(struct nic_info, nic, &gnic_list, list) {
char line[64];
snprintf(line, sizeof(line), " nic-%s-%s",
@@ -423,6 +447,7 @@ hud_get_num_nics(bool displayhelp)
puts(line);
}
+ pipe_mutex_unlock(gnic_mutex);
return gnic_count;
}
diff --git a/src/gallium/auxiliary/hud/hud_sensors_temp.c b/src/gallium/auxiliary/hud/hud_sensors_temp.c
index e41b847..4d19c61 100644
--- a/src/gallium/auxiliary/hud/hud_sensors_temp.c
+++ b/src/gallium/auxiliary/hud/hud_sensors_temp.c
@@ -33,6 +33,8 @@
#include "util/list.h"
#include "os/os_time.h"
#include "util/u_memory.h"
+#include "util/u_inlines.h"
+#include "os/os_thread.h"
#include <stdio.h>
#include <unistd.h>
#include <dirent.h>
@@ -49,6 +51,7 @@
*/
static int gsensors_temp_count = 0;
static struct list_head gsensors_temp_list;
+static pipe_mutex gsensors_temp_mutex;
struct sensors_temp_info
{
@@ -68,6 +71,7 @@ struct sensors_temp_info
sensors_chip_name *chip;
const sensors_feature *feature;
double current, min, max, critical;
+ struct pipe_reference refcnt;
};
static double
@@ -142,13 +146,18 @@ get_sensor_values(struct sensors_temp_info *sti)
static struct sensors_temp_info *
find_sti_by_name(const char *n, unsigned int mode)
{
+ struct sensors_temp_info *ret = 0;
+ pipe_mutex_lock(gsensors_temp_mutex);
list_for_each_entry(struct sensors_temp_info, sti, &gsensors_temp_list, list) {
if (sti->mode != mode)
continue;
- if (strcasecmp(sti->name, n) == 0)
- return sti;
+ if (strcasecmp(sti->name, n) == 0) {
+ ret = sti;
+ break;
+ }
}
- return 0;
+ pipe_mutex_unlock(gsensors_temp_mutex);
+ return ret;
}
static void
@@ -193,11 +202,20 @@ static void
free_query_data(void *p)
{
struct sensors_temp_info *sti = (struct sensors_temp_info *) p;
- list_del(&sti->list);
- if (sti->chip)
- sensors_free_chip_name(sti->chip);
- FREE(sti);
- sensors_cleanup();
+ /* atomic dec */
+ if (pipe_reference(&sti->refcnt, NULL)) {
+ pipe_mutex_lock(gsensors_temp_mutex);
+ list_del(&sti->list);
+ pipe_mutex_unlock(gsensors_temp_mutex);
+
+ if (sti->chip)
+ sensors_free_chip_name(sti->chip);
+
+ FREE(sti);
+ }
+
+ if (gsensors_temp_count == 0)
+ sensors_cleanup();
}
/**
@@ -234,6 +252,7 @@ hud_sensors_temp_graph_install(struct hud_pane *pane, const char *dev_name,
sti->mode == SENSORS_POWER_CURRENT ? "Pow" :
sti->mode == SENSORS_TEMP_CRITICAL ? "Crit" : "Unkn");
+ pipe_reference(NULL, &sti->refcnt); /* atomic inc */
gr->query_data = sti;
gr->query_new_value = query_sti_load;
@@ -267,6 +286,7 @@ create_object(const char *chipname, const char *featurename,
{
struct sensors_temp_info *sti = CALLOC_STRUCT(sensors_temp_info);
+ pipe_reference_init(&sti->refcnt, 1);
sti->mode = mode;
sti->chip = (sensors_chip_name *) chip;
sti->feature = feature;
@@ -275,8 +295,10 @@ create_object(const char *chipname, const char *featurename,
snprintf(sti->name, sizeof(sti->name), "%s.%s", sti->chipname,
sti->featurename);
+ pipe_mutex_lock(gsensors_temp_mutex);
list_addtail(&sti->list, &gsensors_temp_list);
gsensors_temp_count++;
+ pipe_mutex_unlock(gsensors_temp_mutex);
}
static void
@@ -345,6 +367,7 @@ hud_get_num_sensors(bool displayhelp)
if (ret)
return 0;
+ pipe_mutex_init(gsensors_temp_mutex);
list_inithead(&gsensors_temp_list);
/* Scan /sys/block, for every object type we support, create and
@@ -353,6 +376,7 @@ hud_get_num_sensors(bool displayhelp)
build_sensor_list();
if (displayhelp) {
+ pipe_mutex_lock(gsensors_temp_mutex);
list_for_each_entry(struct sensors_temp_info, sti, &gsensors_temp_list, list) {
char line[64];
switch (sti->mode) {
@@ -375,6 +399,7 @@ hud_get_num_sensors(bool displayhelp)
puts(line);
}
+ pipe_mutex_unlock(gsensors_temp_mutex);
}
return gsensors_temp_count;
--
2.7.4
More information about the mesa-dev
mailing list