Fix free topology by ranj063 · Pull Request #340 · thesofproject/linux

ranj063 · 2018-11-30T07:36:31Z

This PR is untested.

pcm_free() is not needed anymore as the pcm dma pages should be freed in dai_unload().
So move free_topoloyg() to pcm_remove().

TODO: check if/when pcm_remove() is called and modify it appropriately.

SOF pcm data spcm is freed in dai_unload(). So free the associated dma pages in dai_unload() too. Also remove the call to free topology in pcm_free() because it cannot be called multiple times. Finally remove pcm_free() itself because it does nothing now. Add the call to free_topology in pcm_remove(). Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

…gy() snd_soc_tplg_component_remove() is called when platform device unregister gets called in sof_remove. So remove the duplicate call. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

dapm is not used, so remove it. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

bardliao

Lets keep sof_pcm_free() since we need to call snd_pcm_lib_preallocate_free_for_all() in sof_pcm_free()

bardliao · 2018-11-30T08:37:51Z

sound/soc/sof/pcm.c

-
-	snd_sof_free_topology(sdev);
-}
-


Should we keep sof_pcm_free? Please see cbdc405. We still need to call snd_pcm_lib_preallocate_free_for_all in sof_pcm_free

Yeah, it's ugly to allocate dma pages in pcm_new and release them in dai_unload.

or let's allocate everything in dai_load, but not do a half-baked job in two places.

Should we keep sof_pcm_free? Please see cbdc405. We still need to call snd_pcm_lib_preallocate_free_for_all in sof_pcm_free

@bardliao doesnt snd_pcm_free() already call this? Do we need to call it again in sof_pcm_free?

@ranj063 You are right. We don't need to call it in our driver. @plbossart I believe that we don't need snd_pcm_lib_preallocate_free_for_all() see https://patchwork.kernel.org/patch/6854731/
I vote for allocating everything in dai_load.

bardliao

Yes, soc core will do this for us.

bardliao · 2018-11-30T11:38:08Z

sound/soc/sof/topology.c

@@ -2645,11 +2649,5 @@ void snd_sof_free_topology(struct snd_sof_dev *sdev)
 		kfree(sroute->private);


I am also thinking should we add list_del(&sroute->list); here. We don't need to keep the list after it is freed

plbossart · 2018-11-30T14:38:27Z

sound/soc/sof/topology.c

-					    SND_SOC_TPLG_INDEX_ALL);
-	if (ret < 0)
-		dev_err(sdev->dev,
-			"error: tplg component free failed %d\n", ret);


I have mixed feelings about this.
Is the framework design intended to release all resources (similar to devm_ for memory allocation) or does it call tplg_component_remove for extra safety?

sound/soc/sof/topology.c

ranj063 · 2018-12-17T21:11:58Z

closing this one. Will open a new one shortly!

In section_deactivate(), pfn_to_page() doesn't work any more after ms->section_mem_map is resetting to NULL in SPARSEMEM|!VMEMMAP case. It causes a hot remove failure: kernel BUG at mm/page_alloc.c:4806! invalid opcode: 0000 [thesofproject#1] SMP PTI CPU: 3 PID: 8 Comm: kworker/u16:0 Tainted: G W 5.5.0-next-20200205+ thesofproject#340 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.0.0 02/06/2015 Workqueue: kacpi_hotplug acpi_hotplug_work_fn RIP: 0010:free_pages+0x85/0xa0 Call Trace: __remove_pages+0x99/0xc0 arch_remove_memory+0x23/0x4d try_remove_memory+0xc8/0x130 __remove_memory+0xa/0x11 acpi_memory_device_remove+0x72/0x100 acpi_bus_trim+0x55/0x90 acpi_device_hotplug+0x2eb/0x3d0 acpi_hotplug_work_fn+0x1a/0x30 process_one_work+0x1a7/0x370 worker_thread+0x30/0x380 kthread+0x112/0x130 ret_from_fork+0x35/0x40 Let's move the ->section_mem_map resetting after depopulate_section_memmap() to fix it. [akpm@linux-foundation.org: remove unneeded initialization, per David] Fixes: ba72b4c ("mm/sparsemem: support sub-section hotplug") Signed-off-by: Baoquan He <bhe@redhat.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Reviewed-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com> Reviewed-by: David Hildenbrand <david@redhat.com> Acked-by: Michal Hocko <mhocko@suse.com> Cc: Wei Yang <richardw.yang@linux.intel.com> Cc: Oscar Salvador <osalvador@suse.de> Cc: Mike Rapoport <rppt@linux.ibm.com> Cc: <stable@vger.kernel.org> Link: http://lkml.kernel.org/r/20200307084229.28251-2-bhe@redhat.com Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

