From mboxrd@z Thu Jan 1 00:00:00 1970 From: compudj@krystal.dyndns.org (Mathieu Desnoyers) Date: Wed, 4 Mar 2009 11:55:20 -0500 Subject: [ltt-dev] [PATCH] ltt-relay: remove percpu data, use direct pointer In-Reply-To: <49AB810A.5070008@cn.fujitsu.com> References: <49AB810A.5070008@cn.fujitsu.com> Message-ID: <20090304165519.GA1150@Krystal> * 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 > --- > 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