search for: mstb

On Tue, Sep 03, 2019 at 04:46:04PM -0400, Lyude Paul wrote: > Currently we only print mstb/port pointer addresses in our malloc and > topology refcount functions using the hashed-by-default %p, but > unfortunately if you're trying to debug a use-after-free error caused by > a refcounting error then this really isn't terribly useful. On the other > hand though, everyth...

...sume reprobing very soon, > we need to make sure we are extra careful to ensure that our locking > actually protects the topology state where we expect it to. Turns out > this isn't the case with drm_dp_port_setup_pdt() and > drm_dp_port_teardown_pdt(), both of which change port->mstb without > grabbing &mgr->lock. > > Additionally, since most callers of these functions are just using it to > teardown the port's previous PDT and setup a new one we can simplify > things a bit and combine drm_dp_port_setup_pdt() and > drm_dp_port_teardown_pdt() into a...

Currently we only print mstb/port pointer addresses in our malloc and topology refcount functions using the hashed-by-default %p, but unfortunately if you're trying to debug a use-after-free error caused by a refcounting error then this really isn't terribly useful. On the other hand though, everything in the rest of t...

...umentation/gpu/dp-mst/topology-figure-1.dot > new file mode 100644 > index 000000000000..fb83789e0a3e > --- /dev/null > +++ b/Documentation/gpu/dp-mst/topology-figure-1.dot > @@ -0,0 +1,31 @@ > +digraph T { > + /* Topology references */ > + node [shape=oval]; > + mstb1 -> {port1, port2}; > + port1 -> mstb2; > + port2 -> mstb3 -> {port3, port4}; > + port3 -> mstb4; > + > + /* Malloc references */ > + edge [style=dashed]; > + mstb4 -> port3; > + {port4, port3} -> mstb3; > + mstb3 -> port2;...

...tation/gpu/dp-mst/topology-figure-1.dot b/Documentation/gpu/dp-mst/topology-figure-1.dot new file mode 100644 index 000000000000..fb83789e0a3e --- /dev/null +++ b/Documentation/gpu/dp-mst/topology-figure-1.dot @@ -0,0 +1,31 @@ +digraph T { + /* Topology references */ + node [shape=oval]; + mstb1 -> {port1, port2}; + port1 -> mstb2; + port2 -> mstb3 -> {port3, port4}; + port3 -> mstb4; + + /* Malloc references */ + edge [style=dashed]; + mstb4 -> port3; + {port4, port3} -> mstb3; + mstb3 -> port2; + mstb2 -> port1; + {port1, port2} -...

...> index 000000000000..fb83789e0a3e > > > --- /dev/null > > > +++ b/Documentation/gpu/dp-mst/topology-figure-1.dot > > > @@ -0,0 +1,31 @@ > > > +digraph T { > > > + /* Topology references */ > > > + node [shape=oval]; > > > + mstb1 -> {port1, port2}; > > > + port1 -> mstb2; > > > + port2 -> mstb3 -> {port3, port4}; > > > + port3 -> mstb4; > > > + > > > + /* Malloc references */ > > > + edge [style=dashed]; > > > + mstb4 -> port...

...gt; > new file mode 100644 > > index 000000000000..fb83789e0a3e > > --- /dev/null > > +++ b/Documentation/gpu/dp-mst/topology-figure-1.dot > > @@ -0,0 +1,31 @@ > > +digraph T { > > + /* Topology references */ > > + node [shape=oval]; > > + mstb1 -> {port1, port2}; > > + port1 -> mstb2; > > + port2 -> mstb3 -> {port3, port4}; > > + port3 -> mstb4; > > + > > + /* Malloc references */ > > + edge [style=dashed]; > > + mstb4 -> port3; > > + {port4, port3} -&...

s/drm_dp_get_validated_port_ref/drm_dp_mst_topology_get_port_validated/ s/drm_dp_put_port/drm_dp_mst_topology_put_port/ s/drm_dp_get_validated_mstb_ref/drm_dp_mst_topology_get_mstb_validated/ s/drm_dp_put_mst_branch_device/drm_dp_mst_topology_put_mstb/ This is a much more consistent naming scheme, and will make even more sense once we redesign how the current refcounting scheme here works. Signed-off-by: Lyude Paul <lyude at redhat.com&gt...

...lementing suspend/resume reprobing very soon, we need to make sure we are extra careful to ensure that our locking actually protects the topology state where we expect it to. Turns out this isn't the case with drm_dp_port_setup_pdt() and drm_dp_port_teardown_pdt(), both of which change port->mstb without grabbing &mgr->lock. Additionally, since most callers of these functions are just using it to teardown the port's previous PDT and setup a new one we can simplify things a bit and combine drm_dp_port_setup_pdt() and drm_dp_port_teardown_pdt() into a single function: drm_dp_port_...

...lementing suspend/resume reprobing very soon, we need to make sure we are extra careful to ensure that our locking actually protects the topology state where we expect it to. Turns out this isn't the case with drm_dp_port_setup_pdt() and drm_dp_port_teardown_pdt(), both of which change port->mstb without grabbing &mgr->lock. Additionally, since most callers of these functions are just using it to teardown the port's previous PDT and setup a new one we can simplify things a bit and combine drm_dp_port_setup_pdt() and drm_dp_port_teardown_pdt() into a single function: drm_dp_port_...

...soon, > > we need to make sure we are extra careful to ensure that our locking > > actually protects the topology state where we expect it to. Turns out > > this isn't the case with drm_dp_port_setup_pdt() and > > drm_dp_port_teardown_pdt(), both of which change port->mstb without > > grabbing &mgr->lock. > > > > Additionally, since most callers of these functions are just using it to > > teardown the port's previous PDT and setup a new one we can simplify > > things a bit and combine drm_dp_port_setup_pdt() and > > drm...

...uct references in kernel-docs - danvet * Drop the "when they are no longer being used" bits from the kernel docs - danvet * Modify diagrams to show how the DRM driver interacts with the topology and payloads - danvet * Make suggested documentation changes for drm_dp_mst_topology_get_mstb() and drm_dp_mst_topology_get_port() - danvet * Better explain the relationship between malloc refs and topology krefs in the documentation for drm_dp_mst_topology_get_port() and drm_dp_mst_topology_get_mstb() - danvet * Fix "See also" in drm_dp_mst_topology_get_mstb() - danvet * Re...

On Tue, Sep 03, 2019 at 04:45:41PM -0400, Lyude Paul wrote: > When reprobing an MST topology during resume, we have to account for the > fact that while we were suspended it's possible that mstbs may have been > removed from any ports in the topology. Since iterating downwards in the > topology requires that we hold &mgr->lock, destroying MSTBs from this > context would result in attempting to lock &mgr->lock a second time and > deadlocking. > > So, fix thi...

...uct references in kernel-docs - danvet * Drop the "when they are no longer being used" bits from the kernel docs - danvet * Modify diagrams to show how the DRM driver interacts with the topology and payloads - danvet * Make suggested documentation changes for drm_dp_mst_topology_get_mstb() and drm_dp_mst_topology_get_port() - danvet * Better explain the relationship between malloc refs and topology krefs in the documentation for drm_dp_mst_topology_get_port() and drm_dp_mst_topology_get_mstb() - danvet * Fix "See also" in drm_dp_mst_topology_get_mstb() - danvet * Re...

...Paul wrote: > On Wed, 2019-09-25 at 14:16 -0400, Sean Paul wrote: > > On Tue, Sep 03, 2019 at 04:45:41PM -0400, Lyude Paul wrote: > > > When reprobing an MST topology during resume, we have to account for the > > > fact that while we were suspended it's possible that mstbs may have been > > > removed from any ports in the topology. Since iterating downwards in the > > > topology requires that we hold &mgr->lock, destroying MSTBs from this > > > context would result in attempting to lock &mgr->lock a second time and > >...

...to the atomic checking code I added to i915 and nouveau (but not the helpers). Also, per-request I've made a gitlab branch available for this: https://gitlab.freedesktop.org/lyudess/linux/commits/wip/mst-dual-kref-start-v4 Lyude Paul (16): drm/dp_mst: Rename drm_dp_mst_get_validated_(port|mstb)_ref and friends drm/dp_mst: Introduce new refcounting scheme for mstbs and ports drm/dp_mst: Restart last_connected_port_and_mstb() if topology ref fails drm/dp_mst: Stop releasing VCPI when removing ports from topology drm/dp_mst: Fix payload deallocation on hotplugs using malloc...

When reprobing an MST topology during resume, we have to account for the fact that while we were suspended it's possible that mstbs may have been removed from any ports in the topology. Since iterating downwards in the topology requires that we hold &mgr->lock, destroying MSTBs from this context would result in attempting to lock &mgr->lock a second time and deadlocking. So, fix this by first moving destruction...

When reprobing an MST topology during resume, we have to account for the fact that while we were suspended it's possible that mstbs may have been removed from any ports in the topology. Since iterating downwards in the topology requires that we hold &mgr->lock, destroying MSTBs from this context would result in attempting to lock &mgr->lock a second time and deadlocking. So, fix this by first moving destruction...

On Wed, 2019-09-25 at 14:16 -0400, Sean Paul wrote: > On Tue, Sep 03, 2019 at 04:45:41PM -0400, Lyude Paul wrote: > > When reprobing an MST topology during resume, we have to account for the > > fact that while we were suspended it's possible that mstbs may have been > > removed from any ports in the topology. Since iterating downwards in the > > topology requires that we hold &mgr->lock, destroying MSTBs from this > > context would result in attempting to lock &mgr->lock a second time and > > deadlocking. &g...

...%s", + entry->count, + topology_ref_type_to_str(entry->type), + ts_nsec, rem_nsec / 1000, buf); + } + + /* Now free the history, since this is the only time we expose it */ + kfree(history->entries); +out: + kfree(buf); +} + +static __always_inline void +drm_dp_mst_dump_mstb_topology_history(struct drm_dp_mst_branch *mstb) +{ + __dump_topology_ref_history(&mstb->topology_ref_history, mstb, + "MSTB"); +} + +static __always_inline void +drm_dp_mst_dump_port_topology_history(struct drm_dp_mst_port *port) +{ + __dump_topology_ref_history(&port-&...