Hi Yunke,
Thanks for the patch.
On Tue, Apr 12, 2022 at 03:23:13PM +0900, Yunke Cao wrote:
> The original implementation removes reverse links for any input link and
> assumes the presense of sink/source.
> It fails when the link is a not a data link.
> media_entity_remove_links when there's an ancillary link can also fail.
The function's return type is void. Are there other adverse effects from
this? Looking at the function, it would seem like that the reverse link
simply isn't found in this case, and so not removed.
>
> We only need to remove reverse links for a data link.
Ideally this would not be based on the link flags as it's not a very robust
way to test whather a backlink needs to be removed.
>
> Signed-off-by: Yunke Cao <yunkec@xxxxxxxxxx>
> ---
> drivers/media/mc/mc-entity.c | 34 +++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index 1ff60d411ea9..11f5207f73aa 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -597,26 +597,30 @@ static void __media_entity_remove_link(struct media_entity *entity,
> struct media_link *rlink, *tmp;
> struct media_entity *remote;
>
> - if (link->source->entity == entity)
> - remote = link->sink->entity;
> - else
> - remote = link->source->entity;
> + /* Remove the reverse links for a data link. */
> + if ((link->flags & MEDIA_LNK_FL_LINK_TYPE) == MEDIA_LNK_FL_DATA_LINK) {
> + if (link->source->entity == entity)
> + remote = link->sink->entity;
> + else
> + remote = link->source->entity;
>
> - list_for_each_entry_safe(rlink, tmp, &remote->links, list) {
> - if (rlink != link->reverse)
> - continue;
> + list_for_each_entry_safe(rlink, tmp, &remote->links, list) {
> + if (rlink != link->reverse)
> + continue;
>
> - if (link->source->entity == entity)
> - remote->num_backlinks--;
> + if (link->source->entity == entity)
> + remote->num_backlinks--;
>
> - /* Remove the remote link */
> - list_del(&rlink->list);
> - media_gobj_destroy(&rlink->graph_obj);
> - kfree(rlink);
> + /* Remove the remote link */
> + list_del(&rlink->list);
> + media_gobj_destroy(&rlink->graph_obj);
> + kfree(rlink);
>
> - if (--remote->num_links == 0)
> - break;
> + if (--remote->num_links == 0)
> + break;
> + }
> }
> +
> list_del(&link->list);
> media_gobj_destroy(&link->graph_obj);
> kfree(link);
--
Kind regards,
Sakari Ailus
