On Fri, Aug 31, 2012 at 1:35 AM, Roland Dreier <[email protected]> wrote: > On Thu, Aug 30, 2012 at 3:17 PM, Or Gerlitz <[email protected]> wrote:
>> 1. on the time CQ A is deleted an interrupt that relates to CQ B >> takes place and a radix >> tree lookup is running while an element is being deleted from the >> tree, looking on the radix tree API, I don't see that this is allowed. > > I don't think this is a real problem; the radix tree code is > explicitly designed for RCU use, and the data structure is pretty > clearly safe for looking up one slot while another slot is being > cleared. In fact it's hard to see how this could screw up. OK, I'll give it a 2nd look, thanks for elaborating on the design, one thing which probably confused us was the driver cq event callback which does take the lock before searching the radix tree, and increase the refcount before invoking the user event handler, any reason for the driver event callback to use different practice vs. the comp one? >> 2. while a CQ is being freed an interrupt takes place and the driver >> attempts to run the comp handler which can turn to use after free, >> null pointer deref, etc. This can happen even if the ULP made sure to >> consume all the WCs related to flushed/etc, e.g an "empty" interrupt > So in mlx4_cq_free() we do > mlx4_HW2SW_CQ(dev, NULL, cq->cqn); > //... > synchronize_irq(priv->eq_table.eq[cq->vector].irq); > > before we touch the cq table. I don't think we should get a CQ > completion event for the CQ we're freeing after we've done HW2SW_CQ on > it and then waited for any outstanding completion interrupts to finish. yes, makes sense, this wouldn't handle use cases where the user does context switch, e.g from hard_irq to softirq/tasklet and let their handler touch the CQ, but the refcount in the driver wouldn't help either in that case. > Also we know that there are no QPs attached to this CQ so there > shouldn't be any completion events anyway... All to all, your reasoning makes much sense, we probably need to look deeper into the crash report that triggered this RFC, see next email. Yishai - were you thinking on other possible races that the patch could address? also, can you double check the point Roland made on HW2SW_CQ. Or. Or. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
