From: Anthony Iliopoulos <ailiop@suse.com> To: Robin Murphy <robin.murphy@arm.com> Cc: Christoph Hellwig <hch@lst.de>, Marek Szyprowski <m.szyprowski@samsung.com>, iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] dma-debug: fix debugfs initialization order Date: Thu, 22 Jul 2021 16:10:55 +0200 [thread overview] Message-ID: <YPl8b7KuoNBg52LE@technoir> (raw) In-Reply-To: <1ea36b32-9bbc-a611-402d-9fa196eda166@arm.com> On Thu, Jul 22, 2021 at 11:10:24AM +0100, Robin Murphy wrote: > On 2021-07-22 10:18, Anthony Iliopoulos wrote: > > Due to link order, dma_debug_init is called before debugfs has a chance > > to initialize (via debugfs_init which also happens in the core initcall > > stage), so the directories for dma-debug are never created. > > > > Move the dma_debug_init initcall from core to postcore stage so that > > debugfs will already be initialized by the time this is called, making > > it oblivious to link-ordering. > > Playing initcall chicken here doesn't work so well - the later you > initialise dma-debug itself, the more chance it has to miss early mappings > and raise false positives later. As discussed previously[1] the better > solution would be to decouple the debugfs setup so that just that part can > be deferred until core_initcall_sync or later. Thanks for pointing it out, makes sense. What about the following: From: Anthony Iliopoulos <ailiop@suse.com> Due to link order, dma_debug_init is called before debugfs has a chance to initialize (via debugfs_init which also happens in the core initcall stage), so the directories for dma-debug are never created. Decouple dma_debug_fs_init from dma_debug_init and defer its init until core_initcall_sync (after debugfs has been initialized) while letting dma-debug initialization occur as soon as possible to catch any early mappings, as suggested in [1]. [1] https://lore.kernel.org/linux-iommu/YIgGa6yF%2Fadg8OSN@kroah.com/ Fixes: 15b28bbcd567 ("dma-debug: move initialization to common code") Signed-off-by: Anthony Iliopoulos <ailiop@suse.com> --- kernel/dma/debug.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c index 14de1271463f..445754529917 100644 --- a/kernel/dma/debug.c +++ b/kernel/dma/debug.c @@ -794,7 +794,7 @@ static int dump_show(struct seq_file *seq, void *v) } DEFINE_SHOW_ATTRIBUTE(dump); -static void dma_debug_fs_init(void) +static int __init dma_debug_fs_init(void) { struct dentry *dentry = debugfs_create_dir("dma-api", NULL); @@ -807,7 +807,10 @@ static void dma_debug_fs_init(void) debugfs_create_u32("nr_total_entries", 0444, dentry, &nr_total_entries); debugfs_create_file("driver_filter", 0644, dentry, NULL, &filter_fops); debugfs_create_file("dump", 0444, dentry, NULL, &dump_fops); + + return 0; } +core_initcall_sync(dma_debug_fs_init); static int device_dma_allocations(struct device *dev, struct dma_debug_entry **out_entry) { @@ -892,8 +895,6 @@ static int dma_debug_init(void) spin_lock_init(&dma_entry_hash[i].lock); } - dma_debug_fs_init(); - nr_pages = DIV_ROUND_UP(nr_prealloc_entries, DMA_DEBUG_DYNAMIC_ENTRIES); for (i = 0; i < nr_pages; ++i) dma_debug_create_entries(GFP_KERNEL);
WARNING: multiple messages have this Message-ID (diff)
From: Anthony Iliopoulos via iommu <iommu@lists.linux-foundation.org> To: Robin Murphy <robin.murphy@arm.com> Cc: linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, Christoph Hellwig <hch@lst.de> Subject: Re: [PATCH] dma-debug: fix debugfs initialization order Date: Thu, 22 Jul 2021 16:10:55 +0200 [thread overview] Message-ID: <YPl8b7KuoNBg52LE@technoir> (raw) In-Reply-To: <1ea36b32-9bbc-a611-402d-9fa196eda166@arm.com> On Thu, Jul 22, 2021 at 11:10:24AM +0100, Robin Murphy wrote: > On 2021-07-22 10:18, Anthony Iliopoulos wrote: > > Due to link order, dma_debug_init is called before debugfs has a chance > > to initialize (via debugfs_init which also happens in the core initcall > > stage), so the directories for dma-debug are never created. > > > > Move the dma_debug_init initcall from core to postcore stage so that > > debugfs will already be initialized by the time this is called, making > > it oblivious to link-ordering. > > Playing initcall chicken here doesn't work so well - the later you > initialise dma-debug itself, the more chance it has to miss early mappings > and raise false positives later. As discussed previously[1] the better > solution would be to decouple the debugfs setup so that just that part can > be deferred until core_initcall_sync or later. Thanks for pointing it out, makes sense. What about the following: From: Anthony Iliopoulos <ailiop@suse.com> Due to link order, dma_debug_init is called before debugfs has a chance to initialize (via debugfs_init which also happens in the core initcall stage), so the directories for dma-debug are never created. Decouple dma_debug_fs_init from dma_debug_init and defer its init until core_initcall_sync (after debugfs has been initialized) while letting dma-debug initialization occur as soon as possible to catch any early mappings, as suggested in [1]. [1] https://lore.kernel.org/linux-iommu/YIgGa6yF%2Fadg8OSN@kroah.com/ Fixes: 15b28bbcd567 ("dma-debug: move initialization to common code") Signed-off-by: Anthony Iliopoulos <ailiop@suse.com> --- kernel/dma/debug.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c index 14de1271463f..445754529917 100644 --- a/kernel/dma/debug.c +++ b/kernel/dma/debug.c @@ -794,7 +794,7 @@ static int dump_show(struct seq_file *seq, void *v) } DEFINE_SHOW_ATTRIBUTE(dump); -static void dma_debug_fs_init(void) +static int __init dma_debug_fs_init(void) { struct dentry *dentry = debugfs_create_dir("dma-api", NULL); @@ -807,7 +807,10 @@ static void dma_debug_fs_init(void) debugfs_create_u32("nr_total_entries", 0444, dentry, &nr_total_entries); debugfs_create_file("driver_filter", 0644, dentry, NULL, &filter_fops); debugfs_create_file("dump", 0444, dentry, NULL, &dump_fops); + + return 0; } +core_initcall_sync(dma_debug_fs_init); static int device_dma_allocations(struct device *dev, struct dma_debug_entry **out_entry) { @@ -892,8 +895,6 @@ static int dma_debug_init(void) spin_lock_init(&dma_entry_hash[i].lock); } - dma_debug_fs_init(); - nr_pages = DIV_ROUND_UP(nr_prealloc_entries, DMA_DEBUG_DYNAMIC_ENTRIES); for (i = 0; i < nr_pages; ++i) dma_debug_create_entries(GFP_KERNEL); _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2021-07-22 14:11 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-22 9:18 [PATCH] dma-debug: fix debugfs initialization order Anthony Iliopoulos 2021-07-22 9:18 ` Anthony Iliopoulos via iommu 2021-07-22 10:10 ` Robin Murphy 2021-07-22 10:10 ` Robin Murphy 2021-07-22 14:10 ` Anthony Iliopoulos [this message] 2021-07-22 14:10 ` Anthony Iliopoulos via iommu 2021-08-09 15:15 ` Christoph Hellwig 2021-08-09 15:15 ` Christoph Hellwig
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=YPl8b7KuoNBg52LE@technoir \ --to=ailiop@suse.com \ --cc=hch@lst.de \ --cc=iommu@lists.linux-foundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=m.szyprowski@samsung.com \ --cc=robin.murphy@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.