Messages in this thread |  | | From | Adrián Moreno <> | | Date | Wed, 13 May 2026 10:54:36 +0000 | | Subject | Re: [PATCH net-next v3 2/2] net: openvswitch: decouple flow_table from ovs_mutex |
| |
On Thu, May 07, 2026 at 03:37:54PM +0100, David Laight wrote: > On Thu, 7 May 2026 13:31:24 +0200 > Paolo Abeni <pabeni@redhat.com> 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 > > > > >
|  |