[PATCH] EDAC: use debugfs_remove_recursive instead of debugfs_remove

[PATCH] EDAC: use debugfs_remove_recursive instead of debugfs_remove

[Tan Xiaojun]

debugfs_remove is used to remove a file or an empty directory from
the debugfs filesystem, but mci->debugfs is not empty.

This is not easily discovered, because debugfs_remove return nothing
when failed.

Reported-by: Yun Wu (Abel) <wuyun.wu@huawei.com>
Signed-off-by: Tan Xiaojun <tanxiaojun@huawei.com>
---
 drivers/edac/edac_mc_sysfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 33df7d9..292e806 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -956,7 +956,7 @@ static int edac_create_debug_nodes(struct mem_ctl_info *mci)
 	mci->debugfs = parent;
 	return 0;
 nomem:
-	debugfs_remove(mci->debugfs);
+	debugfs_remove_recursive(mci->debugfs);
 	return -ENOMEM;
 }
 #endif
@@ -1070,7 +1070,7 @@ void edac_remove_sysfs_mci_device(struct mem_ctl_info *mci)
 	edac_dbg(0, "\n");
 
 #ifdef CONFIG_EDAC_DEBUG
-	debugfs_remove(mci->debugfs);
+	debugfs_remove_recursive(mci->debugfs);
 #endif
 #ifdef CONFIG_EDAC_LEGACY_SYSFS
 	edac_delete_csrow_objects(mci);
-- 
2.5.3

Re: [PATCH] EDAC: use debugfs_remove_recursive instead of debugfs_remove

[Borislav Petkov]

On Tue, Oct 13, 2015 at 10:22:51AM +0800, Tan Xiaojun wrote:
> debugfs_remove is used to remove a file or an empty directory from
> the debugfs filesystem, but mci->debugfs is not empty.

How exactly do you trigger this? And by "this" I'm not even sure what
the problem is as your commit message doesn't say. So please explain
also what the problem is.

Also, use this branch instead:

http://git.kernel.org/cgit/linux/kernel/git/bp/bp.git/log/?h=for-next

We did change EDAC debugfs handling recently so your patch doesn't apply
anymore.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH] EDAC: use debugfs_remove_recursive instead of debugfs_remove

[Tan Xiaojun]

On 2015/10/13 15:41, Borislav Petkov wrote:
> On Tue, Oct 13, 2015 at 10:22:51AM +0800, Tan Xiaojun wrote:
>> debugfs_remove is used to remove a file or an empty directory from
>> the debugfs filesystem, but mci->debugfs is not empty.
> 
> How exactly do you trigger this? And by "this" I'm not even sure what
> the problem is as your commit message doesn't say. So please explain
> also what the problem is.
> 
> Also, use this branch instead:
> 
> http://git.kernel.org/cgit/linux/kernel/git/bp/bp.git/log/?h=for-next
> 
> We did change EDAC debugfs handling recently so your patch doesn't apply
> anymore.
> 
> Thanks.
> 

Sorry, I don't describe it well. I do test like that:
1)open EDAC config and open CONFIG_EDAC_DEBUG
2)insmod a edac_mc module (like i3000_edac or others in drivers/edac/)
3)rmmod this module
4)we can also see files under /sys/kernel/debug/edac/ like "fake_inject"

If you think this patch is OK, I will do patch in your branch and send V2.

Thanks.
Xiaojun

Re: [PATCH] EDAC: use debugfs_remove_recursive instead of debugfs_remove

[Borislav Petkov]

On Tue, Oct 13, 2015 at 04:45:26PM +0800, Tan Xiaojun wrote:
> Sorry, I don't describe it well. I do test like that:
> 1)open EDAC config and open CONFIG_EDAC_DEBUG
> 2)insmod a edac_mc module (like i3000_edac or others in drivers/edac/)
> 3)rmmod this module
> 4)we can also see files under /sys/kernel/debug/edac/ like "fake_inject"
> 
> If you think this patch is OK, I will do patch in your branch and send V2.

Yes, please. I just triggered:

