On Fri, Jan 04, 2019 at 07:12:54PM +0200, Leon Romanovsky wrote:
> On Fri, Jan 04, 2019 at 04:23:49PM +0000, Jason Gunthorpe wrote:
> > On Fri, Jan 04, 2019 at 08:05:06AM +0200, Leon Romanovsky wrote:
> > > On Thu, Jan 03, 2019 at 10:30:26PM +0000, Jason Gunthorpe wrote:
> > > > On Thu, Jan 03, 2019 at 10:05:59AM +0200, Leon Romanovsky wrote:
> > > > > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > > >
> > > > > XArray uses internal lock for updates to XArray. This means that our
> > > > > external RW lock is not needed anymore and we can delete it.
> > > >
> > > > The xarray lock only protects the array itself - not the things it
> > > > points to.
> > > >
> > > > > - down_read(&res->rwsem);
> > > > > xa_for_each(&res->xa[type], e, index, ULONG_MAX, XA_PRESENT) {
> > > > > if (ns == &init_pid_ns ||
> > > > > (!rdma_is_kernel_res(e) &&
> > > >
> > > > So eg here, 'e' is still needs the locking to protect against
> > > > concurrent restrack_del causing e to be freed.
> > > >
> > > > Most likely the internal xarray locking doesn't save anything in this
> > > > usage..
> > >
> > > The "rwsem" protects hash lists insert/delete and we are still using
> > > kref to protect specific entries. And those xarray insert/deletion are
> > > handled by xa_lock() now.
> >
> > This doesn't explain where the locking for 'e' comes from above
>
> According to the documentation, xa_for_each takes RCU lock while
> iterating, so I assume that "e" is safe once it returned from xarray,
The documentation means it internally takes/releases the RCU lock (in
contrast to radix tree that requires callers to hold the RCU lock to
protect the internals of the radix tree).
See the implementation, the loop body is run with RCU unlocked.
The xarray interfaces only protect its insides, the caller is
responsible to protect the returned data.
Jason
