Re: [PATCH] relay: Use per CPU constructs for the relay channel buffer pointers

[Chris Wilson]

On Thu, Aug 11, 2016 at 12:35:40PM +0530, akash.g...@intel.com wrote:
> From: Akash Goel 
> 
> relay essentially needs to maintain the per CPU array of channel buffer
> pointers but it manually creates that array.
> Instead its better to avail the per CPU constructs, provided by the
> kernel, to allocate & access the array of pointer to channel buffers.
> 
> Cc: Chris Wilson 
> Cc: Tvrtko Ursulin 
> Signed-off-by: Akash Goel 

This has the benefit of being a mechnical change and I could not think
of a better way to express the per-cpu indirection.

relay.h should probably include  so that it pulls in
the percpu api explicitly.

Reviewed-by: Chris Wilson 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] relay: Use per CPU constructs for the relay channel buffer pointers

[Chris Wilson]

On Thu, Aug 11, 2016 at 12:35:40PM +0530, akash.g...@intel.com wrote:
> From: Akash Goel 
> 
> relay essentially needs to maintain the per CPU array of channel buffer
> pointers but it manually creates that array.
> Instead its better to avail the per CPU constructs, provided by the
> kernel, to allocate & access the array of pointer to channel buffers.
> 
> Cc: Chris Wilson 
> Cc: Tvrtko Ursulin 
> Signed-off-by: Akash Goel 

This has the benefit of being a mechnical change and I could not think
of a better way to express the per-cpu indirection.

relay.h should probably include  so that it pulls in
the percpu api explicitly.

Reviewed-by: Chris Wilson 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] relay: Use per CPU constructs for the relay channel buffer pointers

[akash . goel]

From: Akash Goel 

relay essentially needs to maintain the per CPU array of channel buffer
pointers but it manually creates that array.
Instead its better to avail the per CPU constructs, provided by the
kernel, to allocate & access the array of pointer to channel buffers.

Cc: Chris Wilson 
Cc: Tvrtko Ursulin 
Signed-off-by: Akash Goel 
---
 include/linux/relay.h | 16 ++-
 kernel/relay.c| 74 +--
 2 files changed, 51 insertions(+), 39 deletions(-)

