Skip to content

Conversation

@keyonjie
Copy link
Contributor

No description provided.

@keyonjie keyonjie force-pushed the multi_core_test branch 7 times, most recently from 7669e28 to 76cf1b2 Compare October 22, 2021 08:37
@ranj063
Copy link
Collaborator

ranj063 commented Oct 24, 2021

closing this. rolled into #4702

@ranj063 ranj063 closed this Oct 24, 2021
@ranj063 ranj063 reopened this Oct 24, 2021
@ranj063
Copy link
Collaborator

ranj063 commented Oct 24, 2021

sorry closed the wrong one. meant to close mine :)

@keyonjie keyonjie force-pushed the multi_core_test branch 4 times, most recently from ffb5385 to 9e23888 Compare October 28, 2021 09:03
@keyonjie keyonjie force-pushed the multi_core_test branch 6 times, most recently from c83bdf4 to 42a0197 Compare November 9, 2021 10:05
@keyonjie keyonjie force-pushed the multi_core_test branch 9 times, most recently from 8d6c2b8 to a01579a Compare November 16, 2021 05:56
Copy link
Collaborator

@lyakh lyakh Nov 17, 2021

Choose a reason for hiding this comment

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

no, this breaks atomicity. The original code made sure that no IRQ could interrupt execution between line 353 and the call to dw_dma_stop(). After that patch this atomicity is broken. Nested calls to irq_local_disable() or irq_local_enable() are fine as long as they're done with different flags. Those names are a bit misleading, in the kernel they're called local_irq_save() and local_irq_restore(). But the important point is, that if you call irq_local_disable() twice with different flags, if interrupts are enabled during the first call, it will save the CPU interrupt state with enabled interrupts into flags and disable interrupts. The next call to irq_local_disable() will save already disabled interrupts into a different copy of flags. Then the first call to irq_local_enable() with the second copy of the flags will restore that CPU state, which still had interrupts disabled. And only the second call to irq_local_enable() with the first copy of flags will re-enable the interrupts.
It is a different question though whether atomicity is important here.

@gkbldcig
Copy link
Collaborator

Can one of the admins verify this patch?

Only for test at the moment.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
@lgirdwood
Copy link
Member

@keyonjie what's the ETA to merge this ?

@keyonjie keyonjie closed this Dec 20, 2021
@keyonjie
Copy link
Contributor Author

this was only for test, closing it now.

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.

5 participants