[PATCH 1/1] Fix bugs determining active VT consoles
Joe Marcus Clarke
marcus at FreeBSD.org
Thu Aug 30 14:56:40 PDT 2007
On FreeBSD, having a process sit in VT_WAITACTIVE can cause kernel panics or
unkillable processes that consume 100% of the CPU. To work around this
problem, replace the model where individual threads call VT_WAITACTIVE
with a g_timeout model which checks the active VT every second, and queues
an event when it changes.
---
src/ck-sysdeps-freebsd.c | 5 --
src/ck-vt-monitor.c | 123 +++++++++++-----------------------------------
2 files changed, 29 insertions(+), 99 deletions(-)
diff --git a/src/ck-sysdeps-freebsd.c b/src/ck-sysdeps-freebsd.c
index 1fbdf7d..d6add91 100644
--- a/src/ck-sysdeps-freebsd.c
+++ b/src/ck-sysdeps-freebsd.c
@@ -335,11 +335,6 @@ ck_get_max_num_consoles (guint *num)
max_consoles++;
}
- /* Increment one more so that all consoles are properly counted
- * this is arguable a bug in vt_add_watches().
- */
- max_consoles++;
-
ret = TRUE;
endttyent ();
diff --git a/src/ck-vt-monitor.c b/src/ck-vt-monitor.c
index 59d3d1c..4e7fdb0 100644
--- a/src/ck-vt-monitor.c
+++ b/src/ck-vt-monitor.c
@@ -49,7 +49,7 @@
struct CkVtMonitorPrivate
{
int vfd;
- GHashTable *vt_thread_hash;
+ int timeout_id;
guint active_num;
GAsyncQueue *event_queue;
@@ -75,7 +75,6 @@ static void vt_add_watches (CkVtMonitor *vt_monitor);
G_DEFINE_TYPE (CkVtMonitor, ck_vt_monitor, G_TYPE_OBJECT)
-G_LOCK_DEFINE_STATIC (hash_lock);
G_LOCK_DEFINE_STATIC (schedule_lock);
static gpointer vt_object = NULL;
@@ -167,9 +166,6 @@ change_active_num (CkVtMonitor *vt_monitor,
vt_monitor->priv->active_num = num;
- /* add a watch to every vt without a thread */
- vt_add_watches (vt_monitor);
-
g_signal_emit (vt_monitor, signals[ACTIVE_CHANGED], 0, num);
} else {
g_debug ("VT activated but already active: %d", num);
@@ -186,16 +182,6 @@ typedef struct {
} EventData;
static void
-thread_data_free (ThreadData *data)
-{
- if (data == NULL) {
- return;
- }
-
- g_free (data);
-}
-
-static void
event_data_free (EventData *data)
{
if (data == NULL) {
@@ -220,8 +206,6 @@ process_queue (CkVtMonitor *vt_monitor)
queue_length = g_async_queue_length_unlocked (vt_monitor->priv->event_queue);
data = NULL;
- G_LOCK (hash_lock);
-
/* compress events in the queue */
for (i = 0; i < queue_length; i++) {
d = g_async_queue_try_pop_unlocked (vt_monitor->priv->event_queue);
@@ -238,8 +222,6 @@ process_queue (CkVtMonitor *vt_monitor)
data = d;
}
- G_UNLOCK (hash_lock);
-
if (data != NULL) {
change_active_num (vt_monitor, data->num);
event_data_free (data);
@@ -264,27 +246,28 @@ schedule_process_queue (CkVtMonitor *vt_monitor)
G_UNLOCK (schedule_lock);
}
-static void *
-vt_thread_start (ThreadData *data)
+static gboolean
+check_active_vt (CkVtMonitor *vt_monitor)
{
- CkVtMonitor *vt_monitor;
- gboolean res;
- int ret;
- gint32 num;
+ int last_active;
+ int active;
+ gboolean res;
- vt_monitor = data->vt_monitor;
- num = data->num;
+ last_active = vt_monitor->priv->active_num;
- res = ck_wait_for_active_console_num (vt_monitor->priv->vfd, num);
+ res = ck_get_active_console_num (vt_monitor->priv->vfd, &active);
if (! res) {
- /* FIXME: what do we do if it fails? */
- } else {
+ /* FIXME handle failure (maybe return FALSE?) */
+ g_warning ("check_active_vt: could not determine active VT");
+ active = last_active;
+ }
+
+ if (active != last_active) {
EventData *event;
- /* add event to queue */
event = g_new0 (EventData, 1);
- event->num = num;
- g_debug ("Pushing activation event for VT %d onto queue", num);
+ event->num = active;
+ g_debug ("Pushing activation event for VT %d onto queue", active);
g_async_queue_push (vt_monitor->priv->event_queue, event);
@@ -292,55 +275,13 @@ vt_thread_start (ThreadData *data)
schedule_process_queue (vt_monitor);
}
- G_LOCK (hash_lock);
- if (vt_monitor->priv->vt_thread_hash != NULL) {
- g_hash_table_remove (vt_monitor->priv->vt_thread_hash, GUINT_TO_POINTER (num));
- }
- G_UNLOCK (hash_lock);
-
- g_thread_exit (NULL);
- thread_data_free (data);
-
- return NULL;
-}
-
-static void
-vt_add_watch_unlocked (CkVtMonitor *vt_monitor,
- gint32 num)
-{
- GThread *thread;
- GError *error;
- ThreadData *data;
- gpointer id;
-
- data = g_new0 (ThreadData, 1);
- data->num = num;
- data->vt_monitor = vt_monitor;
-
- g_debug ("Creating thread for vt %d", num);
-
- id = GINT_TO_POINTER (num);
-
- error = NULL;
- thread = g_thread_create_full ((GThreadFunc)vt_thread_start, data, 65536, FALSE, TRUE, G_THREAD_PRIORITY_NORMAL, &error);
- if (thread == NULL) {
- g_debug ("Unable to create thread: %s", error->message);
- g_error_free (error);
- } else {
- g_hash_table_insert (vt_monitor->priv->vt_thread_hash, id, thread);
- }
+ return TRUE;
}
static void
vt_add_watches (CkVtMonitor *vt_monitor)
{
guint max_consoles;
- int i;
- gint32 current_num;
-
- G_LOCK (hash_lock);
-
- current_num = vt_monitor->priv->active_num;
max_consoles = 1;
@@ -348,23 +289,18 @@ vt_add_watches (CkVtMonitor *vt_monitor)
/* FIXME: this can fail on solaris and freebsd */
}
- for (i = 1; i < max_consoles; i++) {
- gpointer id;
-
- /* don't wait on the active vc */
- if (i == current_num) {
- continue;
- }
-
- id = GINT_TO_POINTER (i);
-
- /* add a watch to all other VTs that don't have threads */
- if (g_hash_table_lookup (vt_monitor->priv->vt_thread_hash, id) == NULL) {
- vt_add_watch_unlocked (vt_monitor, i);
- }
+ if (max_consoles > 1) {
+#if GLIB_CHECK_VERSION(2,14,0)
+ vt_monitor->priv->timeout_id = g_timeout_add_seconds (1,
+ (GSourceFunc) check_active_vt,
+ vt_monitor);
+#else
+ vt_monitor->priv->timeout_id = g_timeout_add (1000,
+ (GSourceFunc) check_active_vt,
+ vt_monitor);
+#endif
}
- G_UNLOCK (hash_lock);
}
static void
@@ -414,7 +350,6 @@ ck_vt_monitor_init (CkVtMonitor *vt_monitor)
vt_monitor->priv->active_num = active;
vt_monitor->priv->event_queue = g_async_queue_new ();
- vt_monitor->priv->vt_thread_hash = g_hash_table_new (g_direct_hash, g_direct_equal);
vt_add_watches (vt_monitor);
}
@@ -440,8 +375,8 @@ ck_vt_monitor_finalize (GObject *object)
g_async_queue_unref (vt_monitor->priv->event_queue);
}
- if (vt_monitor->priv->vt_thread_hash != NULL) {
- g_hash_table_destroy (vt_monitor->priv->vt_thread_hash);
+ if (vt_monitor->priv->timeout_id > 0) {
+ g_source_remove (vt_monitor->priv->timeout_id);
}
if (vt_monitor->priv->vfd != ERROR) {
--
1.5.2.1
More information about the hal
mailing list