On Fri, 2022-10-14 at 23:06 +0200, Jan Hoffmann wrote: > These two functions are identical apart from writing different values to > the read/write bit. Create a new function rtl_table_exec to reduce code > duplication. > > Also replace the unbounded busy-waiting loop. As the hardware usually > responds very quickly, always call cond_resched, so that callers do not > need to worry about blocking for too long. This is especially important > for the L2 table, as a full loop (as in rtl83xx_port_fdb_dump) takes > about 20ms on RTL839x (and that function actually ends up being called > in a loop for every port). > > So far, polling timeout errors are only handled by logging an error, but > a return value is added to allow proper handling in the future. > > Signed-off-by: Jan Hoffmann <[email protected]> > --- > > I'm not really sure if putting cond_resched in this place is really the > best solution. An alternative would be to place it directly in the loops > (i.e. rtl83xx_port_fdb_dump and l2_table_show) instead.
I'm no expert on this, but it does appear to be more common to call cond_resched() from loops. If rtl_table_exec() once is not an issue, but calling it repeatedly is, then it would make more sense to me to put this in the loops. If anyone want to argue otherwise, I'd be happy to hear so though. Otherwise this patch looks good to me. Putting upper bounds on loop durations, however unlikely to fail, is not something I'm going to refuse :) > > About error handling: Actually implementing that for all calls is going > to require large changes. And doing it in a proper way which is better > than just printing an error (i.e. actually trying to leave a consistent > state) is non-trivial. I started to work on that only for fdb/mdb > access, but that is only a part of where these tables are used: > > https://github.com/janh/openwrt/commit/e45403299208d49675a0403ad94349ebdff2a374 > https://github.com/janh/openwrt/commit/fa68b23cab542683832696f039cf160b96da83db > https://github.com/janh/openwrt/commit/92abfc2e4925e5bc1a0e25e464ca1d08833c9b30 Thanks for your work on improving this driver! Best, Sander _______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