diff --git a/include/linux/relay.h b/include/linux/relay.h
index d7c8359..74a6e72 100644
--- a/include/linux/relay.h
+++ b/include/linux/relay.h
@@ -63,7 +63,7 @@ struct rchan
struct kref kref;   /* channel refcount */
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 rchan_buf ** __percpu buf; /* per-cpu channel buffers */
int is_global;  /* One global buffer ? */
struct list_head list;  /* for channel list */
struct dentry *parent;  /* parent dentry passed to open */
@@ -204,7 +204,7 @@ static inline void relay_write(struct rchan *chan,
struct rchan_buf *buf;
 
local_irq_save(flags);
-   buf = chan->buf[smp_processor_id()];
+   buf = *this_cpu_ptr(chan->buf);
if (unlikely(buf->offset + length > chan->subbuf_size))
length = relay_switch_subbuf(buf, length);
memcpy(buf->data + buf->offset, data, length);
@@ -230,12 +230,12 @@ static inline void __relay_write(struct rchan *chan,
 {
struct rchan_buf *buf;
 
-   buf = chan->buf[get_cpu()];
+   buf = *get_cpu_ptr(chan->buf);
if (unlikely(buf->offset + length > buf->chan->subbuf_size))
length = relay_switch_subbuf(buf, length);
memcpy(buf->data + buf->offset, data, length);
buf->offset += length;
-   put_cpu();
+   put_cpu_ptr(chan->buf);
 }
 
 /**
@@ -251,17 +251,19 @@ static inline void __relay_write(struct rchan *chan,
  */
 static inline void *relay_reserve(struct rchan *chan, size_t length)
 {
-   void *reserved;
-   struct rchan_buf *buf = chan->buf[smp_processor_id()];
+   void *reserved = NULL;
+   struct rchan_buf *buf = *get_cpu_ptr(chan->buf);
 
if (unlikely(buf->offset + length > buf->chan->subbuf_size)) {
length = relay_switch_subbuf(buf, length);
if (!length)
-   return NULL;
+   goto end;
}
reserved = buf->data + buf->offset;
buf->offset += length;
 
+end:
+   put_cpu_ptr(chan->buf);
return reserved;
 }
 
diff --git a/kernel/relay.c b/kernel/relay.c
index d797502..f55ab82 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -214,7 +214,7 @@ static void relay_destroy_buf(struct rchan_buf *buf)
__free_page(buf->page_array[i]);
relay_free_page_array(buf->page_array);
}
-   chan->buf[buf->cpu] = NULL;
+   *per_cpu_ptr(chan->buf, buf->cpu) = NULL;
kfree(buf->padding);
kfree(buf);
kref_put(>kref, relay_destroy_channel);
@@ -382,20 +382,21 @@ static void __relay_reset(struct rchan_buf *buf, unsigned 
int init)
  */
 void relay_reset(struct rchan *chan)
 {
+   struct rchan_buf *buf;
unsigned int i;
 
if (!chan)
return;
 
-   if (chan->is_global && chan->buf[0]) {
-   __relay_reset(chan->buf[0], 0);
+   if (chan->is_global && (buf = *per_cpu_ptr(chan->buf, 0))) {
+   __relay_reset(buf, 0);
return;
}
 
mutex_lock(_channels_mutex);
for_each_possible_cpu(i)
-   if (chan->buf[i])
-   __relay_reset(chan->buf[i], 0);
+   if ((buf = *per_cpu_ptr(chan->buf, i)))
+   __relay_reset(buf, 0);
mutex_unlock(_channels_mutex);
 }
 EXPORT_SYMBOL_GPL(relay_reset);
@@ -440,7 +441,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, 
unsigned int cpu)
struct dentry *dentry;
 
if (chan->is_global)
-   return chan->buf[0];
+   return *per_cpu_ptr(chan->buf, 0);
 
buf = relay_create_buf(chan);
if (!buf)
@@ -464,7 +465,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, 
unsigned int cpu)
__relay_reset(buf, 1);
 
if(chan->is_global) {
-   chan->buf[0] = buf;
+   *per_cpu_ptr(chan->buf, 0) = buf;
buf->cpu = 0;
}
 