[  119.331642] EDAC DEBUG: mc_attr_release: Releasing device mc
[  159.735172] BUG: unable to handle kernel paging request at ffffffffa0056c80
[  159.742184] IP: [<ffffffff811fda90>] do_dentry_open.isra.15+0xe0/0x2d0
[  159.748755] PGD 3e0c067 PUD 3e0d063 PMD 43759e067 PTE 0
[  159.754059] Oops: 0000 [#1] PREEMPT SMP 
[  159.758026] Modules linked in: binfmt_misc nls_iso8859_15 nls_cp437 btrfs x86_pkg_temp_thermal coretemp kvm_intel xor kvm raid6_pq crc32_pclmul crc32c_intel ghash_clmulni_intel hmac drbg usbhid aesni_intel aes_x86_64 snd_hda_codec_hdmi glue_helper snd_hda_codec_realtek dcdbas lrw snd_hda_codec_generic dell_smm_hwmon gf128mul serio_raw snd_hda_intel ablk_helper iTCO_wdt iTCO_vendor_support cryptd snd_hda_codec efivars button acpi_cpufreq snd_hwdep snd_hda_core snd_pcm snd_timer snd lpc_ich mfd_core processor soundcore i2c_i801 [last unloaded: edac_core]
[  159.807818] CPU: 5 PID: 2120 Comm: grep Tainted: G        W       4.3.0-rc5+ #1
[  159.815130] Hardware name: Dell Inc. Precision T3600/0PTTT9, BIOS A13 05/11/2014
[  159.822526] task: ffff880439ef5b00 ti: ffff88001fc18000 task.ti: ffff88001fc18000
[  159.830009] RIP: 0010:[<ffffffff811fda90>]  [<ffffffff811fda90>] do_dentry_open.isra.15+0xe0/0x2d0
[  159.838984] RSP: 0018:ffff88001fc1bcc8  EFLAGS: 00010282
[  159.844291] RAX: ffffffffa0056c80 RBX: ffff8804390996c0 RCX: 0000000000000000
[  159.851418] RDX: ffff880439ef62b0 RSI: 0000000000000000 RDI: 00000000ffffffff
[  159.858545] RBP: ffff88001fc1bcf0 R08: 0000000000000000 R09: 0000000000000000
[  159.865679] R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000000
[  159.872806] R13: ffff88043897baf8 R14: 0000000000008000 R15: ffff8804390996d0
[  159.879935] FS:  00007fa8aae6a700(0000) GS:ffff88043df40000(0000) knlGS:0000000000000000
[  159.888019] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  159.893760] CR2: ffffffffa0056c80 CR3: 0000000037b1e000 CR4: 00000000000406e0
[  159.900894] Stack:
[  159.902903]  ffff8804390996c0 ffff88001fc1bdf0 0000000000000000 0000000000008000
[  159.910357]  ffff88001fc1befc ffff88001fc1bd08 ffffffff811fef02 0000000000000000
[  159.917807]  ffff88001fc1bde0 ffffffff8120e6dd ffffffff810bfdfe ffff880439ef6300
[  159.925253] Call Trace:
[  159.927721]  [<ffffffff811fef02>] vfs_open+0x52/0x60
[  159.932691]  [<ffffffff8120e6dd>] path_openat+0x20d/0x16e0
[  159.938180]  [<ffffffff810bfdfe>] ? __lock_acquire+0x94e/0x2230
[  159.944094]  [<ffffffff810bfdfe>] ? __lock_acquire+0x94e/0x2230
[  159.950018]  [<ffffffff812114de>] do_filp_open+0x7e/0xe0
[  159.955333]  [<ffffffff81220737>] ? expand_files+0x217/0x300
[  159.961560]  [<ffffffff818360c1>] ? _raw_spin_unlock+0x31/0x50
[  159.967952]  [<ffffffff81221181>] ? __alloc_fd+0xb1/0x180
[  159.973899]  [<ffffffff811ff27c>] do_sys_open+0x12c/0x210
[  159.979844]  [<ffffffff811ff394>] SyS_openat+0x14/0x20
[  159.985527]  [<ffffffff81836bb6>] entry_SYSCALL_64_fastpath+0x16/0x7a
[  159.992506] Code: 00 81 4b 74 00 00 01 00 41 0f b7 45 00 66 25 00 f0 66 3d 00 80 0f 84 54 01 00 00 49 8b 85 d8 01 00 00 48 85 c0 0f 84 e0 00 00 00 <48> 8b 38 e8 88 48 f0 ff 84 c0 0f 84 d0 00 00 00 49 8b 85 d8 01 
[  160.012831] RIP  [<ffffffff811fda90>] do_dentry_open.isra.15+0xe0/0x2d0
[  160.020071]  RSP <ffff88001fc1bcc8>
[  160.024164] CR2: ffffffffa0056c80
[  160.028106] ---[ end trace afc84e562a248374 ]---

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.