Re: On the road to Xen : LTTng and relay CPU hotplug support

Mathieu Desnoyers wrote:
> Hi,
> 
> In my current work of porting LTTng to the Xen hypervisor, I notice that their
> project use the Linux CPU hotplug feature extensively in the domain-0 and
> domain-Us. This brings me to a preliminary adaptation phase of the LTTng kernel
> tracer : adding CPU hotplug support.
> 
> The basic idea is that we have (at least) to create a new buffer for every
> channel when a CPU is "UP_PREPARE". In order to do that, Relay must be
> modified so the buffer creation function is less coupled with the channel
> creation and that this function is exported as part of the API.
> (I just implemented this modification) We can also destroy the buffer when the
> CPU goes down (see below).
> 
> Relay cannot, by itself, deal with the CPU hotplug events because it does not
> have a list of the allocated channels. This is why I choose to put the hotplug

Hi,

I think it would be useful to add a channel list to relay and do some of 
this there.  Here's a first cut patch that does it - see if it makes 
sense for this.  It adds new buffers when cpus come online, but only 
flushes them when cpus go offline ,i.e. doesn't destroy the offline buffers.

> 
> What I could do is to implement (A), keeping in mind that the proper way to do
> it would be (B), the longer term target therefore being (B). In the mindset of
> doing incremental changes that "works", I think (A) would be a good start.
> 
> Having your insight on this would be appreciated,
> 

I think this approach makes sense - since all the buffers will be 
destroyed anyway when the channel is destroyed, you could probably avoid 
the complications of plan B unless you find you really need to.

Tom


diff --git a/include/linux/relay.h b/include/linux/relay.h
index 24accb4..6969fd2 100644
--- a/include/linux/relay.h
+++ b/include/linux/relay.h
@@ -24,7 +24,7 @@
  /*
   * Tracks changes to rchan/rchan_buf structs
   */
-#define RELAYFS_CHANNEL_VERSION        6
+#define RELAYFS_CHANNEL_VERSION        7

  /*
   * Per-cpu relay channel buffer
@@ -64,6 +64,9 @@ struct rchan
      void *private_data;        /* for user-defined data */
      size_t last_toobig;        /* tried to log event > subbuf size */
      struct rchan_buf *buf[NR_CPUS]; /* per-cpu channel buffers */
