On Thu, May 07, 2026 at 03:37:54PM +0100, David Laight wrote:
> On Thu, 7 May 2026 13:31:24 +0200
> Paolo Abeni <[email protected]> wrote:
>
> > On 5/5/26 10:42 AM, Adrian Moreno wrote:
> ..
> > > +/* Must be called with flow_table->lock held. */
> > > int ovs_flow_tbl_flush(struct flow_table *flow_table)
> > > {
> > > struct table_instance *old_ti, *new_ti;
> > > struct table_instance *old_ufid_ti, *new_ufid_ti;
> > >
> > > + ASSERT_OVS_TBL(flow_table);
> >
> > Minor nit: adding the assert and the comment is redundant. I think the
> > assert alone would be better. There are other similar later occurrences.
>
Yeah, this file (and datapath.c) seem to rely heavily on comments to
clarify locking requirements. I can add a follow-up patch converting all
comments to these kind of warnings.
> There is no point adding an ASSERT() for a pointer being NULL.
> The NULL pointer dereference does the same job and can be easier to
> debug because all the registers are still live.
>
This asserts for the mutex being held:
#define ASSERT_OVS_TBL(tbl) WARN_ON(!lockdep_ovs_tbl_is_held(tbl))
maybe the macro should be renamed to make this clearer?
Adrián
> -- David
>
> >
> > /P
> >
> >
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
