WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-changelog

[Xen-changelog] Remove ill-conceived concept of watches blocking reply o

To: xen-changelog@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-changelog] Remove ill-conceived concept of watches blocking reply on
From: Xen patchbot -unstable <patchbot-unstable@xxxxxxxxxxxxxxxxxxx>
Date: Tue, 26 Jul 2005 20:26:14 -0400
Delivery-date: Wed, 27 Jul 2005 00:27:18 +0000
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-changelog-request@lists.xensource.com?subject=help>
List-id: BK change log <xen-changelog.lists.xensource.com>
List-post: <mailto:xen-changelog@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=unsubscribe>
Reply-to: xen-devel@xxxxxxxxxxxxxxxxxxx
Sender: xen-changelog-bounces@xxxxxxxxxxxxxxxxxxx
# HG changeset patch
# User cl349@xxxxxxxxxxxxxxxxxxxx
# Node ID 1853a6e966bd808af19568ba3c0c05a4e69750c1
# Parent  b9903985e9b65969daabec9cada972e3b956da6c
Remove ill-conceived concept of watches blocking reply on
connection which did write/mkdir/rm/setperm etc.
This causes deadlocks in real life, and I can't see a sane way
of avoiding them: it is reasonable for someone to ignore watch
notifications while doing other actions, and that means that
we can do other writes.  These writes can block pending other
watchers; if one of these is the process blocked awaiting our
ack, we deadlock.
Signed-off-by: Rusty Russel <rusty@xxxxxxxxxxxxxxx>
Signed-off-by: Christian Limpach <Christian.Limpach@xxxxxxxxxxxx>

diff -r b9903985e9b6 -r 1853a6e966bd tools/xenstore/xenstored_core.c
--- a/tools/xenstore/xenstored_core.c   Tue Jul 26 15:24:28 2005
+++ b/tools/xenstore/xenstored_core.c   Tue Jul 26 15:26:32 2005
@@ -976,10 +976,7 @@
        }
 
        add_change_node(conn->transaction, node, false);
-       if (fire_watches(conn, node, false)) {
-               conn->watch_ack = XS_WRITE;
-               return;
-       }
+       fire_watches(conn, node, false);
        send_ack(conn, XS_WRITE);
 }
 
@@ -1005,10 +1002,7 @@
        }
 
        add_change_node(conn->transaction, node, false);
-       if (fire_watches(conn, node, false)) {
-               conn->watch_ack = XS_MKDIR;
-               return;
-       }
+       fire_watches(conn, node, false);
        send_ack(conn, XS_MKDIR);
 }
 
@@ -1046,10 +1040,7 @@
        }
 
        add_change_node(conn->transaction, node, true);
-       if (fire_watches(conn, node, true)) {
-               conn->watch_ack = XS_RM;
-               return;
-       }
+       fire_watches(conn, node, true);
        send_ack(conn, XS_RM);
 }
 
@@ -1121,10 +1112,7 @@
        }
 
        add_change_node(conn->transaction, node, false);
-       if (fire_watches(conn, node, false)) {
-               conn->watch_ack = XS_SET_PERMS;
-               return;
-       }
+       fire_watches(conn, node, false);
        send_ack(conn, XS_SET_PERMS);
 }
 
@@ -1359,12 +1347,6 @@
                                consider_message(i);
                        }
                        break;
-               case WATCHED:
-                       if (i->watches_unacked == 0) {
-                               i->state = OK;
-                               send_ack(i, i->watch_ack);
-                       }
-                       break;
                case OK:
                        break;
                }
@@ -1389,8 +1371,6 @@
 
        new->state = OK;
        new->blocked_by = NULL;
-       new->watch_ack = XS_ERROR;
-       new->watches_unacked = 0;
        new->out = new->waiting_reply = NULL;
        new->fd = -1;
        new->id = 0;
@@ -1471,7 +1451,6 @@
                printf("    state = %s\n",
                       i->state == OK ? "OK"
                       : i->state == BLOCKED ? "BLOCKED"
-                      : i->state == WATCHED ? "WATCHED"
                       : "INVALID");
                if (i->id)
                        printf("    id = %i\n", i->id);
diff -r b9903985e9b6 -r 1853a6e966bd tools/xenstore/xenstored_core.h
--- a/tools/xenstore/xenstored_core.h   Tue Jul 26 15:24:28 2005
+++ b/tools/xenstore/xenstored_core.h   Tue Jul 26 15:26:32 2005
@@ -51,8 +51,6 @@
 {
        /* Blocked by transaction. */
        BLOCKED,
-       /* Waiting for watchers to ack event we caused */
-       WATCHED,
        /* Completed */
        OK,
 };
@@ -72,12 +70,6 @@
 
        /* Node we are waiting for (if state == BLOCKED) */
        char *blocked_by;
-
-       /* Are we waiting for watches to be acked from an event we caused? */
-       unsigned int watches_unacked;
-
-       /* Type of ack to send once watches fired. */
-       enum xsd_sockmsg_type watch_ack;
 
        /* Is this a read-only connection? */
        bool can_write;
diff -r b9903985e9b6 -r 1853a6e966bd tools/xenstore/xenstored_transaction.c
--- a/tools/xenstore/xenstored_transaction.c    Tue Jul 26 15:24:28 2005
+++ b/tools/xenstore/xenstored_transaction.c    Tue Jul 26 15:26:32 2005
@@ -307,7 +307,6 @@
 {
        struct changed_node *i;
        struct transaction *trans;
-       bool fired = false;
 
        if (!arg || (!streq(arg, "T") && !streq(arg, "F"))) {
                send_error(conn, EINVAL);
@@ -337,12 +336,8 @@
 
                /* Fire off the watches for everything that changed. */
                list_for_each_entry(i, &trans->changes, list)
-                       fired |= fire_watches(conn, i->node, i->recurse);
-       }
-
-       if (fired)
-               conn->watch_ack = XS_TRANSACTION_END;
-       else
-               send_ack(conn, XS_TRANSACTION_END);
-}
-
+                       fire_watches(conn, i->node, i->recurse);
+       }
+       send_ack(conn, XS_TRANSACTION_END);
+}
+
diff -r b9903985e9b6 -r 1853a6e966bd tools/xenstore/xenstored_watch.c
--- a/tools/xenstore/xenstored_watch.c  Tue Jul 26 15:24:28 2005
+++ b/tools/xenstore/xenstored_watch.c  Tue Jul 26 15:26:32 2005
@@ -41,9 +41,6 @@
        /* Data to send (node\0token\0). */
        unsigned int len;
        char *data;
-
-       /* Connection which caused watch event (which we are blocking) */
-       struct connection *cause;
 };
 
 struct watch
@@ -95,14 +92,10 @@
        struct watch_event *event = _event;
 
        trace_destroy(event, "watch_event");
-       assert(event->cause->watches_unacked != 0);
-       /* If it hits zero, will unblock in unblock_connections. */
-       event->cause->watches_unacked--;
        return 0;
 }
 
-static void add_event(struct connection *cause, struct watch *watch,
-                     const char *node)
+static void add_event(struct watch *watch, const char *node)
 {
        struct watch_event *event;
 
@@ -117,22 +110,20 @@
        event->data = talloc_array(event, char, event->len);
        strcpy(event->data, node);
        strcpy(event->data + strlen(node) + 1, watch->token);
-       event->cause = cause;
-       cause->watches_unacked++;
        talloc_set_destructor(event, destroy_watch_event);
        list_add_tail(&event->list, &watch->events);
        trace_create(event, "watch_event");
 }
 
 /* FIXME: we fail to fire on out of memory.  Should drop connections. */
-bool fire_watches(struct connection *conn, const char *node, bool recurse)
+void fire_watches(struct connection *conn, const char *node, bool recurse)
 {
        struct connection *i;
        struct watch *watch;
 
        /* During transactions, don't fire watches. */
        if (conn->transaction)
-               return false;
+               return;
 
        /* Create an event for each watch.  Don't send to self. */
        list_for_each_entry(i, &connections, list) {
@@ -141,18 +132,16 @@
 
                list_for_each_entry(watch, &i->watches, list) {
                        if (is_child(node, watch->node))
-                               add_event(conn, watch, node);
+                               add_event(watch, node);
                        else if (recurse && is_child(watch->node, node))
-                               add_event(conn, watch, watch->node);
+                               add_event(watch, watch->node);
                        else
                                continue;
-                       conn->state = WATCHED;
                        /* If connection not doing anything, queue this. */
                        if (!i->out)
                                queue_next_event(i);
                }
        }
-       return conn->state == WATCHED;
 }
 
 static int destroy_watch(void *_watch)
diff -r b9903985e9b6 -r 1853a6e966bd tools/xenstore/xenstored_watch.h
--- a/tools/xenstore/xenstored_watch.h  Tue Jul 26 15:24:28 2005
+++ b/tools/xenstore/xenstored_watch.h  Tue Jul 26 15:26:32 2005
@@ -33,9 +33,8 @@
 void queue_next_event(struct connection *conn);
 
 /* Fire all watches: recurse means all the children are effected (ie. rm).
- * Returns true if there were any, meaning connection has to wait.
  */
-bool fire_watches(struct connection *conn, const char *node, bool recurse);
+void fire_watches(struct connection *conn, const char *node, bool recurse);
 
 /* Find shortest timeout: if any, reduce tv (may already be set). */
 void shortest_watch_ack_timeout(struct timeval *tv);

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-changelog] Remove ill-conceived concept of watches blocking reply on, Xen patchbot -unstable <=