[ltt-dev] [RFC UST PATCH] Code base to fix the print errors in UST

[ltt-dev] [RFC UST PATCH] Code base to fix the print errors in UST

[David Goulet]

The last problem is the commit_count in ust_buffer. It's not
allocated in the shared memory thus cannot be access via
the buffer_info struct.

So, with this code, the consumerd segfault when accessing
these counters.
---
 include/ust/ustconsumer.h       |    4 ++
 libust/buffers.c                |   80 +--------------------------------------
 libustconsumer/libustconsumer.c |    4 ++
 libustconsumer/lowlevel.c       |   64 +++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+), 79 deletions(-)

diff --git a/include/ust/ustconsumer.h b/include/ust/ustconsumer.h
index e07b75e..f99f8f1 100644
--- a/include/ust/ustconsumer.h
+++ b/include/ust/ustconsumer.h
@@ -57,6 +57,10 @@ struct buffer_info {
 	int n_subbufs;
 	/* size of each subbuffer */
 	int subbuf_size;
+	/* subbuf size count order */
+	int subbuf_size_order;
+	/* alloc size of all subbuf */
+	int alloc_size;
 
 	/* the buffer information struct */
 	void *bufstruct_mem;
diff --git a/libust/buffers.c b/libust/buffers.c
index 4011b64..ba293f4 100644
--- a/libust/buffers.c
+++ b/libust/buffers.c
@@ -254,8 +254,6 @@ unmap_buf:
 	return -1;
 }
 
-static void ltt_relay_print_buffer_errors(struct ust_channel *chan, int cpu);
-
 static void close_buf(struct ust_buffer *buf)
 {
 	struct ust_channel *chan = buf->chan;
@@ -267,7 +265,7 @@ static void close_buf(struct ust_buffer *buf)
 		PERROR("shmdt");
 	}
 
-	free(buf->commit_count);
+	//free(buf->commit_count);
 
 	result = close(buf->data_ready_fd_read);
 	if (result < 0) {
@@ -278,9 +276,6 @@ static void close_buf(struct ust_buffer *buf)
 	if (result < 0 && errno != EBADF) {
 		PERROR("close");
 	}
-
-	/* FIXME: This spews out errors, are they real?:
-	 * ltt_relay_print_buffer_errors(chan, cpu); */
 }
 
 
@@ -517,78 +512,6 @@ int ust_buffers_put_subbuf(struct ust_buffer *buf, unsigned long uconsumed_old)
 	return 0;
 }
 
-static void ltt_relay_print_subbuffer_errors(
-		struct ust_channel *channel,
-		long cons_off, int cpu)
-{
-	struct ust_buffer *ltt_buf = channel->buf[cpu];
-	long cons_idx, commit_count, commit_count_sb, write_offset;
-
-	cons_idx = SUBBUF_INDEX(cons_off, channel);
-	commit_count = uatomic_read(&ltt_buf->commit_count[cons_idx].cc);
-	commit_count_sb = uatomic_read(&ltt_buf->commit_count[cons_idx].cc_sb);
-
-	/*
-	 * No need to order commit_count and write_offset reads because we
-	 * execute after trace is stopped when there are no readers left.
-	 */
-	write_offset = uatomic_read(&ltt_buf->offset);
-	WARN( "LTT : unread channel %s offset is %ld "
-		"and cons_off : %ld (cpu %d)\n",
-		channel->channel_name, write_offset, cons_off, cpu);
-	/* Check each sub-buffer for non filled commit count */
-	if (((commit_count - channel->subbuf_size) & channel->commit_count_mask)
-	    - (BUFFER_TRUNC(cons_off, channel) >> channel->n_subbufs_order) != 0) {
-		ERR("LTT : %s : subbuffer %lu has non filled "
-			"commit count [cc, cc_sb] [%lu,%lu].\n",
-			channel->channel_name, cons_idx, commit_count, commit_count_sb);
-	}
-	ERR("LTT : %s : commit count : %lu, subbuf size %zd\n",
-			channel->channel_name, commit_count,
-			channel->subbuf_size);
-}
-
-static void ltt_relay_print_errors(struct ust_trace *trace,
-		struct ust_channel *channel, int cpu)
-{
-	struct ust_buffer *ltt_buf = channel->buf[cpu];
-	long cons_off;
-
-	/*
-	 * Can be called in the error path of allocation when
-	 * trans_channel_data is not yet set.
-	 */
-	if (!channel)
-	        return;
-
-//ust//	for (cons_off = 0; cons_off < rchan->alloc_size;
-//ust//	     cons_off = SUBBUF_ALIGN(cons_off, rchan))
-//ust//		ust_buffers_print_written(ltt_chan, cons_off, cpu);
-	for (cons_off = uatomic_read(&ltt_buf->consumed);
-			(SUBBUF_TRUNC(uatomic_read(&ltt_buf->offset),
-				      channel)
-			 - cons_off) > 0;
-			cons_off = SUBBUF_ALIGN(cons_off, channel))
-		ltt_relay_print_subbuffer_errors(channel, cons_off, cpu);
-}
-
-static void ltt_relay_print_buffer_errors(struct ust_channel *channel, int cpu)
-{
-	struct ust_trace *trace = channel->trace;
-	struct ust_buffer *ltt_buf = channel->buf[cpu];
-
-	if (uatomic_read(&ltt_buf->events_lost))
-		ERR("channel %s: %ld events lost (cpu %d)",
-			channel->channel_name,
-			uatomic_read(&ltt_buf->events_lost), cpu);
-	if (uatomic_read(&ltt_buf->corrupted_subbuffers))
-		ERR("channel %s : %ld corrupted subbuffers (cpu %d)",
-			channel->channel_name,
-			uatomic_read(&ltt_buf->corrupted_subbuffers), cpu);
-
-	ltt_relay_print_errors(trace, channel, cpu);
-}
-
 static int map_buf_structs(struct ust_channel *chan)
 {
 	void *ptr;
@@ -721,7 +644,6 @@ static void remove_channel(struct ust_channel *chan)
 	free(chan->buf_struct_shmids);
 
 	free(chan->buf);
-
 }
 
 static void ltt_relay_async_wakeup_chan(struct ust_channel *ltt_channel)
diff --git a/libustconsumer/libustconsumer.c b/libustconsumer/libustconsumer.c
index c51b106..ef54fe8 100644
--- a/libustconsumer/libustconsumer.c
+++ b/libustconsumer/libustconsumer.c
@@ -353,6 +353,10 @@ struct buffer_info *connect_buffer(struct ustconsumer_instance *instance, pid_t
 		goto close_fifo;
 	}
 
+	/* Set subbuffer's information */
+	buf->subbuf_size_order = get_count_order(buf->subbuf_size);
+	buf->alloc_size = buf->subbuf_size * buf->n_subbufs;
+
 	/* attach memory */
 	buf->mem = shmat(buf->shmid, NULL, 0);
 	if(buf->mem == (void *) 0) {
diff --git a/libustconsumer/lowlevel.c b/libustconsumer/lowlevel.c
index 730dd11..a65ae4d 100644
--- a/libustconsumer/lowlevel.c
+++ b/libustconsumer/lowlevel.c
@@ -31,6 +31,67 @@
 #define LTT_MAGIC_NUMBER 0x00D6B7ED
 #define LTT_REV_MAGIC_NUMBER 0xEDB7D600
 
+
+static void ltt_relay_print_subbuffer_errors(
+		struct buffer_info *buf,
+		long cons_off, int cpu)
+{
+	struct ust_buffer *ust_buf = buf->bufstruct_mem;
+	long cons_idx, commit_count, commit_count_sb, commit_count_mask, write_offset;
+
+	cons_idx = SUBBUF_INDEX(cons_off, buf);
+	commit_count = uatomic_read(&ust_buf->commit_count[cons_idx].cc);
+	commit_count_sb = uatomic_read(&ust_buf->commit_count[cons_idx].cc_sb);
+	commit_count_mask = (~0UL >> get_count_order(buf->n_subbufs));
+
+	/*
+	 * No need to order commit_count and write_offset reads because we
+	 * execute after trace is stopped when there are no readers left.
+	 */
+	write_offset = uatomic_read(&ust_buf->offset);
+	WARN( "LTT : unread channel %s offset is %ld "
+			"and cons_off : %ld (cpu %d)\n",
+			buf->channel, write_offset, cons_off, cpu);
+	/* Check each sub-buffer for non filled commit count */
+	if (((commit_count - buf->subbuf_size) & commit_count_mask)
+			- (BUFFER_TRUNC(cons_off, buf) >> get_count_order(buf->n_subbufs)) != 0) {
+		ERR("LTT : %s : subbuffer %lu has non filled "
+				"commit count [cc, cc_sb] [%lu,%lu].\n",
+				buf->channel, cons_idx, commit_count, commit_count_sb);
+	}
+	ERR("LTT : %s : commit count : %lu, subbuf size %d\n",
+			buf->channel, commit_count,
+			buf->subbuf_size);
+}
+
+static void ltt_relay_print_errors(struct buffer_info *buf, int cpu)
+{
+	struct ust_buffer *ust_buf = buf->bufstruct_mem;
+	long cons_off;
+
+	for (cons_off = uatomic_read(&ust_buf->consumed);
+			(SUBBUF_TRUNC(uatomic_read(&ust_buf->offset), buf)
+				- cons_off) > 0;
+			cons_off = SUBBUF_ALIGN(cons_off, buf))
+		ltt_relay_print_subbuffer_errors(buf, cons_off, cpu);
+}
+
+static void ltt_relay_print_buffer_errors(struct buffer_info *buf, int cpu)
+{
+	struct ust_buffer *ust_buf = buf->bufstruct_mem;
+
+	if (uatomic_read(&ust_buf->events_lost))
+		ERR("channel %s: %ld events lost (cpu %d)",
+				buf->channel,
+				uatomic_read(&ust_buf->events_lost), cpu);
+	if (uatomic_read(&ust_buf->corrupted_subbuffers))
+		ERR("channel %s : %ld corrupted subbuffers (cpu %d)",
+				buf->channel,
+				uatomic_read(&ust_buf->corrupted_subbuffers), cpu);
+
+	ltt_relay_print_errors(buf, cpu);
+}
+
 /* Returns the size of a subbuffer size. This is the size that
  * will need to be written to disk.
  *
@@ -86,6 +147,9 @@ void finish_consuming_dead_subbuffer(struct ustconsumer_callbacks *callbacks, st
 	DBG("first_subbuf=%ld", first_subbuf);
 	DBG("last_subbuf=%ld", last_subbuf);
 
+	printf("PRINT ERRORS\n");
+	ltt_relay_print_buffer_errors(buf, buf->channel_cpu);
+
 	if(last_subbuf - first_subbuf >= buf->n_subbufs) {
 		DBG("an overflow has occurred, nothing can be recovered");
 		return;
-- 
1.7.4.1

[ltt-dev] [RFC UST PATCH] Code base to fix the print errors in UST

[Mathieu Desnoyers]

* David Goulet (david.goulet at polymtl.ca) wrote:
> The last problem is the commit_count in ust_buffer. It's not
> allocated in the shared memory thus cannot be access via
> the buffer_info struct.
> 
> So, with this code, the consumerd segfault when accessing
> these counters.

replace "commit_count" by "commit_seq" in the error printing function
and everything should work fine. "commit_seq" is exported (it's at the
end of the struct buf, as a variable-length array), and is what we
should use.

Thanks,

Mathieu

> ---
>  include/ust/ustconsumer.h       |    4 ++
>  libust/buffers.c                |   80 +--------------------------------------
>  libustconsumer/libustconsumer.c |    4 ++
>  libustconsumer/lowlevel.c       |   64 +++++++++++++++++++++++++++++++
>  4 files changed, 73 insertions(+), 79 deletions(-)
> 
> diff --git a/include/ust/ustconsumer.h b/include/ust/ustconsumer.h
> index e07b75e..f99f8f1 100644
> --- a/include/ust/ustconsumer.h
> +++ b/include/ust/ustconsumer.h
> @@ -57,6 +57,10 @@ struct buffer_info {
>  	int n_subbufs;
>  	/* size of each subbuffer */
>  	int subbuf_size;
> +	/* subbuf size count order */
> +	int subbuf_size_order;
> +	/* alloc size of all subbuf */
> +	int alloc_size;
>  
>  	/* the buffer information struct */
>  	void *bufstruct_mem;
> diff --git a/libust/buffers.c b/libust/buffers.c
> index 4011b64..ba293f4 100644
> --- a/libust/buffers.c
> +++ b/libust/buffers.c
> @@ -254,8 +254,6 @@ unmap_buf:
>  	return -1;
>  }
>  
> -static void ltt_relay_print_buffer_errors(struct ust_channel *chan, int cpu);
> -
>  static void close_buf(struct ust_buffer *buf)
>  {
>  	struct ust_channel *chan = buf->chan;
> @@ -267,7 +265,7 @@ static void close_buf(struct ust_buffer *buf)
>  		PERROR("shmdt");
>  	}
>  
> -	free(buf->commit_count);
> +	//free(buf->commit_count);
>  
>  	result = close(buf->data_ready_fd_read);
>  	if (result < 0) {
> @@ -278,9 +276,6 @@ static void close_buf(struct ust_buffer *buf)
>  	if (result < 0 && errno != EBADF) {
>  		PERROR("close");
>  	}
> -
> -	/* FIXME: This spews out errors, are they real?:
> -	 * ltt_relay_print_buffer_errors(chan, cpu); */
>  }
>  
>  
> @@ -517,78 +512,6 @@ int ust_buffers_put_subbuf(struct ust_buffer *buf, unsigned long uconsumed_old)
>  	return 0;
>  }
>  
> -static void ltt_relay_print_subbuffer_errors(
> -		struct ust_channel *channel,
> -		long cons_off, int cpu)
> -{
> -	struct ust_buffer *ltt_buf = channel->buf[cpu];
> -	long cons_idx, commit_count, commit_count_sb, write_offset;
> -
> -	cons_idx = SUBBUF_INDEX(cons_off, channel);
> -	commit_count = uatomic_read(&ltt_buf->commit_count[cons_idx].cc);
> -	commit_count_sb = uatomic_read(&ltt_buf->commit_count[cons_idx].cc_sb);
> -
> -	/*
> -	 * No need to order commit_count and write_offset reads because we
> -	 * execute after trace is stopped when there are no readers left.
> -	 */
> -	write_offset = uatomic_read(&ltt_buf->offset);
> -	WARN( "LTT : unread channel %s offset is %ld "
> -		"and cons_off : %ld (cpu %d)\n",
> -		channel->channel_name, write_offset, cons_off, cpu);
> -	/* Check each sub-buffer for non filled commit count */
> -	if (((commit_count - channel->subbuf_size) & channel->commit_count_mask)
> -	    - (BUFFER_TRUNC(cons_off, channel) >> channel->n_subbufs_order) != 0) {
> -		ERR("LTT : %s : subbuffer %lu has non filled "
> -			"commit count [cc, cc_sb] [%lu,%lu].\n",
> -			channel->channel_name, cons_idx, commit_count, commit_count_sb);
> -	}
> -	ERR("LTT : %s : commit count : %lu, subbuf size %zd\n",
> -			channel->channel_name, commit_count,
> -			channel->subbuf_size);
> -}
> -
> -static void ltt_relay_print_errors(struct ust_trace *trace,
> -		struct ust_channel *channel, int cpu)
> -{
> -	struct ust_buffer *ltt_buf = channel->buf[cpu];
> -	long cons_off;
> -
> -	/*
> -	 * Can be called in the error path of allocation when
> -	 * trans_channel_data is not yet set.
> -	 */
> -	if (!channel)
> -	        return;
> -
> -//ust//	for (cons_off = 0; cons_off < rchan->alloc_size;
> -//ust//	     cons_off = SUBBUF_ALIGN(cons_off, rchan))
> -//ust//		ust_buffers_print_written(ltt_chan, cons_off, cpu);
> -	for (cons_off = uatomic_read(&ltt_buf->consumed);
> -			(SUBBUF_TRUNC(uatomic_read(&ltt_buf->offset),
> -				      channel)
> -			 - cons_off) > 0;
> -			cons_off = SUBBUF_ALIGN(cons_off, channel))
> -		ltt_relay_print_subbuffer_errors(channel, cons_off, cpu);
> -}
> -
> -static void ltt_relay_print_buffer_errors(struct ust_channel *channel, int cpu)
> -{
> -	struct ust_trace *trace = channel->trace;
> -	struct ust_buffer *ltt_buf = channel->buf[cpu];
> -
> -	if (uatomic_read(&ltt_buf->events_lost))
> -		ERR("channel %s: %ld events lost (cpu %d)",
> -			channel->channel_name,
> -			uatomic_read(&ltt_buf->events_lost), cpu);
> -	if (uatomic_read(&ltt_buf->corrupted_subbuffers))
> -		ERR("channel %s : %ld corrupted subbuffers (cpu %d)",
> -			channel->channel_name,
> -			uatomic_read(&ltt_buf->corrupted_subbuffers), cpu);
> -
> -	ltt_relay_print_errors(trace, channel, cpu);
> -}
> -
>  static int map_buf_structs(struct ust_channel *chan)
>  {
>  	void *ptr;
> @@ -721,7 +644,6 @@ static void remove_channel(struct ust_channel *chan)
>  	free(chan->buf_struct_shmids);
>  
>  	free(chan->buf);
> -
>  }
>  
>  static void ltt_relay_async_wakeup_chan(struct ust_channel *ltt_channel)
> diff --git a/libustconsumer/libustconsumer.c b/libustconsumer/libustconsumer.c
> index c51b106..ef54fe8 100644
> --- a/libustconsumer/libustconsumer.c
> +++ b/libustconsumer/libustconsumer.c
> @@ -353,6 +353,10 @@ struct buffer_info *connect_buffer(struct ustconsumer_instance *instance, pid_t
>  		goto close_fifo;
>  	}
>  
> +	/* Set subbuffer's information */
> +	buf->subbuf_size_order = get_count_order(buf->subbuf_size);
> +	buf->alloc_size = buf->subbuf_size * buf->n_subbufs;
> +
>  	/* attach memory */
>  	buf->mem = shmat(buf->shmid, NULL, 0);
>  	if(buf->mem == (void *) 0) {
> diff --git a/libustconsumer/lowlevel.c b/libustconsumer/lowlevel.c
> index 730dd11..a65ae4d 100644
> --- a/libustconsumer/lowlevel.c
> +++ b/libustconsumer/lowlevel.c
> @@ -31,6 +31,67 @@
>  #define LTT_MAGIC_NUMBER 0x00D6B7ED
>  #define LTT_REV_MAGIC_NUMBER 0xEDB7D600
>  
> +
> +static void ltt_relay_print_subbuffer_errors(
> +		struct buffer_info *buf,
> +		long cons_off, int cpu)
> +{
> +	struct ust_buffer *ust_buf = buf->bufstruct_mem;
> +	long cons_idx, commit_count, commit_count_sb, commit_count_mask, write_offset;
> +
> +	cons_idx = SUBBUF_INDEX(cons_off, buf);
> +	commit_count = uatomic_read(&ust_buf->commit_count[cons_idx].cc);
> +	commit_count_sb = uatomic_read(&ust_buf->commit_count[cons_idx].cc_sb);
> +	commit_count_mask = (~0UL >> get_count_order(buf->n_subbufs));
> +
> +	/*
> +	 * No need to order commit_count and write_offset reads because we
> +	 * execute after trace is stopped when there are no readers left.
> +	 */
> +	write_offset = uatomic_read(&ust_buf->offset);
> +	WARN( "LTT : unread channel %s offset is %ld "
> +			"and cons_off : %ld (cpu %d)\n",
> +			buf->channel, write_offset, cons_off, cpu);
> +	/* Check each sub-buffer for non filled commit count */
> +	if (((commit_count - buf->subbuf_size) & commit_count_mask)
> +			- (BUFFER_TRUNC(cons_off, buf) >> get_count_order(buf->n_subbufs)) != 0) {
> +		ERR("LTT : %s : subbuffer %lu has non filled "
> +				"commit count [cc, cc_sb] [%lu,%lu].\n",
> +				buf->channel, cons_idx, commit_count, commit_count_sb);
> +	}
> +	ERR("LTT : %s : commit count : %lu, subbuf size %d\n",
> +			buf->channel, commit_count,
> +			buf->subbuf_size);
> +}
> +
> +static void ltt_relay_print_errors(struct buffer_info *buf, int cpu)
> +{
> +	struct ust_buffer *ust_buf = buf->bufstruct_mem;
> +	long cons_off;
> +
> +	for (cons_off = uatomic_read(&ust_buf->consumed);
> +			(SUBBUF_TRUNC(uatomic_read(&ust_buf->offset), buf)
> +				- cons_off) > 0;
> +			cons_off = SUBBUF_ALIGN(cons_off, buf))
> +		ltt_relay_print_subbuffer_errors(buf, cons_off, cpu);
> +}
> +
> +static void ltt_relay_print_buffer_errors(struct buffer_info *buf, int cpu)
> +{
> +	struct ust_buffer *ust_buf = buf->bufstruct_mem;
> +
> +	if (uatomic_read(&ust_buf->events_lost))
> +		ERR("channel %s: %ld events lost (cpu %d)",
> +				buf->channel,
> +				uatomic_read(&ust_buf->events_lost), cpu);
> +	if (uatomic_read(&ust_buf->corrupted_subbuffers))
> +		ERR("channel %s : %ld corrupted subbuffers (cpu %d)",
> +				buf->channel,
> +				uatomic_read(&ust_buf->corrupted_subbuffers), cpu);
> +
> +	ltt_relay_print_errors(buf, cpu);
> +}
> +
>  /* Returns the size of a subbuffer size. This is the size that
>   * will need to be written to disk.
>   *
> @@ -86,6 +147,9 @@ void finish_consuming_dead_subbuffer(struct ustconsumer_callbacks *callbacks, st
>  	DBG("first_subbuf=%ld", first_subbuf);
>  	DBG("last_subbuf=%ld", last_subbuf);
>  
> +	printf("PRINT ERRORS\n");
> +	ltt_relay_print_buffer_errors(buf, buf->channel_cpu);
> +
>  	if(last_subbuf - first_subbuf >= buf->n_subbufs) {
>  		DBG("an overflow has occurred, nothing can be recovered");
>  		return;
> -- 
> 1.7.4.1
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com