Re: [PATCH v4 16/40] cxl/core/port: Use dedicated lock for decoder target list

[Dan Williams <dan.j.williams@intel.com>]

On Mon, Jan 31, 2022 at 3:34 PM Ben Widawsky <ben.widawsky@intel.com> 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@intel.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> > 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.