[<prev] [next>] [day] [month] [year] [list]
Message-ID: <1003225.1671393594@warthog.procyon.org.uk>
Date: Sun, 18 Dec 2022 19:59:54 +0000
From: David Howells <dhowells@...hat.com>
To: Hillf Danton <hdanton@...a.com>
Cc: dhowells@...hat.com, netdev@...r.kernel.org,
Marc Dionne <marc.dionne@...istor.com>,
linux-afs@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net 7/9] rxrpc: Fix I/O thread stop
Hillf Danton <hdanton@...a.com> 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
Powered by blists - more mailing lists