On 17/11/2021 18:50, Yann Ylavic wrote:
On Wed, Nov 17, 2021 at 5:19 PM Mladen Turk <mt...@apache.org> wrote:

On 17/11/2021 16:53, Yann Ylavic wrote:

apr_poll_drain_wakeup_pipe() should consume each byte sent on the pipe
corresponding to a wakeup_set flip.


Yes, its basically just one byte and one call to the apr_file_getc.
The 'while (apr_atomic_cas32(wakeup_set, 0, 1) > 0)' is there
because some thread might call apr_pollset_wakeup while OS is
inside write call. Anyhow, this ensures everything is pulled
without blocking since non zero wakeup_set means there is
something to read.

Possibly we could use lighter atomics to achieve the same,something
like the attached patch.
Still the compare-and-swap in apr_pollset_wakeup() to wakeup_set only
once, but apr_poll_drain_wakeup_pipe() does now unconditionally
consume one byte of the pipe and reset wakeup_set just after with an
atomic set.
Since we reach apr_poll_drain_wakeup_pipe() only when the system's
poll() triggers on the pipe, there is something on the pipe to read
(and being non-blocking is not an issue either).
If a new wakeup happens just after apr_poll_drain_wakeup_pipe() resets
wakeup_set, well it's for the next turn (no different than with the
current version..).

The advantage here is that we'd have a single "real" atomic operation
now for the wakeup (the atomic cas), draining would be as light as an
atomic set (i.e. a simple volatile access on platforms with non-weak
memory ordering, i.e. most).

WDYT?


Dunno

If only a single thread can call apr_poll_drain_wakeup_pipe
it shoud be OK. However I'd rather check if there is
something to read before calling apr_file_getc since it'll block.


Regards
--
^TM

Reply via email to