| Message ID | 20200719154545.GA41231@xin-virtual-machine (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: fix drm_dp_mst_port refcount leaks in drm_dp_mst_allocate_vcpi | expand |
Hi Xin Xong, I have seen insufficient vcpi ports issues with MST daisy chain. While running the IGT tests on MST, some times the vcpi ports are all used up. Due to this the atomic check fails and the flips start failing with ENOSPC. <https://gitlab.freedesktop.org/drm/intel/-/issues/1255> https://gitlab.freedesktop.org/drm/intel/-/issues/1255 I think this patch would be fixing this. Regards, Ankit On 7/19/2020 9:15 PM, Xin Xiong wrote: > drm_dp_mst_allocate_vcpi() invokes > drm_dp_mst_topology_get_port_validated(), which increases the refcount > of the "port". > > These reference counting issues take place in two exception handling > paths separately. Either when “slots” is less than 0 or when > drm_dp_init_vcpi() returns a negative value, the function forgets to > reduce the refcnt increased drm_dp_mst_topology_get_port_validated(), > which results in a refcount leak. > > Fix these issues by pulling up the error handling when "slots" is less > than 0, and calling drm_dp_mst_topology_put_port() before termination > when drm_dp_init_vcpi() returns a negative value. > > Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn> > Signed-off-by: Xin Tan <tanxin.ctf@gmail.com> > Signed-off-by: Xin Xiong <xiongx18@fudan.edu.cn> > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 1e26b89628f9..97b48b531ec6 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -4261,11 +4261,11 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, > { > int ret; > > - port = drm_dp_mst_topology_get_port_validated(mgr, port); > - if (!port) > + if (slots < 0) > return false; > > - if (slots < 0) > + port = drm_dp_mst_topology_get_port_validated(mgr, port); > + if (!port) > return false; > > if (port->vcpi.vcpi > 0) { > @@ -4281,6 +4281,7 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, > if (ret) { > DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n", > DIV_ROUND_UP(pbn, mgr->pbn_div), ret); > + drm_dp_mst_topology_put_port(port); > goto out; > } > DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",
Heh, I remember this being mentioned to me way back but completely forgot to ever go fix it. Thanks for the patch! Reviewed-by: Lyude Paul <lyude@redhat.com> This is missing a Fixes: tag though: Fixes: 1e797f556c61 ("drm/dp: Split drm_dp_mst_allocate_vcpi") Cc: <stable@vger.kernel.org> # v4.12+ So I will go ahead and add that, then push this to drm-misc-next-fixes. Thanks! On Sun, 2020-07-19 at 23:45 +0800, Xin Xiong wrote: > drm_dp_mst_allocate_vcpi() invokes > drm_dp_mst_topology_get_port_validated(), which increases the refcount > of the "port". > > These reference counting issues take place in two exception handling > paths separately. Either when “slots” is less than 0 or when > drm_dp_init_vcpi() returns a negative value, the function forgets to > reduce the refcnt increased drm_dp_mst_topology_get_port_validated(), > which results in a refcount leak. > > Fix these issues by pulling up the error handling when "slots" is less > than 0, and calling drm_dp_mst_topology_put_port() before termination > when drm_dp_init_vcpi() returns a negative value. > > Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn> > Signed-off-by: Xin Tan <tanxin.ctf@gmail.com> > Signed-off-by: Xin Xiong <xiongx18@fudan.edu.cn> > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 1e26b89628f9..97b48b531ec6 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -4261,11 +4261,11 @@ bool drm_dp_mst_allocate_vcpi(struct > drm_dp_mst_topology_mgr *mgr, > { > int ret; > > - port = drm_dp_mst_topology_get_port_validated(mgr, port); > - if (!port) > + if (slots < 0) > return false; > > - if (slots < 0) > + port = drm_dp_mst_topology_get_port_validated(mgr, port); > + if (!port) > return false; > > if (port->vcpi.vcpi > 0) { > @@ -4281,6 +4281,7 @@ bool drm_dp_mst_allocate_vcpi(struct > drm_dp_mst_topology_mgr *mgr, > if (ret) { > DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n", > DIV_ROUND_UP(pbn, mgr->pbn_div), ret); > + drm_dp_mst_topology_put_port(port); > goto out; > } > DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 1e26b89628f9..97b48b531ec6 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4261,11 +4261,11 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, { int ret; - port = drm_dp_mst_topology_get_port_validated(mgr, port); - if (!port) + if (slots < 0) return false; - if (slots < 0) + port = drm_dp_mst_topology_get_port_validated(mgr, port); + if (!port) return false; if (port->vcpi.vcpi > 0) { @@ -4281,6 +4281,7 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, if (ret) { DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n", DIV_ROUND_UP(pbn, mgr->pbn_div), ret); + drm_dp_mst_topology_put_port(port); goto out; } DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",