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-devel

[Xen-devel] Re: [PATCH] Mini-OS events handling cleanup

To: Horms <horms@xxxxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] Mini-OS events handling cleanup
From: Grzegorz Milos <gm281@xxxxxxxxx>
Date: Wed, 12 Jul 2006 08:24:28 +0100
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, Steven Smith <sos22@xxxxxxxxx>
Delivery-date: Wed, 12 Jul 2006 00:24:57 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <20060711123017.AEAD834041@xxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20060711123017.AEAD834041@xxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Hi Grzegorz,

I've made a few coding-style comments inline.


Steven already answerd the most difficult questions, here is the reminder:

--Apple-Mail-3--158434814
Content-Transfer-Encoding: 7bit
Content-Type: application/octet-stream; x-unix-mode=0644;
        name="minios.events.patch"
Content-Disposition: attachment;
        filename=minios.events.patch

# HG changeset patch
# User gm281@xxxxxxxxxxxxxxxxxx
# Node ID ef01229d6344d7ff1c8970512fdb812445ec3430
# Parent  c3d6610fc0f0510a09bd3d6c63a4e8a3f78312cc
Mini-OS: Events handling cleaned up. The interface extended to provide void* pointer to handlers.

Signed-off-by: Steven Smith <sos22@xxxxxxxxx>
Signed-off-by: Grzegorz Milos <gm281@xxxxxxxxx>

diff -r c3d6610fc0f0 -r ef01229d6344 extras/mini-os/console/ xencons_ring.c
--- a/extras/mini-os/console/xencons_ring.c     Wed Jul  5 10:06:14 2006
+++ b/extras/mini-os/console/xencons_ring.c     Wed Jul  5 10:20:15 2006
@@ -53,7 +53,7 @@



-static void handle_input(int port, struct pt_regs *regs)
+static void handle_input(int port, struct pt_regs *regs, void *ign)
{
        struct xencons_interface *intf = xencons_interface();
        XENCONS_RING_IDX cons, prod;
@@ -83,7 +83,8 @@
        if (!start_info.console_evtchn)
                return 0;

-       err = bind_evtchn(start_info.console_evtchn, handle_input);
+       err = bind_evtchn(start_info.console_evtchn, handle_input,
+                         NULL);

Could you move the NULL ont the same line as bind_evtchn( ?


Would the line still be shorter then 80 characters? (that was the reason to put NULL on the next line).


        if (err <= 0) {
                printk("XEN console request chn bind failed %i\n", err);
                return err;
diff -r c3d6610fc0f0 -r ef01229d6344 extras/mini-os/events.c
--- a/extras/mini-os/events.c   Wed Jul  5 10:06:14 2006
+++ b/extras/mini-os/events.c   Wed Jul  5 10:20:15 2006
@@ -22,9 +22,18 @@
#include <events.h>
#include <lib.h>

+#define NR_EVS 1024
+
+/* this represents a event handler. Chaining or sharing is not allowed */
+typedef struct _ev_action_t {
+       void (*handler)(int, struct pt_regs *, void *);
+       void *data;
+    u32 count;
+} ev_action_t;
+

Tabs and spaces are used inconsistently in the above structure definition.

Ok - will change that.


static ev_action_t ev_actions[NR_EVS];
-void default_handler(int port, struct pt_regs *regs);
+void default_handler(int port, struct pt_regs *regs, void *data);


/*
@@ -35,42 +44,33 @@
    ev_action_t  *action;
    if (port >= NR_EVS) {
        printk("Port number too large: %d\n", port);
-        goto out;
+               goto out;
    }

    action = &ev_actions[port];
    action->count++;

-    if (!action->handler)
-    {
-        printk("Spurious event on port %d\n", port);
-        goto out;
-    }
-
-    if (action->status & EVS_DISABLED)
-    {
-        printk("Event on port %d disabled\n", port);
-        goto out;
-    }
-
    /* call the handler */
-    action->handler(port, regs);
-
+       action->handler(port, regs, action->data);
+
 out:
        clear_evtchn(port);
+
    return 1;

}

-int bind_evtchn( u32 port, void (*handler)(int, struct pt_regs *) )
+int bind_evtchn( u32 port, void (*handler)(int, struct pt_regs *, void *),
+                                void *data )
{
        if(ev_actions[port].handler != default_handler)
printk("WARN: Handler for port %d already registered, replacing\n",
                                port);

+       ev_actions[port].data = data;
+       wmb();
        ev_actions[port].handler = handler;
-       ev_actions[port].status &= ~EVS_DISABLED;   
-
+
        /* Finally unmask the port */
        unmask_evtchn(port);

@@ -82,13 +82,14 @@
        if (ev_actions[port].handler == default_handler)
                printk("WARN: No handler for port %d when unbinding\n", port);
        ev_actions[port].handler = default_handler;
-       ev_actions[port].status |= EVS_DISABLED;
+       wmb();
+       ev_actions[port].data = NULL;
}

-static void xenbus_evtchn_handler(int port, struct pt_regs *regs)
+static void xenbus_evtchn_handler(int port, struct pt_regs *regs, void *ign)
{
    wake_up(&xb_waitq);
}
@@ -174,7 +174,8 @@
    create_thread("xenstore", xenbus_thread_func, NULL);
    DEBUG("buf at %p.\n", xenstore_buf);
    err = bind_evtchn(start_info.store_evtchn,
-                     xenbus_evtchn_handler);
+                     xenbus_evtchn_handler,
+              NULL);

Again, could the NULL be appended to the previous line?

If the lines become too long to put more then one argument I generally separate each argument into a new line. This seems to be slightly more readable when you trying to find nth argument.

Cheers
Gregor

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