[ovs-dev] [PATCH] fatal-signal: Skip acquiring log fd lock.
Ilya Maximets
i.maximets at ovn.org
Mon Mar 23 23:13:12 UTC 2020
On 3/23/20 11:34 PM, William Tu wrote:
> On Mon, Mar 23, 2020 at 2:58 PM William Tu <u9012063 at gmail.com> wrote:
>>
>> On Mon, Mar 23, 2020 at 2:34 PM Ben Pfaff <blp at ovn.org> wrote:
>>>
>>> On Mon, Mar 23, 2020 at 02:22:17PM -0700, William Tu wrote:
>>>> Due to not acquiring lock, clang reports:
>>>> lib/vlog.c:618:12: error: reading variable 'log_fd' requires holding mutex
>>>> 'log_file_mutex' [-Werror,-Wthread-safety-analysis]
>>>> return log_fd;
>>>> The patch fixes by skipping it using OVS_NO_THREAD_SAFETY_ANALYSIS.
>>>>
>>>> Tested-at: https://travis-ci.org/github/williamtu/ovs-travis/builds/666051210
>>>> Fixex: ecd4a8fcdff2 ("fatal-signal: Log backtrace when no monitor daemon.")
>>>> Signed-off-by: William Tu <u9012063 at gmail.com>
>>>
>>> You can't just mark the existing send_backtrace_to_monitor()
>>> OVS_NO_THREAD_SAFETY_ANALYSIS? It seems like the easiest change.
>> OK
>> that will also work.
>>>
>>> Also, the OVS_REQUIRES should also go in vlog.h.
>
> Adding 'OVS_REQUIRES(log_file_mutext)' to vlog.h has error like this:
> ./include/openvswitch/vlog.h:147:36: error: use of undeclared
> identifier 'log_file_mutex'
> int vlog_get_fd(void) OVS_REQUIRES(log_file_mutex);
>
> I will have to move log_file_mutex and pattern_rwlock to vlog.h.
> Do we want this?
> Does it make difference if OVS_REQUIRES is only used in C file
> but not in header file?
To avoid exposure of the vlog internals, maybe we could create a new function like:
/*
* This function writes directly to log file without using async writer or
* taking a lock. Caller must hold 'log_file_mutex' or be sure that it's
* not necessary. Could be used in exceptional cases like dumping of backtrace
* on fatal signals.
*/
void
vlog_direct_write_to_log_file_unsafe(const char *s)
OVS_NO_THREAD_SAFETY_ANALYSIS
{
if (log_fd >= 0) {
ignore(write(log_fd, s, strlen(s)));
}
}
William, Ben, WDYT?
Best regards, Ilya Maximets.
More information about the dev
mailing list