Commit f47cb66d authored by Petr Mladek's avatar Petr Mladek Committed by Steven Rostedt
Browse files

ring_buffer: Fix more races when terminating the producer in the benchmark 

The commit b44754d8 ("ring_buffer: Allow to exit the ring
buffer benchmark immediately") added a hack into ring_buffer_producer()
that set @kill_test when kthread_should_stop() returned true. It improved
the situation a lot. It stopped the kthread in most cases because
the producer spent most of the time in the patched while cycle.

But there are still few possible races when kthread_should_stop()
is set outside of the cycle. Then we do not set @kill_test and
some other checks pass.

This patch adds a better fix. It renames @test_kill/TEST_KILL() into
a better descriptive @test_error/TEST_ERROR(). Also it introduces
break_test() function that checks for both @test_error and
kthread_should_stop().

The new function is used in the producer when the check for @test_error
is not enough. It is not used in the consumer because its state
is manipulated by the producer via the "reader_finish" variable.

Also we add a missing check into ring_buffer_producer_thread()
between setting TASK_INTERRUPTIBLE and calling schedule_timeout().
Otherwise, we might miss a wakeup from kthread_stop().

Link: http://lkml.kernel.org/r/1441629518-32712-3-git-send-email-pmladek@suse.com



Signed-off-by: default avatarPetr Mladek <pmladek@suse.com>
Signed-off-by: default avatarSteven Rostedt <rostedt@goodmis.org>
parent 8b46ff69

kernel/trace/ring_buffer_benchmark.c

+29 −25
Original line number Diff line number Diff line
@@ -60,12 +60,12 @@ MODULE_PARM_DESC(consumer_fifo, "fifo prio for consumer"); 


static int read_events;



static int kill_test;

static int test_error;



#define KILL_TEST()				\

#define TEST_ERROR()				\

	do {					\

		if (!kill_test) {		\

			kill_test = 1;		\

		if (!test_error) {		\

			test_error = 1;		\

			WARN_ON(1);		\

		}				\

	} while (0)
@@ -75,6 +75,11 @@ enum event_status { 
	EVENT_DROPPED,

};



static bool break_test(void)

{

	return test_error || kthread_should_stop();

}



static enum event_status read_event(int cpu)

{

	struct ring_buffer_event *event;
@@ -87,7 +92,7 @@ static enum event_status read_event(int cpu) 


	entry = ring_buffer_event_data(event);

	if (*entry != cpu) {

		KILL_TEST();

		TEST_ERROR();

		return EVENT_DROPPED;

	}
@@ -115,10 +120,10 @@ static enum event_status read_page(int cpu) 
		rpage = bpage;

		/* The commit may have missed event flags set, clear them */

		commit = local_read(&rpage->commit) & 0xfffff;

		for (i = 0; i < commit && !kill_test; i += inc) {

		for (i = 0; i < commit && !test_error ; i += inc) {



			if (i >= (PAGE_SIZE - offsetof(struct rb_page, data))) {

				KILL_TEST();

				TEST_ERROR();

				break;

			}
@@ -128,7 +133,7 @@ static enum event_status read_page(int cpu) 
			case RINGBUF_TYPE_PADDING:

				/* failed writes may be discarded events */

				if (!event->time_delta)

					KILL_TEST();

					TEST_ERROR();

				inc = event->array[0] + 4;

				break;

			case RINGBUF_TYPE_TIME_EXTEND:
@@ -137,12 +142,12 @@ static enum event_status read_page(int cpu) 
			case 0:

				entry = ring_buffer_event_data(event);

				if (*entry != cpu) {

					KILL_TEST();

					TEST_ERROR();

					break;

				}

				read++;

				if (!event->array[0]) {

					KILL_TEST();

					TEST_ERROR();

					break;

				}

				inc = event->array[0] + 4;
@@ -150,17 +155,17 @@ static enum event_status read_page(int cpu) 
			default:

				entry = ring_buffer_event_data(event);

				if (*entry != cpu) {

					KILL_TEST();

					TEST_ERROR();

					break;

				}

				read++;

				inc = ((event->type_len + 1) * 4);

			}

			if (kill_test)

			if (test_error)

				break;



			if (inc <= 0) {

				KILL_TEST();

				TEST_ERROR();

				break;

			}

		}
@@ -185,7 +190,7 @@ static void ring_buffer_consumer(void) 
	while (!READ_ONCE(reader_finish)) {

		int found = 1;



		while (found && !kill_test) {

		while (found && !test_error) {

			int cpu;



			found = 0;
@@ -197,7 +202,7 @@ static void ring_buffer_consumer(void) 
				else

					stat = read_page(cpu);



				if (kill_test)

				if (test_error)

					break;



				if (stat == EVENT_FOUND)
@@ -273,10 +278,7 @@ static void ring_buffer_producer(void) 
		if (cnt % wakeup_interval)

			cond_resched();

#endif

		if (kthread_should_stop())

			kill_test = 1;



	} while (ktime_before(end_time, timeout) && !kill_test);

	} while (ktime_before(end_time, timeout) && !break_test());

	trace_printk("End ring buffer hammer\n");



	if (consumer) {
@@ -297,7 +299,7 @@ static void ring_buffer_producer(void) 
	entries = ring_buffer_entries(buffer);

	overruns = ring_buffer_overruns(buffer);



	if (kill_test && !kthread_should_stop())

	if (test_error)

		trace_printk("ERROR!\n");



	if (!disable_reader) {
@@ -378,15 +380,14 @@ static void wait_to_die(void) 


static int ring_buffer_consumer_thread(void *arg)

{

	while (!kthread_should_stop() && !kill_test) {

	while (!break_test()) {

		complete(&read_start);



		ring_buffer_consumer();



		set_current_state(TASK_INTERRUPTIBLE);

		if (kthread_should_stop() || kill_test)

		if (break_test())

			break;



		schedule();

	}

	__set_current_state(TASK_RUNNING);
@@ -399,7 +400,7 @@ static int ring_buffer_consumer_thread(void *arg) 


static int ring_buffer_producer_thread(void *arg)

{

	while (!kthread_should_stop() && !kill_test) {

	while (!break_test()) {

		ring_buffer_reset(buffer);



		if (consumer) {
@@ -408,15 +409,18 @@ static int ring_buffer_producer_thread(void *arg) 
		}



		ring_buffer_producer();

		if (kill_test)

		if (break_test())

			goto out_kill;



		trace_printk("Sleeping for 10 secs\n");

		set_current_state(TASK_INTERRUPTIBLE);

		if (break_test())

			goto out_kill;

		schedule_timeout(HZ * SLEEP_TIME);

	}



out_kill:

	__set_current_state(TASK_RUNNING);

	if (!kthread_should_stop())

		wait_to_die();

CRA Git | Maintained and supported by SUSTech CRA and CCSE