+    struct list_head list;        /* for channel list */
+    struct dentry *parent;        /* parent dentry passed to open */
+    char base_filename[NAME_MAX];    /* saved base filename */
  };

  /*
diff --git a/kernel/relay.c b/kernel/relay.c
index f04bbdb..5e03f0f 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -18,6 +18,11 @@
  #include <linux/relay.h>
  #include <linux/vmalloc.h>
  #include <linux/mm.h>
+#include <linux/cpu.h>
+
+/* list of open channels, for cpu hotplug */
+static DEFINE_MUTEX(relay_channels_mutex);
+static LIST_HEAD(relay_channels);

  /*
   * close() vm_op implementation for relay file mapping.
@@ -169,6 +174,9 @@ free_buf:
  void relay_destroy_channel(struct kref *kref)
  {
      struct rchan *chan = container_of(kref, struct rchan, kref);
+    mutex_lock(&relay_channels_mutex);
+    list_del(&chan->list);
+    mutex_unlock(&relay_channels_mutex);
      kfree(chan);
  }

@@ -366,9 +374,9 @@ void relay_reset(struct rchan *chan)
      if (!chan)
          return;

-    for (i = 0; i < NR_CPUS; i++) {
+    for_each_possible_cpu(i) {
          if (!chan->buf[i] || chan->buf[i] == prev)
-            break;
+            continue;
          __relay_reset(chan->buf[i], 0);
          prev = chan->buf[i];
      }
@@ -446,6 +454,67 @@ static inline void setup_callbacks(struc
      chan->cb = cb;
  }

+static int relay_add_hotcpu(int hotcpu)
+{
+    struct rchan *chan;
+    int err = 0, is_global = 0;
+    char *tmpname = kmalloc(NAME_MAX + 1, GFP_KERNEL);
+    if (!tmpname)
+        return -ENOMEM;
+
+    mutex_lock(&relay_channels_mutex);
+    list_for_each_entry(chan, &relay_channels, list) {
+        if (chan->buf[hotcpu])
+            continue;
+        sprintf(tmpname, "%s%d", chan->base_filename, hotcpu);
+        chan->buf[hotcpu] = relay_open_buf(chan, tmpname, chan->parent,
+                           &is_global);
+        if (!chan->buf[hotcpu]) {
+            err = -ENOMEM;
+            break;
+        }
+        chan->buf[hotcpu]->cpu = hotcpu;
+    }
+    mutex_unlock(&relay_channels_mutex);
+
+    kfree(tmpname);
+
+    return err;
+}
+
+static void relay_flush_hotcpu(int hotcpu)
+{
+    struct rchan *chan;
+
+    mutex_lock(&relay_channels_mutex);
+    list_for_each_entry(chan, &relay_channels, list) {
+        if (chan->buf[hotcpu])
+            continue;
+        relay_switch_subbuf(chan->buf[hotcpu], 0);
+    }
+    mutex_unlock(&relay_channels_mutex);
+}
+
+static int __cpuinit relay_hotcpu_callback(struct notifier_block *nb,
+                       unsigned long action,
+                       void *hcpu)
+{
+    unsigned int hotcpu = (unsigned long)hcpu;
+
+    switch(action) {
+    case CPU_UP_PREPARE:
+        if (relay_add_hotcpu(hotcpu)) {
+            printk(KERN_ERR "relay_add_hotcpu: cpu %d buffer creation 
failed\n", hotcpu);
+            return NOTIFY_BAD;
+        }
+        break;
+    case CPU_DEAD:
+        relay_flush_hotcpu(hotcpu);
+        break;
+    }
+    return NOTIFY_OK;
+}
+
  /**
   *    relay_open - create a new relay channel
   *    @base_filename: base name of files to create
@@ -486,6 +555,8 @@ struct rchan *relay_open(const char *bas
      chan->n_subbufs = n_subbufs;
      chan->subbuf_size = subbuf_size;
      chan->alloc_size = FIX_SIZE(subbuf_size * n_subbufs);
+    chan->parent = parent;
+    strncpy(chan->base_filename, base_filename, NAME_MAX + 1);
      setup_callbacks(chan, cb);
      kref_init(&chan->kref);

@@ -503,13 +574,16 @@ struct rchan *relay_open(const char *bas
          chan->buf[i]->cpu = i;
      }

+    mutex_lock(&relay_channels_mutex);
+    list_add(&chan->list, &relay_channels);
+    mutex_unlock(&relay_channels_mutex);
      kfree(tmpname);
      return chan;

  free_bufs:
-    for (i = 0; i < NR_CPUS; i++) {
+    for_each_possible_cpu(i) {
          if (!chan->buf[i])
-            break;
+            continue;
          relay_close_buf(chan->buf[i]);
          if (is_global)
              break;
@@ -622,9 +696,9 @@ void relay_close(struct rchan *chan)
      if (!chan)
          return;

-    for (i = 0; i < NR_CPUS; i++) {
+    for_each_possible_cpu(i) {
          if (!chan->buf[i] || chan->buf[i] == prev)
-            break;
+            continue;
          relay_close_buf(chan->buf[i]);
          prev = chan->buf[i];
      }
@@ -652,9 +726,9 @@ void relay_flush(struct rchan *chan)
      if (!chan)
          return;

-    for (i = 0; i < NR_CPUS; i++) {
+    for_each_possible_cpu(i) {
          if (!chan->buf[i] || chan->buf[i] == prev)
-            break;
+            continue;
          relay_switch_subbuf(chan->buf[i], 0);
          prev = chan->buf[i];
      }
@@ -1021,3 +1095,12 @@ struct file_operations relay_file_operat
      .sendfile       = relay_file_sendfile,
  };
  EXPORT_SYMBOL_GPL(relay_file_operations);
+
+static __init int relay_init(void)
+{
+    hotcpu_notifier(relay_hotcpu_callback, 0);
+
+    return 0;
+}
+
+module_init(relay_init);