Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASoC: sof: use iopoll.h macro for polled register reads #750

Merged

Conversation

kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Mar 25, 2019

Change to align our polled register reads with iopoll.h, related to issue #358
Some discussion already in the patch, see my commets to the issue.

Introduce a variant of readx_poll_timeout() macro from linux/iopoll.h
to SOF and use it to replace the old snd_sof_dsp_register_poll()
function. Due to indirection of register i/o in SOF, we can't directly
use the iopoll.h macros.

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

Introduce a variant of readx_poll_timeout() macro from linux/iopoll.h
to SOF and use it to replace the old snd_sof_dsp_register_poll()
function. Due to indirection of register i/o in SOF, we can't directly
use the iopoll.h macros.

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

ranj063 commented Mar 26, 2019

@kv2019i looks good to me. One of the main reasons we added the 2 resolution register polling was because of a race that was seen between when the audio fw authentication was requested and when the CSME was available. Could you please try a quick stress test with your patch on your chromebook?
This will run for a long time and you can probably leave it running overnight. Thanks!

./suspend_stress_test -c 1000.

@ranj063
Copy link
Collaborator

ranj063 commented Mar 28, 2019

@kv2019i sorry I just realized i said ./suspend_strest_test instead of suspend_stress_test.

Anyway, I've kicked off a 1000 iterations test today. I will update the result tomorrow.

@ranj063
Copy link
Collaborator

ranj063 commented Mar 28, 2019

@kv2019i @plbossart a stress test at my end has passed 2500 iterations. So I think this one is good to be merged.

@plbossart
Copy link
Member

ok, one last comment: isn't there a macro to avoid code like:
((status & chip->ipc_ack_mask) == chip->ipc_ack_mask),

or can we add one?

@kv2019i
Copy link
Collaborator Author

kv2019i commented Mar 29, 2019

@plbossart wrote:

ok, one last comment: isn't there a macro to avoid code like:
((status & chip->ipc_ack_mask) == chip->ipc_ack_mask),

or can we add one?

I was wondering the same. I went through all generic bitops headers in include/linux and grepped through dozens of drivers, but couldn't spot an established macro for this.

Then I thought having the bare logic visible is easier to read than a custom macro that is not used elsewhere. In most code locations, the code won't be much shorter in any case.

@kv2019i kv2019i marked this pull request as ready for review March 29, 2019 12:00
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

Thanks @kv2019i, merging now!

@plbossart plbossart merged commit f304b5c into thesofproject:topic/sof-dev Mar 30, 2019
@wenqingfu
Copy link

should #358 be closed then? @kv2019i

If so, recommend approach: https://help.github.com/en/articles/closing-issues-using-keywords for the future

@kv2019i
Copy link
Collaborator Author

kv2019i commented Apr 1, 2019

@wenqingfu

should #358 be closed then? @kv2019i

Ack, #358 closed now. The issue had multiple subissues and this was the last one.

If so, recommend approach: https://help.github.com/en/articles/closing-issues-using-keywords for the future

Thanks, ack, will follow this in the future.

plbossart pushed a commit that referenced this pull request Nov 29, 2022
The page table check trigger BUG_ON() unexpectedly when collapse hugepage:

 ------------[ cut here ]------------
 kernel BUG at mm/page_table_check.c:82!
 Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
 Dumping ftrace buffer:
    (ftrace buffer empty)
 Modules linked in:
 CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
 Hardware name: linux,dummy-virt (DT)
 pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
 pc : page_table_check_clear.isra.0+0x258/0x3f0
 lr : page_table_check_clear.isra.0+0x240/0x3f0
[...]
 Call trace:
  page_table_check_clear.isra.0+0x258/0x3f0
  __page_table_check_pmd_clear+0xbc/0x108
  pmdp_collapse_flush+0xb0/0x160
  collapse_huge_page+0xa08/0x1080
  hpage_collapse_scan_pmd+0xf30/0x1590
  khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
  khugepaged+0x338/0x518
  kthread+0x278/0x2f8
  ret_from_fork+0x10/0x20
[...]

Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
and so trigger BUG_ON() unexpectedly.

Fix this problem by using pmd_leaf() insteal of pmd_present() in
pmd_user_accessible_page(). Moreover, use pud_leaf() for
pud_user_accessible_page() too.

Fixes: 42b2547 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Reviewed-by: Kefeng Wang <wangkefeng.wang@huawei.com>
Acked-by: Will Deacon <will@kernel.org>
Link: https://lore.kernel.org/r/20221117075602.2904324-2-liushixin2@huawei.com
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
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