On Mon, Jan 31, 2022 at 3:43 PM Ben Widawsky <ben.widawsky@xxxxxxxxx> wrote:
>
> On 22-01-31 15:38:44, Dan Williams wrote:
> > On Mon, Jan 31, 2022 at 3:34 PM Ben Widawsky <ben.widawsky@xxxxxxxxx> wrote:
> > >
> > > On 22-01-25 18:54:36, Dan Williams wrote:
> > > > Lockdep reports:
> > > >
> > > > ======================================================
> > > > WARNING: possible circular locking dependency detected
> > > > 5.16.0-rc1+ #142 Tainted: G OE
> > > > ------------------------------------------------------
> > > > cxl/1220 is trying to acquire lock:
> > > > ffff979b85475460 (kn->active#144){++++}-{0:0}, at: __kernfs_remove+0x1ab/0x1e0
> > > >
> > > > but task is already holding lock:
> > > > ffff979b87ab38e8 (&dev->lockdep_mutex#2/4){+.+.}-{3:3}, at: cxl_remove_ep+0x50c/0x5c0 [cxl_core]
> > > >
> > > > ...where cxl_remove_ep() is a helper that wants to delete ports while
> > > > holding a lock on the host device for that port. That sets up a lockdep
> > > > violation whereby target_list_show() can not rely holding the decoder's
> > > > device lock while walking the target_list. Switch to a dedicated seqlock
> > > > for this purpose.
> > > >
> > > > Reported-by: Ben Widawsky <ben.widawsky@xxxxxxxxx>
> > > > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> > > > ---
> > > > Changes in v4:
> > > > - Fix missing unlock in error exit case (Ben)
> > >
> > > Could you help me understand why we need a lock at all for the target list? I
> > > thought the target list remains static throughout the lifetime of the decoder at
> > > which point, the only issue would be reading the sysfs entries while the decoder
> > > is being destroyed. Is that possible?
> >
> > This is emitting the target list per the current configuration. If
> > another thread or the kernel is configuring the decoder and while the
> > target list is being read it should get a coherent snapshot, not the
> > intermediate settings.
>
> How can you see the decoder in sysfs before it is finished being configured?
After cxl_decoder_add() the attribute is visible to userspace. At the
beginning of time a disabled decoder will have zeroes in all "Target
Port Identifier" fields. When region creation changes the target list
to valid values it needs to synchronize against userspace that may be
actively walking the target list as it is being updated.
