On Mon, Mar 23, 2020 at 4:13 PM Ilya Maximets <[email protected]> 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?
>
Hi Ilya,
I think this is a good idea, let me work on it.
Thanks
William
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
