<p><a href="https://gerrit.osmocom.org/c/libosmocore/+/20296">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/20296/4/src/logging.c">File src/logging.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/20296/4/src/logging.c@869">Patch Set #4, Line 869:</a> <code style="font-family:monospace,monospace">   osmo_wqueue_enqueue_quiet(target->tgt_file.wqueue, msg);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">here it fails if wqueue is too full right? What about manually introducing a last msgb with content  […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I have a follow-up patch that introduces a rate_counter for overruns.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Unconditionally adding a "last" msgb means that we would have to rate-limit those somehow.  And if we e.g. are in a 'disk full' situation, none of the messages in the queue would ever be written, including that 'last message'.</p><p style="white-space: pre-wrap; word-wrap: break-word;">There's not really any "good" solution, so I think the rate_counter is the best we can do for now.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/20296/4/src/logging.c@926">Patch Set #4, Line 926:</a> <code style="font-family:monospace,monospace">     if (rc != msgb_length(msg))</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Here, upon short write (possible if we are in NONBLOCK afaiu) instead of returning error and losing  […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">short writes are possible, but rather unlikely. We always only write one log line/msgb every time we get out of select().  So this would mean that the kernel tells us the fd is write-able, but then we cannot even write the (few hundreds of bytes) log line.  So unless we will have dozens-of-kilobytes in one log statement, I think the situation is a theoretical one.</p><p style="white-space: pre-wrap; word-wrap: break-word;">But yes,we can re-enqueue the remainder of the msgb in such a situation.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/20296/4/src/select.c">File src/select.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/20296/4/src/select.c@283">Patch Set #4, Line 283:</a> <code style="font-family:monospace,monospace">        log_target_file_switch_to_wqueue(osmo_stderr_target);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Fine with doing this by default, but it may make sense to have a VTY command to set a flag to avoid  […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I would avoid adding more and more flags for use cases that we don't have yet.  I think in the current code it's rather simple: non-blocking I/O if osmo_select_main() is used, and blocking I/O if it isn't (or as long as it isn't).</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/20296">change 20296</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/libosmocore/+/20296"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ia58fd78535c41b3da3aeb7733aadc785ace610da </div>
<div style="display:none"> Gerrit-Change-Number: 20296 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-CC: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Sun, 27 Sep 2020 14:06:49 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>