Re: [PATCH net 7/9] rxrpc: Fix I/O thread stop

[David Howells]

Hillf Danton <hdanton@xxxxxxxx> wrote:

> In line with
> 
> 	if (condition)
> 		return;
> 	add to wait queue
> 	if (!condition)
> 		schedule();
> 
> this change should look like
> 
>    		if (!skb_queue_empty(&local->rx_queue) ...)
>  			continue;
> 
>  		if (kthread_should_stop())
>    			if (!skb_queue_empty(&local->rx_queue) ...)
>  				continue;
> 			else
>   				break;
> 
> as checking condition once barely makes sense.

Really, no.  The condition is going to expand to have a whole bunch of things
in it and I don't want to have it twice, e.g.:

                if (!skb_queue_empty(&local->rx_queue) ||
                   READ_ONCE(local->events) ||
                    !list_empty(&local->call_attend_q) ||
                    !list_empty(&local->conn_attend_q) ||
                    !list_empty(&local->new_client_calls) ||
		    test_bit(RXRPC_CLIENT_CONN_REAP_TIMER,
			     &local->client_conn_flags)) {
			...

Hmmm...  I wonder if kthread_should_stop() needs a barrier associated with
it.  It's just a test_bit(), so the compiler can cache the results of all
these tests - or reorder them.

David