Re: [RFC][PATCH 1/5 v2] tracing: Add trace_seq_buffer_ptr() helper function

From: Steven Rostedt
Date: Thu Jun 26 2014 - 21:06:27 EST



As this patch is in my 3.17 queue and it touches the kvm and scsi
tracepoint code, I figured I should at least do the courtesy of
notifying the maintainers of those subsystems.

Do you have any issues with this going through my tree? If not, please
give me an Acked-by.

Thanks!

-- Steve

On Thu, 26 Jun 2014 17:49:02 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> From: "Steven Rostedt (Red Hat)" <rostedt@xxxxxxxxxxx>
>
> There's several locations in the kernel that open code the calculation
> of the next location in the trace_seq buffer. This is usually done with
>
> p->buffer + p->len
>
> Instead of having this open coded, supply a helper function in the
> header to do it for them. This function is called trace_seq_buffer_ptr().
>
> Signed-off-by: Steven Rostedt <rostedt@xxxxxxxxxxx>
> ---
> arch/x86/kvm/mmutrace.h | 2 +-
> drivers/scsi/scsi_trace.c | 16 ++++++++--------
> include/linux/trace_seq.h | 15 +++++++++++++++
> kernel/trace/trace_output.c | 14 +++++++-------
> 4 files changed, 31 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
> index 9d2e0ffcb190..2e5652b62fd6 100644
> --- a/arch/x86/kvm/mmutrace.h
> +++ b/arch/x86/kvm/mmutrace.h
> @@ -22,7 +22,7 @@
> __entry->unsync = sp->unsync;
>
> #define KVM_MMU_PAGE_PRINTK() ({ \
> - const char *ret = p->buffer + p->len; \
> + const char *ret = trace_seq_buffer_ptr(p); \
> static const char *access_str[] = { \
> "---", "--x", "w--", "w-x", "-u-", "-ux", "wu-", "wux" \
> }; \
> diff --git a/drivers/scsi/scsi_trace.c b/drivers/scsi/scsi_trace.c
> index 2bea4f0b684a..503594e5f76d 100644
> --- a/drivers/scsi/scsi_trace.c
> +++ b/drivers/scsi/scsi_trace.c
> @@ -28,7 +28,7 @@ scsi_trace_misc(struct trace_seq *, unsigned char *, int);
> static const char *
> scsi_trace_rw6(struct trace_seq *p, unsigned char *cdb, int len)
> {
> - const char *ret = p->buffer + p->len;
> + const char *ret = trace_seq_buffer_ptr(p);
> sector_t lba = 0, txlen = 0;
>
> lba |= ((cdb[1] & 0x1F) << 16);
> @@ -46,7 +46,7 @@ scsi_trace_rw6(struct trace_seq *p, unsigned char *cdb, int len)
> static const char *
> scsi_trace_rw10(struct trace_seq *p, unsigned char *cdb, int len)
> {
> - const char *ret = p->buffer + p->len;
> + const char *ret = trace_seq_buffer_ptr(p);
> sector_t lba = 0, txlen = 0;
>
> lba |= (cdb[2] << 24);
> @@ -71,7 +71,7 @@ scsi_trace_rw10(struct trace_seq *p, unsigned char *cdb, int len)
> static const char *
> scsi_trace_rw12(struct trace_seq *p, unsigned char *cdb, int len)
> {
> - const char *ret = p->buffer + p->len;
> + const char *ret = trace_seq_buffer_ptr(p);
> sector_t lba = 0, txlen = 0;
>
> lba |= (cdb[2] << 24);
> @@ -94,7 +94,7 @@ scsi_trace_rw12(struct trace_seq *p, unsigned char *cdb, int len)
> static const char *
> scsi_trace_rw16(struct trace_seq *p, unsigned char *cdb, int len)
> {
> - const char *ret = p->buffer + p->len;
> + const char *ret = trace_seq_buffer_ptr(p);
> sector_t lba = 0, txlen = 0;
>
> lba |= ((u64)cdb[2] << 56);
> @@ -125,7 +125,7 @@ scsi_trace_rw16(struct trace_seq *p, unsigned char *cdb, int len)
> static const char *
> scsi_trace_rw32(struct trace_seq *p, unsigned char *cdb, int len)
> {
> - const char *ret = p->buffer + p->len, *cmd;
> + const char *ret = trace_seq_buffer_ptr(p), *cmd;
> sector_t lba = 0, txlen = 0;
> u32 ei_lbrt = 0;
>
> @@ -180,7 +180,7 @@ out:
> static const char *
> scsi_trace_unmap(struct trace_seq *p, unsigned char *cdb, int len)
> {
> - const char *ret = p->buffer + p->len;
> + const char *ret = trace_seq_buffer_ptr(p);
> unsigned int regions = cdb[7] << 8 | cdb[8];
>
> trace_seq_printf(p, "regions=%u", (regions - 8) / 16);
> @@ -192,7 +192,7 @@ scsi_trace_unmap(struct trace_seq *p, unsigned char *cdb, int len)
> static const char *
> scsi_trace_service_action_in(struct trace_seq *p, unsigned char *cdb, int len)
> {
> - const char *ret = p->buffer + p->len, *cmd;
> + const char *ret = trace_seq_buffer_ptr(p), *cmd;
> sector_t lba = 0;
> u32 alloc_len = 0;
>
> @@ -247,7 +247,7 @@ scsi_trace_varlen(struct trace_seq *p, unsigned char *cdb, int len)
> static const char *
> scsi_trace_misc(struct trace_seq *p, unsigned char *cdb, int len)
> {
> - const char *ret = p->buffer + p->len;
> + const char *ret = trace_seq_buffer_ptr(p);
>
> trace_seq_printf(p, "-");
> trace_seq_putc(p, 0);
> diff --git a/include/linux/trace_seq.h b/include/linux/trace_seq.h
> index dd85753e1bb0..ea6c9dea79e3 100644
> --- a/include/linux/trace_seq.h
> +++ b/include/linux/trace_seq.h
> @@ -25,6 +25,21 @@ trace_seq_init(struct trace_seq *s)
> s->full = 0;
> }
>
> +/**
> + * trace_seq_buffer_ptr - return pointer to next location in buffer
> + * @s: trace sequence descriptor
> + *
> + * Returns the pointer to the buffer where the next write to
> + * the buffer will happen. This is useful to save the location
> + * that is about to be written to and then return the result
> + * of that write.
> + */
> +static inline unsigned char *
> +trace_seq_buffer_ptr(struct trace_seq *s)
> +{
> + return s->buffer + s->len;
> +}
> +
> /*
> * Currently only defined when tracing is enabled.
> */
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index b8930f79a04b..c6977d5a9b12 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -75,7 +75,7 @@ ftrace_print_flags_seq(struct trace_seq *p, const char *delim,
> {
> unsigned long mask;
> const char *str;
> - const char *ret = p->buffer + p->len;
> + const char *ret = trace_seq_buffer_ptr(p);
> int i, first = 1;
>
> for (i = 0; flag_array[i].name && flags; i++) {
> @@ -111,7 +111,7 @@ ftrace_print_symbols_seq(struct trace_seq *p, unsigned long val,
> const struct trace_print_flags *symbol_array)
> {
> int i;
> - const char *ret = p->buffer + p->len;
> + const char *ret = trace_seq_buffer_ptr(p);
>
> for (i = 0; symbol_array[i].name; i++) {
>
> @@ -122,7 +122,7 @@ ftrace_print_symbols_seq(struct trace_seq *p, unsigned long val,
> break;
> }
>
> - if (ret == (const char *)(p->buffer + p->len))
> + if (ret == (const char *)(trace_seq_buffer_ptr(p)))
> trace_seq_printf(p, "0x%lx", val);
>
> trace_seq_putc(p, 0);
> @@ -137,7 +137,7 @@ ftrace_print_symbols_seq_u64(struct trace_seq *p, unsigned long long val,
> const struct trace_print_flags_u64 *symbol_array)
> {
> int i;
> - const char *ret = p->buffer + p->len;
> + const char *ret = trace_seq_buffer_ptr(p);
>
> for (i = 0; symbol_array[i].name; i++) {
>
> @@ -148,7 +148,7 @@ ftrace_print_symbols_seq_u64(struct trace_seq *p, unsigned long long val,
> break;
> }
>
> - if (ret == (const char *)(p->buffer + p->len))
> + if (ret == (const char *)(trace_seq_buffer_ptr(p)))
> trace_seq_printf(p, "0x%llx", val);
>
> trace_seq_putc(p, 0);
> @@ -162,7 +162,7 @@ const char *
> ftrace_print_bitmask_seq(struct trace_seq *p, void *bitmask_ptr,
> unsigned int bitmask_size)
> {
> - const char *ret = p->buffer + p->len;
> + const char *ret = trace_seq_buffer_ptr(p);
>
> trace_seq_bitmask(p, bitmask_ptr, bitmask_size * 8);
> trace_seq_putc(p, 0);
> @@ -175,7 +175,7 @@ const char *
> ftrace_print_hex_seq(struct trace_seq *p, const unsigned char *buf, int buf_len)
> {
> int i;
> - const char *ret = p->buffer + p->len;
> + const char *ret = trace_seq_buffer_ptr(p);
>
> for (i = 0; i < buf_len; i++)
> trace_seq_printf(p, "%s%2.2x", i == 0 ? "" : " ", buf[i]);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/