On Wed, Oct 02, 2019 at 06:41:14PM +0300, Leon Romanovsky wrote: > On Wed, Oct 02, 2019 at 11:39:28AM -0300, Jason Gunthorpe wrote: > > On Wed, Oct 02, 2019 at 11:18:26AM +0300, Leon Romanovsky wrote: > > > > @@ -202,15 +225,22 @@ static void mr_leaf_free_action(struct work_struct *work) > > > > struct ib_umem_odp *odp = container_of(work, struct ib_umem_odp, work); > > > > int idx = ib_umem_start(odp) >> MLX5_IMR_MTT_SHIFT; > > > > struct mlx5_ib_mr *mr = odp->private, *imr = mr->parent; > > > > + struct ib_umem_odp *odp_imr = to_ib_umem_odp(imr->umem); > > > > + int srcu_key; > > > > > > > > mr->parent = NULL; > > > > synchronize_srcu(&mr->dev->mr_srcu); > > > > > > Are you sure that this line is still needed? > > > > Yes, in this case the mr->parent is the SRCU 'update' and it blocks > > seeing this MR in the pagefault handler. > > > > It is necessary before calling ib_umem_odp_release below that frees > > the memory > > sorry for not being clear, I thought that synchronize_srcu() should be > moved after your read_lock/unlock additions to reuse grace period. It has to be before to ensure that the page fault handler does not undo the invaliate below with new pages Jason
