On Tue, Mar 24, 2020 at 12:13:12AM +0100, Ilya Maximets wrote: > On 3/23/20 11:34 PM, William Tu wrote: > > On Mon, Mar 23, 2020 at 2:58 PM William Tu <[email protected]> wrote: > >> > >> On Mon, Mar 23, 2020 at 2:34 PM Ben Pfaff <[email protected]> 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 <[email protected]> > >>> > >>> 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?
I like that solution. Thanks. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