@@ -526,22 +527,24 @@ static int relay_hotcpu_callback(struct notifier_block 
*nb,
 {
unsigned int hotcpu = (unsigned long)hcpu;
struct rchan *chan;
+ 

[PATCH] relay: Use per CPU constructs for the relay channel buffer pointers

[akash . goel]

From: Akash Goel 

relay essentially needs to maintain the per CPU array of channel buffer
pointers but it manually creates that array.
Instead its better to avail the per CPU constructs, provided by the
kernel, to allocate & access the array of pointer to channel buffers.

Cc: Chris Wilson 
Cc: Tvrtko Ursulin 
Signed-off-by: Akash Goel 
---
 include/linux/relay.h | 16 ++-
 kernel/relay.c| 74 +--
 2 files changed, 51 insertions(+), 39 deletions(-)

diff --git a/include/linux/relay.h b/include/linux/relay.h
index d7c8359..74a6e72 100644
--- a/include/linux/relay.h
+++ b/include/linux/relay.h
@@ -63,7 +63,7 @@ struct rchan
struct kref kref;   /* channel refcount */
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 rchan_buf ** __percpu buf; /* per-cpu channel buffers */
int is_global;  /* One global buffer ? */
struct list_head list;  /* for channel list */
struct dentry *parent;  /* parent dentry passed to open */
@@ -204,7 +204,7 @@ static inline void relay_write(struct rchan *chan,
struct rchan_buf *buf;
 
local_irq_save(flags);
-   buf = chan->buf[smp_processor_id()];
+   buf = *this_cpu_ptr(chan->buf);
if (unlikely(buf->offset + length > chan->subbuf_size))
length = relay_switch_subbuf(buf, length);
memcpy(buf->data + buf->offset, data, length);
@@ -230,12 +230,12 @@ static inline void __relay_write(struct rchan *chan,
 {
struct rchan_buf *buf;
 
-   buf = chan->buf[get_cpu()];
+   buf = *get_cpu_ptr(chan->buf);
if (unlikely(buf->offset + length > buf->chan->subbuf_size))
length = relay_switch_subbuf(buf, length);
memcpy(buf->data + buf->offset, data, length);
buf->offset += length;
-   put_cpu();
+   put_cpu_ptr(chan->buf);
 }
 
 /**
@@ -251,17 +251,19 @@ static inline void __relay_write(struct rchan *chan,
  */
 static inline void *relay_reserve(struct rchan *chan, size_t length)
 {
-   void *reserved;
-   struct rchan_buf *buf = chan->buf[smp_processor_id()];
+   void *reserved = NULL;
+   struct rchan_buf *buf = *get_cpu_ptr(chan->buf);
 
if (unlikely(buf->offset + length > buf->chan->subbuf_size)) {
length = relay_switch_subbuf(buf, length);
if (!length)
-   return NULL;
+   goto end;
}
reserved = buf->data + buf->offset;
buf->offset += length;
 
+end:
+   put_cpu_ptr(chan->buf);
return reserved;
 }
 
diff --git a/kernel/relay.c b/kernel/relay.c
index d797502..f55ab82 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -214,7 +214,7 @@ static void relay_destroy_buf(struct rchan_buf *buf)
__free_page(buf->page_array[i]);
relay_free_page_array(buf->page_array);
}
-   chan->buf[buf->cpu] = NULL;
+   *per_cpu_ptr(chan->buf, buf->cpu) = NULL;
kfree(buf->padding);
kfree(buf);
kref_put(>kref, relay_destroy_channel);
@@ -382,20 +382,21 @@ static void __relay_reset(struct rchan_buf *buf, unsigned 
int init)
  */
 void relay_reset(struct rchan *chan)
 {
+   struct rchan_buf *buf;
unsigned int i;
 
if (!chan)
return;
 
-   if (chan->is_global && chan->buf[0]) {
-   __relay_reset(chan->buf[0], 0);
+   if (chan->is_global && (buf = *per_cpu_ptr(chan->buf, 0))) {
+   __relay_reset(buf, 0);
return;
}
 
mutex_lock(_channels_mutex);
for_each_possible_cpu(i)
-   if (chan->buf[i])
-   __relay_reset(chan->buf[i], 0);
+   if ((buf = *per_cpu_ptr(chan->buf, i)))
+   __relay_reset(buf, 0);
mutex_unlock(_channels_mutex);
 }
 EXPORT_SYMBOL_GPL(relay_reset);
@@ -440,7 +441,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, 
unsigned int cpu)
struct dentry *dentry;
 
if (chan->is_global)
-   return chan->buf[0];
+   return *per_cpu_ptr(chan->buf, 0);
 
buf = relay_create_buf(chan);
if (!buf)
@@ -464,7 +465,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, 
unsigned int cpu)
__relay_reset(buf, 1);
 
if(chan->is_global) {
-   chan->buf[0] = buf;
+   *per_cpu_ptr(chan->buf, 0) = buf;
buf->cpu = 0;
}
 
@@ -526,22 +527,24 @@ static int relay_hotcpu_callback(struct notifier_block 
*nb,
 {
unsigned int hotcpu = (unsigned long)hcpu;
struct rchan *chan;
+   struct rchan_buf *buf;
 
switch(action) {
case CPU_UP_PREPARE:
case