While working on the lazy MMU mode enablement for s390 I hit pretty curious issues in the kasan code. The first is related to a custom kasan-based sanitizer aimed at catching invalid accesses to PTEs and is inspired by [1] conversation. The kasan complains on valid PTE accesses, while the shadow memory is reported as unpoisoned: [ 102.783993] ================================================================== [ 102.784008] BUG: KASAN: out-of-bounds in set_pte_range+0x36c/0x390 [ 102.784016] Read of size 8 at addr 0000780084cf9608 by task vmalloc_test/0/5542 [ 102.784019] [ 102.784040] CPU: 1 UID: 0 PID: 5542 Comm: vmalloc_test/0 Kdump: loaded Tainted: G OE 6.16.0-gcc-ipte-kasan-11657-gb2d930c4950e thesofproject#340 PREEMPT [ 102.784047] Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE [ 102.784049] Hardware name: IBM 8561 T01 703 (LPAR) [ 102.784052] Call Trace: [ 102.784054] [<00007fffe0147ac0>] dump_stack_lvl+0xe8/0x140 [ 102.784059] [<00007fffe0112484>] print_address_description.constprop.0+0x34/0x2d0 [ 102.784066] [<00007fffe011282c>] print_report+0x10c/0x1f8 [ 102.784071] [<00007fffe090785a>] kasan_report+0xfa/0x220 [ 102.784078] [<00007fffe01d3dec>] set_pte_range+0x36c/0x390 [ 102.784083] [<00007fffe01d41c2>] leave_ipte_batch+0x3b2/0xb10 [ 102.784088] [<00007fffe07d3650>] apply_to_pte_range+0x2f0/0x4e0 [ 102.784094] [<00007fffe07e62e4>] apply_to_pmd_range+0x194/0x3e0 [ 102.784099] [<00007fffe07e820e>] __apply_to_page_range+0x2fe/0x7a0 [ 102.784104] [<00007fffe07e86d8>] apply_to_page_range+0x28/0x40 [ 102.784109] [<00007fffe090a3ec>] __kasan_populate_vmalloc+0xec/0x310 [ 102.784114] [<00007fffe090aa36>] kasan_populate_vmalloc+0x96/0x130 [ 102.784118] [<00007fffe0833a04>] alloc_vmap_area+0x3d4/0xf30 [ 102.784123] [<00007fffe083a8ba>] __get_vm_area_node+0x1aa/0x4c0 [ 102.784127] [<00007fffe083c4f6>] __vmalloc_node_range_noprof+0x126/0x4e0 [ 102.784131] [<00007fffe083c980>] __vmalloc_node_noprof+0xd0/0x110 [ 102.784135] [<00007fffe083ca32>] vmalloc_noprof+0x32/0x40 [ 102.784139] [<00007fff608aa336>] fix_size_alloc_test+0x66/0x150 [test_vmalloc] [ 102.784147] [<00007fff608aa710>] test_func+0x2f0/0x430 [test_vmalloc] [ 102.784153] [<00007fffe02841f8>] kthread+0x3f8/0x7a0 [ 102.784159] [<00007fffe014d8b4>] __ret_from_fork+0xd4/0x7d0 [ 102.784164] [<00007fffe299c00a>] ret_from_fork+0xa/0x30 [ 102.784173] no locks held by vmalloc_test/0/5542. [ 102.784176] [ 102.784178] The buggy address belongs to the physical page: [ 102.784186] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x84cf9 [ 102.784198] flags: 0x3ffff00000000000(node=0|zone=1|lastcpupid=0x1ffff) [ 102.784212] page_type: f2(table) [ 102.784225] raw: 3ffff00000000000 0000000000000000 0000000000000122 0000000000000000 [ 102.784234] raw: 0000000000000000 0000000000000000 f200000000000001 0000000000000000 [ 102.784248] page dumped because: kasan: bad access detected [ 102.784250] [ 102.784252] Memory state around the buggy address: [ 102.784260] 0000780084cf9500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 102.784274] 0000780084cf9580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 102.784277] >0000780084cf9600: fd 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 102.784290] ^ [ 102.784293] 0000780084cf9680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 102.784303] 0000780084cf9700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 102.784306] ================================================================== The second issue hits when the custom sanitizer above is not implemented, but the kasan itself is still active: [ 1554.438028] Unable to handle kernel pointer dereference in virtual kernel address space [ 1554.438065] Failing address: 001c0ff0066f0000 TEID: 001c0ff0066f0403 [ 1554.438076] Fault in home space mode while using kernel ASCE. [ 1554.438103] AS:00000000059d400b R2:0000000ffec5c00b R3:00000000c6c9c007 S:0000000314470001 P:00000000d0ab413d [ 1554.438158] Oops: 0011 ilc:2 [#1]SMP [ 1554.438175] Modules linked in: test_vmalloc(E+) nft_fib_inet(E) nft_fib_ipv4(E) nft_fib_ipv6(E) nft_fib(E) nft_reject_inet(E) nf_reject_ipv4(E) nf_reject_ipv6(E) nft_reject(E) nft_ct(E) nft_chain_nat(E) nf_nat(E) nf_conntrack(E) nf_defrag_ipv6(E) nf_defrag_ipv4(E) nf_tables(E) sunrpc(E) pkey_pckmo(E) uvdevice(E) s390_trng(E) rng_core(E) eadm_sch(E) vfio_ccw(E) mdev(E) vfio_iommu_type1(E) vfio(E) sch_fq_codel(E) drm(E) loop(E) i2c_core(E) drm_panel_orientation_quirks(E) nfnetlink(E) ctcm(E) fsm(E) zfcp(E) scsi_transport_fc(E) diag288_wdt(E) watchdog(E) ghash_s390(E) prng(E) aes_s390(E) des_s390(E) libdes(E) sha3_512_s390(E) sha3_256_s390(E) sha512_s390(E) sha1_s390(E) sha_common(E) pkey(E) autofs4(E) [ 1554.438319] Unloaded tainted modules: pkey_uv(E):1 hmac_s390(E):2 [ 1554.438354] CPU: 1 UID: 0 PID: 1715 Comm: vmalloc_test/0 Kdump: loaded Tainted: G E 6.16.0-gcc-ipte-kasan-11657-gb2d930c4950e thesofproject#350 PREEMPT [ 1554.438368] Tainted: [E]=UNSIGNED_MODULE [ 1554.438374] Hardware name: IBM 8561 T01 703 (LPAR) [ 1554.438381] Krnl PSW : 0704e00180000000 00007fffe1d3d6ae (memset+0x5e/0x98) [ 1554.438396] R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3 [ 1554.438409] Krnl GPRS: 0000000000000001 001c0ff0066f0000 001c0ff0066f0000 00000000000000f8 [ 1554.438418] 00000000000009fe 0000000000000009 0000000000000000 0000000000000002 [ 1554.438426] 0000000000005000 000078031ae655c8 00000feffdcf9f59 0000780258672a20 [ 1554.438433] 0000780243153500 00007f8033780000 00007fffe083a510 00007f7fee7cfa00 [ 1554.438452] Krnl Code: 00007fffe1d3d6a0: eb540008000c srlg %r5,%r4,8 00007fffe1d3d6a6: b9020055 ltgr %r5,%r5 #00007fffe1d3d6aa: a784000b brc 8,00007fffe1d3d6c0 >00007fffe1d3d6ae: 42301000 stc %r3,0(%r1) 00007fffe1d3d6b2: d2fe10011000 mvc 1(255,%r1),0(%r1) 00007fffe1d3d6b8: 41101100 la %r1,256(%r1) 00007fffe1d3d6bc: a757fff9 brctg %r5,00007fffe1d3d6ae 00007fffe1d3d6c0: 42301000 stc %r3,0(%r1) [ 1554.438539] Call Trace: [ 1554.438545] [<00007fffe1d3d6ae>] memset+0x5e/0x98 [ 1554.438552] ([<00007fffe083a510>] remove_vm_area+0x220/0x400) [ 1554.438562] [<00007fffe083a9d6>] vfree.part.0+0x26/0x810 [ 1554.438569] [<00007fff6073bd50>] fix_align_alloc_test+0x50/0x90 [test_vmalloc] [ 1554.438583] [<00007fff6073c73a>] test_func+0x46a/0x6c0 [test_vmalloc] [ 1554.438593] [<00007fffe0283ac8>] kthread+0x3f8/0x7a0 [ 1554.438603] [<00007fffe014d8b4>] __ret_from_fork+0xd4/0x7d0 [ 1554.438613] [<00007fffe299ac0a>] ret_from_fork+0xa/0x30 [ 1554.438622] INFO: lockdep is turned off. [ 1554.438627] Last Breaking-Event-Address: [ 1554.438632] [<00007fffe1d3d65c>] memset+0xc/0x98 [ 1554.438644] Kernel panic - not syncing: Fatal exception: panic_on_oops This series fixes the above issues and is a pre-requisite for the s390 lazy MMU mode implementation. test_vmalloc was used to stress-test the fixes. This patch (of 2): When vmalloc shadow memory is established the modification of the corresponding page tables is not protected by any locks. Instead, the locking is done per-PTE. This scheme however has defects. kasan_populate_vmalloc_pte() - while ptep_get() read is atomic the sequence pte_none(ptep_get()) is not. Doing that outside of the lock might lead to a concurrent PTE update and what could be seen as a shadow memory corruption as result. kasan_depopulate_vmalloc_pte() - by the time a page whose address was extracted from ptep_get() read and cached in a local variable outside of the lock is attempted to get free, could actually be freed already. To avoid these put ptep_get() itself and the code that manipulates the result of the read under lock. In addition, move freeing of the page out of the atomic context. Link: https://lkml.kernel.org/r/cover.1755528662.git.agordeev@linux.ibm.com Link: https://lkml.kernel.org/r/adb258634194593db294c0d1fb35646e894d6ead.1755528662.git.agordeev@linux.ibm.com Link: https://lore.kernel.org/linux-mm/5b0609c9-95ee-4e48-bb6d-98f57c5d2c31@arm.com/ [1] Fixes: 3c5c3cf ("kasan: support backing vmalloc space with real shadow memory") Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com> Cc: Daniel Axtens <dja@axtens.net> Cc: Marc Rutland <mark.rutland@arm.com> Cc: Ryan Roberts <ryan.roberts@arm.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

