From: "Luca Ceresoli" <luca.ceresoli@bootlin.com>
To: "Liu Ying" <victor.liu@nxp.com>,
"Andrzej Hajda" <andrzej.hajda@intel.com>,
"Neil Armstrong" <neil.armstrong@linaro.org>,
"Robert Foss" <rfoss@kernel.org>,
"Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>,
"Jonas Karlman" <jonas@kwiboo.se>,
"Jernej Skrabec" <jernej.skrabec@gmail.com>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Maxime Ripard" <mripard@kernel.org>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>
Cc: "Marco Felsch" <m.felsch@pengutronix.de>,
<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] drm/bridge: Fix refcount shown via debugfs for encoder_bridges_show()
Date: Thu, 12 Mar 2026 18:30:22 +0100 [thread overview]
Message-ID: <DH0ZD9UFY68E.2C1ISONVIO42L@bootlin.com> (raw)
In-Reply-To: <20260312-drm-misc-next-2026-03-05-fix-encoder-bridges-refcount-v1-1-b9ba3d844732@nxp.com>
Hello Liu, Maxime,
On Thu Mar 12, 2026 at 7:05 AM CET, Liu Ying wrote:
> A typical bridge refcount value is 3 after a bridge chain is formed:
> - devm_drm_bridge_alloc() initializes the refcount value to be 1.
> - drm_bridge_add() gets an additional reference hence 2.
> - drm_bridge_attach() gets the third reference hence 3.
>
> This typical refcount value aligns with allbridges_show()'s behaviour.
> However, since encoder_bridges_show() uses
> drm_for_each_bridge_in_chain_scoped() to automatically get/put the
> bridge reference while iterating, a bogus reference is accidentally
> got when showing the wrong typical refcount value as 4 to users via
> debugfs. Fix this by caching the refcount value returned from
> kref_read() while iterating and explicitly decreasing the cached
> refcount value by 1 before showing it to users.
Good point, indeed the refcount shown by
<debugfs>/dri/<card>/encoder-0/bridges is by one unit higher than the one
shown in <debugfs>/dri/bridges. I understand it's puzzling from a debugfs
user point of view.
As you noticed, this is because the _scoped loop holds an extra ref on the
current bridge.
For other reasons I proposed a mutex for stronger protection around the
bridge chain [v2]. With the mutex the extra ref is redundant, so in [v2]
the extra ref is removed, thus making your patch unneeded. However Maxime
asked to keep the extra ref, and so my latest iteration [v4] still has the
extra ref.
That series is still on the mailing list, we are still in time to rediscuss
it.
@Maxime: based on the issue Liu is trying to work around, do you think it
would make sense to go back to the initial approach for that series?
I.e. drm_for_each_bridge_in_chain_scoped() grabs the chain lock, which is a
superset of the per-bridge refcount, and thus the refcount can be dropped?
This would remove the debugfs issue, slightly simplify
drm_for_each_bridge_in_chain_scoped(), and introduce no new issues AFAIK.
[v2] https://lore.kernel.org/all/20251003-drm-bridge-alloc-encoder-chain-mutex-v2-4-78bf61580a06@bootlin.com/
[v4] https://lore.kernel.org/all/20260113-drm-bridge-alloc-encoder-chain-mutex-v4-4-60f3135adc45@bootlin.com/
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
next prev parent reply other threads:[~2026-03-12 17:30 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-12 6:05 [PATCH] drm/bridge: Fix refcount shown via debugfs for encoder_bridges_show() Liu Ying
2026-03-12 17:30 ` Luca Ceresoli [this message]
2026-03-13 8:33 ` Liu Ying
2026-03-13 9:57 ` Luca Ceresoli
2026-03-13 10:22 ` Liu Ying
2026-03-13 17:32 ` Luca Ceresoli
2026-03-16 9:47 ` Liu Ying
2026-03-16 11:14 ` Luca Ceresoli
2026-03-17 2:04 ` Liu Ying
2026-03-17 8:15 ` Luca Ceresoli
2026-03-16 11:15 ` Luca Ceresoli
2026-03-17 2:35 ` Liu Ying
2026-03-17 8:15 ` Luca Ceresoli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DH0ZD9UFY68E.2C1ISONVIO42L@bootlin.com \
--to=luca.ceresoli@bootlin.com \
--cc=Laurent.pinchart@ideasonboard.com \
--cc=airlied@gmail.com \
--cc=andrzej.hajda@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=jernej.skrabec@gmail.com \
--cc=jonas@kwiboo.se \
--cc=linux-kernel@vger.kernel.org \
--cc=m.felsch@pengutronix.de \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=neil.armstrong@linaro.org \
--cc=rfoss@kernel.org \
--cc=simona@ffwll.ch \
--cc=tzimmermann@suse.de \
--cc=victor.liu@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.