[ltt-dev] [PATCH] ltt-relay: remove percpu data, use direct pointer
Mathieu Desnoyers
compudj at krystal.dyndns.org
Wed Mar 4 11:55:20 EST 2009
* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
>
> struct ltt_channel_buf_struct is percpu data, we can not
> access it directly.
>
> And this percpu data use channel->kref too, it's not right.
>
Merged, thanks !
Mathieu
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> ---
> include/linux/ltt-relay.h | 1
> ltt/ltt-relay.c | 118 +++++++++++++++-------------------------------
> 2 files changed, 40 insertions(+), 79 deletions(-)
> diff --git a/include/linux/ltt-relay.h b/include/linux/ltt-relay.h
> index ac3415a..b26eaea 100644
> --- a/include/linux/ltt-relay.h
> +++ b/include/linux/ltt-relay.h
> @@ -43,6 +43,7 @@ struct buf_page {
> * Per-cpu relay channel buffer
> */
> struct rchan_buf {
> + void *chan_private; /* private data for this buf */
> struct rchan *chan; /* associated channel */
> wait_queue_head_t read_wait; /* reader wait queue */
> struct timer_list timer; /* reader wake-up timer */
> diff --git a/ltt/ltt-relay.c b/ltt/ltt-relay.c
> index 7c544f4..d1ba449 100644
> --- a/ltt/ltt-relay.c
> +++ b/ltt/ltt-relay.c
> @@ -176,10 +176,7 @@ static void ltt_buffer_begin_callback(struct rchan_buf *buf,
> static notrace void ltt_buffer_end_callback(struct rchan_buf *buf,
> u64 tsc, unsigned int offset, unsigned int subbuf_idx)
> {
> - struct ltt_channel_struct *channel =
> - (struct ltt_channel_struct *)buf->chan->private_data;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(channel->buf, buf->cpu);
> + struct ltt_channel_buf_struct *ltt_buf = buf->chan_private;
> struct ltt_subbuffer_header *header =
> (struct ltt_subbuffer_header *)
> ltt_relay_offset_address(buf,
> @@ -195,10 +192,7 @@ static notrace void ltt_buffer_end_callback(struct rchan_buf *buf,
> static notrace void ltt_deliver(struct rchan_buf *buf, unsigned int subbuf_idx,
> void *subbuf)
> {
> - struct ltt_channel_struct *channel =
> - (struct ltt_channel_struct *)buf->chan->private_data;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(channel->buf, buf->cpu);
> + struct ltt_channel_buf_struct *ltt_buf = buf->chan_private;
>
> atomic_set(<t_buf->wakeup_readers, 1);
> }
> @@ -259,10 +253,7 @@ static notrace void ltt_buf_unfull(struct rchan_buf *buf,
> unsigned int subbuf_idx,
> long offset)
> {
> - struct ltt_channel_struct *ltt_channel =
> - (struct ltt_channel_struct *)buf->chan->private_data;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_channel->buf, buf->cpu);
> + struct ltt_channel_buf_struct *ltt_buf = buf->chan_private;
>
> ltt_relay_wake_writers(ltt_buf);
> }
> @@ -278,10 +269,7 @@ static notrace void ltt_buf_unfull(struct rchan_buf *buf,
> static int ltt_open(struct inode *inode, struct file *file)
> {
> struct rchan_buf *buf = inode->i_private;
> - struct ltt_channel_struct *ltt_channel =
> - (struct ltt_channel_struct *)buf->chan->private_data;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_channel->buf, buf->cpu);
> + struct ltt_channel_buf_struct *ltt_buf = buf->chan_private;
>
> if (!atomic_long_add_unless(<t_buf->active_readers, 1, 1))
> return -EBUSY;
> @@ -298,10 +286,7 @@ static int ltt_open(struct inode *inode, struct file *file)
> static int ltt_release(struct inode *inode, struct file *file)
> {
> struct rchan_buf *buf = inode->i_private;
> - struct ltt_channel_struct *ltt_channel =
> - (struct ltt_channel_struct *)buf->chan->private_data;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_channel->buf, buf->cpu);
> + struct ltt_channel_buf_struct *ltt_buf = buf->chan_private;
> int ret;
>
> WARN_ON(atomic_long_read(<t_buf->active_readers) != 1);
> @@ -323,10 +308,7 @@ static unsigned int ltt_poll(struct file *filp, poll_table *wait)
> unsigned int mask = 0;
> struct inode *inode = filp->f_dentry->d_inode;
> struct rchan_buf *buf = inode->i_private;
> - struct ltt_channel_struct *ltt_channel =
> - (struct ltt_channel_struct *)buf->chan->private_data;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_channel->buf, buf->cpu);
> + struct ltt_channel_buf_struct *ltt_buf = buf->chan_private;
>
> if (filp->f_mode & FMODE_READ) {
> poll_wait_set_exclusive(wait);
> @@ -343,8 +325,7 @@ static unsigned int ltt_poll(struct file *filp, poll_table *wait)
> else
> return 0;
> } else {
> - struct rchan *rchan =
> - ltt_channel->trans_channel_data;
> + struct rchan *rchan = buf->chan;
> if (SUBBUF_TRUNC(local_read(<t_buf->offset),
> buf->chan)
> - SUBBUF_TRUNC(atomic_long_read(
> @@ -385,8 +366,7 @@ static int ltt_ioctl(struct inode *inode, struct file *filp,
> struct rchan_buf *buf = inode->i_private;
> struct ltt_channel_struct *ltt_channel =
> (struct ltt_channel_struct *)buf->chan->private_data;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_channel->buf, buf->cpu);
> + struct ltt_channel_buf_struct *ltt_buf = buf->chan_private;
> u32 __user *argp = (u32 __user *)arg;
>
> WARN_ON(atomic_long_read(<t_buf->active_readers) != 1);
> @@ -518,10 +498,7 @@ static int subbuf_splice_actor(struct file *in,
> unsigned int flags)
> {
> struct rchan_buf *buf = in->private_data;
> - struct ltt_channel_struct *ltt_channel =
> - (struct ltt_channel_struct *)buf->chan->private_data;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_channel->buf, buf->cpu);
> + struct ltt_channel_buf_struct *ltt_buf = buf->chan_private;
> unsigned int poff, subbuf_pages, nr_pages;
> struct page *pages[PIPE_BUFFERS];
> struct partial_page partial[PIPE_BUFFERS];
> @@ -628,8 +605,7 @@ static void ltt_relay_print_subbuffer_errors(
> long cons_off, unsigned int cpu)
> {
> struct rchan *rchan = ltt_chan->trans_channel_data;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_chan->buf, cpu);
> + struct ltt_channel_buf_struct *ltt_buf = rchan->buf[cpu]->chan_private;
> long cons_idx, commit_count, write_offset;
>
> cons_idx = SUBBUF_INDEX(cons_off, rchan);
> @@ -660,8 +636,7 @@ static void ltt_relay_print_errors(struct ltt_trace_struct *trace,
> struct ltt_channel_struct *ltt_chan, int cpu)
> {
> struct rchan *rchan = ltt_chan->trans_channel_data;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_chan->buf, cpu);
> + struct ltt_channel_buf_struct *ltt_buf = rchan->buf[cpu]->chan_private;
> long cons_off;
>
> for (cons_off = atomic_long_read(<t_buf->consumed);
> @@ -676,8 +651,8 @@ static void ltt_relay_print_buffer_errors(struct ltt_channel_struct *ltt_chan,
> unsigned int cpu)
> {
> struct ltt_trace_struct *trace = ltt_chan->trace;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_chan->buf, cpu);
> + struct rchan *rchan = ltt_chan->trans_channel_data;
> + struct ltt_channel_buf_struct *ltt_buf = rchan->buf[cpu]->chan_private;
>
> if (local_read(<t_buf->events_lost))
> printk(KERN_ALERT
> @@ -702,13 +677,6 @@ static void ltt_relay_remove_dirs(struct ltt_trace_struct *trace)
> debugfs_remove(trace->dentry.trace_root);
> }
>
> -static void ltt_relay_release_channel(struct kref *kref)
> -{
> - struct ltt_channel_struct *ltt_chan = container_of(kref,
> - struct ltt_channel_struct, kref);
> - percpu_free(ltt_chan->buf);
> -}
> -
> /*
> * Create ltt buffer.
> */
> @@ -716,18 +684,24 @@ static int ltt_relay_create_buffer(struct ltt_trace_struct *trace,
> struct ltt_channel_struct *ltt_chan, struct rchan_buf *buf,
> unsigned int cpu, unsigned int n_subbufs)
> {
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_chan->buf, cpu);
> + struct ltt_channel_buf_struct *ltt_buf;
> unsigned int j;
>
> + ltt_buf = kzalloc_node(sizeof(*ltt_buf), GFP_KERNEL, cpu_to_node(cpu));
> + if (!ltt_buf)
> + return -ENOMEM;
> +
> ltt_buf->commit_count =
> kzalloc_node(sizeof(ltt_buf->commit_count) * n_subbufs,
> GFP_KERNEL, cpu_to_node(cpu));
> - if (!ltt_buf->commit_count)
> + if (!ltt_buf->commit_count) {
> + kfree(ltt_buf);
> return -ENOMEM;
> + }
> + buf->chan_private = ltt_buf;
> +
> kref_get(&trace->kref);
> kref_get(&trace->ltt_transport_kref);
> - kref_get(<t_chan->kref);
> local_set(<t_buf->offset, ltt_subbuffer_header_size());
> atomic_long_set(<t_buf->consumed, 0);
> atomic_long_set(<t_buf->active_readers, 0);
> @@ -752,15 +726,14 @@ static void ltt_relay_destroy_buffer(struct ltt_channel_struct *ltt_chan,
> unsigned int cpu)
> {
> struct ltt_trace_struct *trace = ltt_chan->trace;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_chan->buf, cpu);
> + struct rchan *rchan = ltt_chan->trans_channel_data;
> + struct ltt_channel_buf_struct *ltt_buf = rchan->buf[cpu]->chan_private;
>
> kref_put(<t_chan->trace->ltt_transport_kref,
> ltt_release_transport);
> ltt_relay_print_buffer_errors(ltt_chan, cpu);
> kfree(ltt_buf->commit_count);
> - ltt_buf->commit_count = NULL;
> - kref_put(<t_chan->kref, ltt_relay_release_channel);
> + kfree(ltt_buf);
> kref_put(&trace->kref, ltt_release_trace);
> wake_up_interruptible(&trace->kref_wq);
> }
> @@ -790,18 +763,12 @@ static int ltt_relay_create_channel(const char *trace_name,
> }
> strncat(tmpname, "_", PATH_MAX-1-strlen(tmpname));
>
> - kref_init(<t_chan->kref);
> -
> ltt_chan->trace = trace;
> ltt_chan->buffer_begin = ltt_buffer_begin_callback;
> ltt_chan->buffer_end = ltt_buffer_end_callback;
> ltt_chan->overwrite = overwrite;
> ltt_chan->n_subbufs_order = get_count_order(n_subbufs);
> ltt_chan->commit_count_mask = (~0UL >> ltt_chan->n_subbufs_order);
> - ltt_chan->buf = percpu_alloc_mask(sizeof(struct ltt_channel_buf_struct),
> - GFP_KERNEL, cpu_possible_map);
> - if (!ltt_chan->buf)
> - goto ltt_percpu_alloc_error;
> ltt_chan->trans_channel_data = ltt_relay_open(tmpname,
> dir,
> subbuf_size,
> @@ -823,8 +790,6 @@ static int ltt_relay_create_channel(const char *trace_name,
> goto end;
>
> relay_open_error:
> - percpu_free(ltt_chan->buf);
> -ltt_percpu_alloc_error:
> err = EPERM;
> end:
> kfree(tmpname);
> @@ -865,9 +830,12 @@ static void ltt_relay_async_wakeup_chan(struct ltt_channel_struct *ltt_channel)
> struct rchan *rchan = ltt_channel->trans_channel_data;
>
> for_each_possible_cpu(i) {
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_channel->buf, i);
> + struct ltt_channel_buf_struct *ltt_buf;
>
> + if (!rchan->buf[i])
> + continue;
> +
> + ltt_buf = rchan->buf[i]->chan_private;
> if (atomic_read(<t_buf->wakeup_readers) == 1) {
> atomic_set(<t_buf->wakeup_readers, 0);
> wake_up_interruptible(&rchan->buf[i]->read_wait);
> @@ -882,7 +850,7 @@ static void ltt_relay_finish_buffer(struct ltt_channel_struct *ltt_channel,
>
> if (rchan->buf[cpu]) {
> struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_channel->buf, cpu);
> + rchan->buf[cpu]->chan_private;
> ltt_relay_buffer_flush(rchan->buf[cpu]);
> ltt_relay_wake_writers(ltt_buf);
> }
> @@ -902,7 +870,6 @@ static void ltt_relay_remove_channel(struct ltt_channel_struct *channel)
> struct rchan *rchan = channel->trans_channel_data;
>
> ltt_relay_close(rchan);
> - kref_put(&channel->kref, ltt_relay_release_channel);
> }
>
> struct ltt_reserve_switch_offsets {
> @@ -1299,10 +1266,8 @@ static notrace int ltt_relay_reserve_slot(struct ltt_trace_struct *trace,
> unsigned int *rflags, int largest_align, int cpu)
> {
> struct rchan *rchan = ltt_channel->trans_channel_data;
> - struct rchan_buf *buf = *transport_data =
> - rchan->buf[cpu];
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_channel->buf, buf->cpu);
> + struct rchan_buf *buf = *transport_data = rchan->buf[cpu];
> + struct ltt_channel_buf_struct *ltt_buf = buf->chan_private;
> struct ltt_reserve_switch_offsets offsets;
>
> offsets.reserve_commit_diff = 0;
> @@ -1373,8 +1338,7 @@ static notrace void ltt_force_switch(struct rchan_buf *buf,
> {
> struct ltt_channel_struct *ltt_channel =
> (struct ltt_channel_struct *)buf->chan->private_data;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_channel->buf, buf->cpu);
> + struct ltt_channel_buf_struct *ltt_buf = buf->chan_private;
> struct rchan *rchan = ltt_channel->trans_channel_data;
> struct ltt_reserve_switch_offsets offsets;
> u64 tsc;
> @@ -1435,10 +1399,7 @@ static notrace void ltt_force_switch(struct rchan_buf *buf,
> static inline void ltt_write_commit_counter(struct rchan_buf *buf,
> long buf_offset, size_t slot_size)
> {
> - struct ltt_channel_struct *ltt_channel =
> - (struct ltt_channel_struct *)buf->chan->private_data;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_channel->buf, buf->cpu);
> + struct ltt_channel_buf_struct *ltt_buf = buf->chan_private;
> struct ltt_subbuffer_header *header;
> long offset, subbuf_idx, commit_count;
> uint32_t lost_old, lost_new;
> @@ -1488,8 +1449,7 @@ static notrace void ltt_relay_commit_slot(
> void **transport_data, long buf_offset, size_t slot_size)
> {
> struct rchan_buf *buf = *transport_data;
> - struct ltt_channel_buf_struct *ltt_buf =
> - percpu_ptr(ltt_channel->buf, buf->cpu);
> + struct ltt_channel_buf_struct *ltt_buf = buf->chan_private;
> struct rchan *rchan = buf->chan;
> long offset_end = buf_offset;
> long endidx = SUBBUF_INDEX(offset_end - 1, rchan);
> @@ -1532,7 +1492,7 @@ static int ltt_relay_user_blocking(struct ltt_trace_struct *trace,
> rchan = channel->trans_channel_data;
> cpu = smp_processor_id();
> relay_buf = rchan->buf[cpu];
> - ltt_buf = percpu_ptr(channel->buf, cpu);
> + ltt_buf = relay_buf->chan_private;
>
> /*
> * Check if data is too big for the channel : do not
> @@ -1583,7 +1543,7 @@ static void ltt_relay_print_user_errors(struct ltt_trace_struct *trace,
> channel = &trace->channels[chan_index];
> rchan = channel->trans_channel_data;
> relay_buf = rchan->buf[cpu];
> - ltt_buf = percpu_ptr(channel->buf, cpu);
> + ltt_buf = relay_buf->chan_private;
>
> printk(KERN_ERR "Error in LTT usertrace : "
> "buffer full : event lost in blocking "
>
>
>
>
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
More information about the lttng-dev
mailing list