On Mon, Mar 26, 2018 at 10:23:02AM -0600, Parav Pandit wrote: > > As already discussed I will use the one line version instead of this. > > > > When I first glanced at this patch I thought it was creating a sensible fence in > > cancel, but it turns out it doesn't even do that... > It does by canceled_delayed_work_sync and invoking the callback too. No, there are two bugs in the fencing: 1) It doesn't cancel process_req(), so callbacks can still be invoked from that work after rdma_addr_cancel returns 2) The order of the list_del and the callback in the worker function is backwards and creates a race where rdma_addr_cancel can return without calling canceled_delayed_work_sync resulting in the callback still potentially running In both cases the callback could be running, or could run in the future, even though rdma_addr_cancel returns, meaning it is not a fence. > Regardless, as we discussed, one liner patch looks fine and follow > on patches that we discussed and reviewed in github at [1]. I agree with your note we should now be able to use an un ordered queue, that seems like a nice small win here, especially when triggering all outstanding work from the netdev notifier. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
