lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