ranj063 added 2 commits November 29, 2018 23:31

ASoC: SOF: remove snd_soc_tplg_component_remove() call in free_topolo…

3aeb3f6

…gy() snd_soc_tplg_component_remove() is called when platform device unregister gets called in sof_remove. So remove the duplicate call. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 requested review from bardliao and plbossart November 30, 2018 07:36

ranj063 changed the title ~~Fix/free topology~~ Fix free topology Nov 30, 2018

ASoC: SOF: remove unused variable

53506ad

dapm is not used, so remove it. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

bardliao requested changes Nov 30, 2018

View reviewed changes

bardliao approved these changes Nov 30, 2018

View reviewed changes

bardliao reviewed Nov 30, 2018

View reviewed changes

plbossart reviewed Nov 30, 2018

View reviewed changes

sound/soc/sof/topology.c Show resolved Hide resolved

ranj063 closed this Dec 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix free topology#340

Fix free topology#340
ranj063 wants to merge 3 commits intothesofproject:topic/sof-devfrom
ranj063:fix/free_topology

ranj063 commented Nov 30, 2018 •

edited

Loading

Uh oh!

bardliao left a comment

Uh oh!

bardliao Nov 30, 2018

Uh oh!

plbossart Nov 30, 2018

Uh oh!

plbossart Nov 30, 2018

Uh oh!

ranj063 Nov 30, 2018

Uh oh!

bardliao Dec 1, 2018

Uh oh!

bardliao left a comment

Uh oh!

bardliao Nov 30, 2018

Uh oh!

plbossart Nov 30, 2018

Uh oh!

Uh oh!

ranj063 commented Dec 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -2645,11 +2649,5 @@ void snd_sof_free_topology(struct snd_sof_dev *sdev)
		kfree(sroute->private);

Conversation

ranj063 commented Nov 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bardliao left a comment

Choose a reason for hiding this comment

Uh oh!

bardliao Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

plbossart Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

plbossart Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

ranj063 Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

bardliao Dec 1, 2018

Choose a reason for hiding this comment

Uh oh!

bardliao left a comment

Choose a reason for hiding this comment

Uh oh!

bardliao Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

plbossart Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ranj063 commented Dec 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ranj063 commented Nov 30, 2018 •

edited

Loading