hal: Branch 'master'
David Zeuthen
david at kemper.freedesktop.org
Tue Sep 12 00:34:44 PDT 2006
hald/hald_dbus.c | 48 +++++++++++++++++++++++++++++++++++++-----------
hald/hald_dbus.h | 2 ++
hald/linux/blockdev.c | 49 +++++++++++++++++++++++++++++++------------------
3 files changed, 70 insertions(+), 29 deletions(-)
New commits:
diff-tree e35d886bf811baa0c15e9d71a6296f37ef403dde (from 5cd39bf7de4d3b6b4d317e7ea6d49fbac041aaa6)
Author: David Zeuthen <davidz at redhat.com>
Date: Tue Sep 12 03:30:44 2006 -0400
fix race condition when unmounting a volume takes a very long time
Whilst trying to fix this bug
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=194296
I discovered that there's a race condition when unmounting a volume
that has lots of outstanding writes. Under Linux, at least, this
results in umount(8) taking very long to complete. However, the
kernel tells us, just after the umount syscall I believe, that
/proc/mounts has changed. So we check that file to see if it was
us, HAL, that unmounted the volume. Because if it wasn't, we want
to clean up the mount point if we created it.
To do this, we first lock /media/.hal-mtab to ensure noone else is
modifying it. But this will block until Unmount() terminates (because
Unmount() is holding the lock on that file) and that can take many
many seconds, e.g. 30 secs for example, if you got a bunch of
outstanding writes.
The fix here is to check whether we're already executing Unmount()
on the device in question. If we are, then, hey, we can infer
that HAL is unmounting that volume, not someone else.
diff --git a/hald/hald_dbus.c b/hald/hald_dbus.c
index b45cf29..9a865d0 100644
--- a/hald/hald_dbus.c
+++ b/hald/hald_dbus.c
@@ -2829,6 +2829,7 @@ typedef struct {
char *execpath;
char **extra_env;
char *mstdin;
+ char *member;
DBusMessage *message;
DBusConnection *connection;
} MethodInvocation;
@@ -2842,10 +2843,12 @@ hald_exec_method_cb (HalDevice *d, guint
static void
hald_exec_method_free_mi (MethodInvocation *mi)
{
+ /* hald_runner_run_method() assumes ownership of mi->message.. so we don't free it here */
g_free (mi->udi);
g_free (mi->execpath);
g_strfreev (mi->extra_env);
g_free (mi->mstdin);
+ g_free (mi->member);
g_free (mi);
}
@@ -2887,6 +2890,31 @@ hald_exec_method_do_invocation (MethodIn
static GHashTable *udi_to_method_queue = NULL;
+
+gboolean
+device_is_executing_method (HalDevice *d, const char *method_name)
+{
+ gpointer origkey;
+ gboolean ret;
+ GList *queue;
+
+ ret = FALSE;
+
+ if (g_hash_table_lookup_extended (udi_to_method_queue, d->udi, &origkey, (gpointer) &queue)) {
+
+ if (queue != NULL) {
+ MethodInvocation *mi;
+ mi = (MethodInvocation *) queue->data;
+ if (strcmp (mi->member, method_name) == 0) {
+ ret = TRUE;
+ }
+ }
+
+ ret = TRUE;
+ }
+ return ret;
+}
+
static void
hald_exec_method_process_queue (const char *udi);
@@ -2911,7 +2939,6 @@ hald_exec_method_enqueue (MethodInvocati
g_hash_table_insert (udi_to_method_queue, g_strdup (mi->udi), queue);
hald_exec_method_do_invocation (mi);
- hald_exec_method_free_mi (mi);
}
}
@@ -2921,30 +2948,30 @@ hald_exec_method_process_queue (const ch
{
gpointer origkey;
GList *queue;
+ MethodInvocation *mi;
if (g_hash_table_lookup_extended (udi_to_method_queue, udi, &origkey, (gpointer) &queue)) {
+
+ /* clean the top of the list */
if (queue != NULL) {
+ mi = (MethodInvocation *) queue->data;
+ hald_exec_method_free_mi (mi);
queue = g_list_delete_link (queue, queue);
}
+ /* process the rest of the list */
if (queue == NULL) {
HAL_INFO (("No more methods in queue"));
g_hash_table_remove (udi_to_method_queue, udi);
} else {
- MethodInvocation *mi;
-
HAL_INFO (("Execing next method in queue"));
g_hash_table_replace (udi_to_method_queue, g_strdup (udi), queue);
mi = (MethodInvocation *) queue->data;
-
if (!hald_exec_method_do_invocation (mi)) {
/* the device went away before we got to it... */
hald_exec_method_process_queue (mi->udi);
}
-
- hald_exec_method_free_mi (mi);
-
}
}
}
@@ -3173,6 +3200,7 @@ hald_exec_method (HalDevice *d, DBusConn
mi->mstdin = g_strdup (stdin_str->str);
mi->message = message;
mi->connection = connection;
+ mi->member = g_strdup (dbus_message_get_member (message));
hald_exec_method_enqueue (mi);
dbus_message_ref (message);
@@ -3566,14 +3594,12 @@ static DBusHandlerResult
hald_dbus_filter_handle_methods (DBusConnection *connection, DBusMessage *message,
void *user_data, dbus_bool_t local_interface)
{
- /*
- HAL_INFO (("connection=0x%x obj_path=%s interface=%s method=%s local_interface=%d",
+ /*HAL_INFO (("connection=0x%x obj_path=%s interface=%s method=%s local_interface=%d",
connection,
dbus_message_get_path (message),
dbus_message_get_interface (message),
dbus_message_get_member (message),
- local_interface));
- */
+ local_interface));*/
if (dbus_message_is_method_call (message,
"org.freedesktop.Hal.Manager",
diff --git a/hald/hald_dbus.h b/hald/hald_dbus.h
index 96fc987..5fe7307 100644
--- a/hald/hald_dbus.h
+++ b/hald/hald_dbus.h
@@ -95,4 +95,6 @@ DBusHandlerResult hald_dbus_filter_funct
char *hald_dbus_local_server_addr (void);
+gboolean device_is_executing_method (HalDevice *d, const char *method_name);
+
#endif /* HAL_DBUS_H */
diff --git a/hald/linux/blockdev.c b/hald/linux/blockdev.c
index 3061424..cc21ddf 100644
--- a/hald/linux/blockdev.c
+++ b/hald/linux/blockdev.c
@@ -281,24 +281,37 @@ blockdev_refresh_mount_state (HalDevice
autofs_node = autofs_node->next;
}
- /* look up in /media/.hal-mtab to see if we mounted this one */
- if (mount_point != NULL && strlen (mount_point) > 0 && hal_util_is_mounted_by_hald (mount_point)) {
- char *cleanup_stdin;
- char *extra_env[2];
-
- HAL_INFO (("Cleaning up directory '%s' since it was created by HAL's Mount()", mount_point));
-
- extra_env[0] = g_strdup_printf ("HALD_CLEANUP=%s", mount_point);
- extra_env[1] = NULL;
- cleanup_stdin = "\n";
-
- hald_runner_run_method (dev,
- "hal-storage-cleanup-mountpoint",
- extra_env,
- cleanup_stdin, TRUE,
- 0,
- cleanup_mountpoint_cb,
- g_strdup (mount_point), NULL);
+ /* look up in /media/.hal-mtab to see if we mounted this one - unless there is a
+ * Unmount() method on the device already being processed. Because, if there is,
+ * the Unmount() method will hold the lock /media/.hal-mtab and then the function
+ * hal_util_is_mounted_by_hald() will block until Unmount() returns.
+ *
+ * And this is a problem because on Linux /proc/mounts is changed immediately, e.g.
+ * we get to here but umount(8) don't return until much later. This is normally
+ * not a problem, it only surfaces under circumstances described in
+ * https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=194296, e.g. when there
+ * is a lot of data waiting to be written.
+ */
+ if (!device_is_executing_method (dev, "Unmount")) {
+ if (mount_point != NULL && strlen (mount_point) > 0 &&
+ hal_util_is_mounted_by_hald (mount_point)) {
+ char *cleanup_stdin;
+ char *extra_env[2];
+
+ HAL_INFO (("Cleaning up directory '%s' since it was created by HAL's Mount()", mount_point));
+
+ extra_env[0] = g_strdup_printf ("HALD_CLEANUP=%s", mount_point);
+ extra_env[1] = NULL;
+ cleanup_stdin = "\n";
+
+ hald_runner_run_method (dev,
+ "hal-storage-cleanup-mountpoint",
+ extra_env,
+ cleanup_stdin, TRUE,
+ 0,
+ cleanup_mountpoint_cb,
+ g_strdup (mount_point), NULL);
+ }
}
g_free (mount_point);
More information about the hal-commit
mailing list