[EXTERNAL] A question about memory order on x86_64

[Liu Xin]

> So they essentially lock out the change while they are writing - which
means when the writing could block indefinitely then the change is also
blocked indefinitely.

Yes,  this kind of synchronization is biased to readers(cheap). updater has
to wait(expensive).
I think it's fair enough, changing log configuration is rare.

> Where is AsyncLogWriter::write participating in this RCU mechanism?

when  logsites enqueue messages to the async buffer, they use RCU counters
as well.

The intention of flush()  is to flush all pending messages, either by
flush() or by async log thread.
_io_sem is a handshake protocol for two stakeholders.  You showed me that
this so-called "protocol" was broken if one thread invokes multiple flush()
in a row.

I changed flush() yesterday.  It just inserts a special token to the buffer
and waits for completion. This guarantees to
serialize multiple flush() calls.  I have run overnight, no problem is
identified.

How about this change?
https://openjdk.github.io/cr/?repo=jdk&pr=4408&range=05#udiff-0-src/hotspot/share/logging/logAsyncWriter.cpp

> I made the change to hold the io_sem for all of write() and the tests
passed in tiers 6 and 7.

What did you change?
P/V operations perform on io_sem(1) once every write(), but I feel it's
difficult to control the scheduler.




On Mon, Jun 21, 2021 at 5:39 PM David Holmes <david.holmes at oracle.com>
wrote:

> On 22/06/2021 9:56 am, Liu, Xin wrote:
> > Hi, David,
> >
> > LogConfiguration::configure_output() is intriguing. That's why I try to
> > explain/comment it in that PR.
> >
> > For sync logging, the RCU reader counter remains > 0 in "writing"
> > period. LogOutputList::Iterator is a RAII class. Its ctor calls
> > increase_readers() and dtor calls decrease_readers().
> >
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/logging/logTagSet.cpp#L77
> >
> > When the users decide to disable a LogOutput, they need to adjust levels
> > first. eg. change tagset 'gc' from DEBUG to OFF. There's a RCU
> > synchronization "wait_until_no_readers()" inside of
> > LogOutputList::update_output_level().
> >
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/logging/logConfiguration.cpp#L241
> >
> > When this function returns, it means that all logsites have seen that
> > tagset's level change and all ongoing "writings" have done because
> > _active_readers was once back to zero. There're 200+ tagsets. Each
> > changed tagset needs a RCU synchronization.
> >
> > void LogOutputList::wait_until_no_readers() const {
> >    OrderAccess::storeload();
> >    while (_active_readers != 0) {
> >      // Busy wait
> >    }
> > }
> >
> > Of course, logsites are still active after wait_until_no_readers(), but
> > they have seen new levels for the tagsets. They won't emit logs to the
> > deleting LogOutput. Therefore, it's safe to delete it.
>
> So they essentially lock out the change while they are writing - which
> means when the writing could block indefinitely then the change is also
> blocked indefinitely:
>
> void LogTagSet::log(LogLevelType level, const char* msg) {
>    LogDecorations decorations(level, *this, _decorators);
>    for (LogOutputList::Iterator it = _output_list.iterator(level); it !=
> _output_list.end(); it++) {
>      (*it)->write(decorations, msg);
>    }
> }
>
> > I would like to make async log a natural extension of current lock-free
> > mechanism. The RCU counters only control "enqueuing" periods instead of
> > "writing" periods. As long as flush() works, it's still correct.
>
> Where is AsyncLogWriter::write participating in this RCU mechanism? The
> AsyncLogMessages that were enqueued, and have now been dequeued ready to
> be written, have already captured their "output", but there is nothing
> to protect those outputs. You can't guarantee that the flush() call
> handles all pre-existing enqueued messages, before that thread deletes
> the output. You would have to ensure that the write() from flush() waits
> for the AsyncLogWriter thread to complete any in-progress write() call
> before the flushing write() proceeds. I think you need to hold the
> io_sem across the entire write() to ensure it is executed atomically (we
> just need to make sure that can't lead to any deadlock scenarios with
> the other locking mechanisms in use.
>
> David
>
> > thanks,
> > --lx
> >
> >
> >
> > On 6/21/21 4:28 PM, David Holmes wrote:
> >> But we can't do that as we could block indefinitely while writing.
> >>
> >> Backing up a step lets ignore async logging for a moment. How does
> >> synchronous logging deal with this problem? What ensures that a log
> >> request can't use an output that is being concurrently deleted by a
> >> configuration change?
>