Skip to content

Conversation

@kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Apr 1, 2022

BUFFER_FREE IPC returns an error if buffer is connected to
two active components.

However it is allowed to free a buffer in the case where either the sink
or the source is no longer active. This happens for example with buffers
that connect two pipelines. If one pipeline is stopped and freed,
the connecting buffer will be also freed.

This does mean pipeline_disconnect() may be run in context
of a pipeline that is running on a different core. This is
fundamentally unsafe and can lead to undefined behaviour
when two cores access the pipeline bsource_list/bsink_list
at the same time on two different cores.

Fix this issue by checking for this case, and running
the BUFFER_FREE on the active core.

The change is not sufficient on its own, but is part of the fixes
to address: #5469

Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com

BUFFER_FREE IPC returns an error if buffer is connected to
two active components.

However it is allowed to free a buffer in the case where either the sink
or the source is no longer active. This happens for example with buffers
that connect two pipelines. If one pipeline is stopped and freed,
the connecting buffer will be also freed.

This does mean pipeline_disconnect() may be run in context
of a pipeline that is running on a different core. This is
fundamentally unsafe and can lead to undefined behaviour
when two cores access the pipeline bsource_list/bsink_list
at the same time on two different cores.

Fix this issue by checking for this case, and running
the BUFFER_FREE on the active core.

The change is not sufficient on its own, but is part of the fixes
to address: thesofproject#5469

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw, @RanderWang doing a similar fix for IPC4

Copy link
Collaborator

@lyakh lyakh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is hitting the "buffer core" topic again. This has been discussed before and my understanding was, that we were trying to achieve the following status:

  1. any connected pipelines should run on the same core. Different cores aren't currently supported and would also be difficult to support without major changes to the code base
  2. (1) also implies that all components within a single pipeline should run on the same core
  3. buffer->core is then redundant, since all buffers have source and sink components handled by the same core, so buffers are never shared.

If the above is correct, can we make it explicit? If it isn't, we need test-cases...

@lgirdwood
Copy link
Member

  1. any connected pipelines should run on the same core. Different cores aren't currently supported and would also be difficult to support without major changes to the code base

Different cores should be supported for connected pipelines especially with Zephyr.

@lgirdwood lgirdwood merged commit 69db9c8 into thesofproject:main Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants