On Thu, Oct 15, 2015 at 8:53 PM, Jason Gunthorpe <jgunthorpe@xxxxxxxxxxxxxxxxxxxx> wrote: > On Thu, Oct 15, 2015 at 01:37:22PM -0400, Doug Ledford wrote: > >> Now, you've put an rcu_read_lock on ndev instead. And you're no longer >> seeing the race. However, does taking the rcu_read_lock on ndev >> actually protect the ifa list on ndev > > It looks OK, the free side for using call_rcu - though I'm not quite > sure why for_ifa isn't using rcu_dereference... > I was wondering the same regarding the rcu_dereference. >> And even if the rcu_read_lock is for sure safe in terms of accessing the >> ifa list, these changes may have just introduced a totally new bug that >> your QE tests haven't exposed but might exist none the less. > > This event driven design is super complex/frail, if it actually > doesn't work, it should probably be changed to a level based system. > > - Record incoming events indicating a change in ip list has happened > for an ib device, Schedule a task to refresh the list > - Refresh task holds rtnl locks and builds a list of IP addresses associated > with the ib device. This is atomic with user changes to the IP list. > - Drops locks and then directly synchs the discovered list above with > the device's record of associated IPs. (ie run set intersection) > > It is much less subtle than hoping all the async event driven stuff > in here works out, probably a lot less code too. > It's simpler, but every IP change could make us refresh the whole list and possibly the GID indices as well. > Jason > -- Matan > 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 -- 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
