Bug 55831

Summary:	[TRIVIAL][block/mtip32xx] Pointless warning when debugfs is disabled
Product:	Drivers	Reporter:	Lauri Kasanen (curaga)
Component:	Other	Assignee:	drivers_other
Status:	NEW ---
Severity:	low	CC:	alan, danielcotton.bugtrackers
Priority:	P1
Hardware:	All
OS:	Linux
Kernel Version:	git, 3.8	Subsystem:
Regression:	No	Bisected commit-id:

Description Lauri Kasanen 2013-03-27 13:37:30 UTC

The mtip32xx block driver always shows a warning at load time:
"Error creating debugfs parent"

It shouldn't be shown if debugfs was not included in the kernel.

Comment 1 Daniel Cotton 2014-10-08 11:05:43 UTC

I'm not sure, but this might be related to the erroneous usage of IS_ERR_OR_NULL as opposed to IS_ERR. They're set to null in all three cases anyway. I'd be happy to put a patch together for it.

drivers/mtip32xx/mtip32xx.c:
2761:
if (IS_ERR_OR_NULL(dd->dfs_node)) {
        dev_warn(&dd->pdev->dev,
                "Error creating node %s under debugfs\n",
                                        dd->disk->disk_name);
        dd->dfs_node = NULL;
        return -1;
}


4689:

if (IS_ERR_OR_NULL(dfs_parent)) {
        pr_warn("Error creating debugfs parent\n");
        dfs_parent = NULL;
}   

4697:

if (IS_ERR_OR_NULL(dfs_device_status)) {
        pr_err("Error creating device_status node\n");
        dfs_device_status = NULL;
}

Comment 2 Alan 2014-10-08 14:31:03 UTC

Go for it, happy to review

Comment 3 Daniel Cotton 2014-10-09 08:17:52 UTC

Okay, it wasn't that. This patch removes the pointless warning and gives an error code in case a real issue is encountered. I'd really appreciate it if you reviewed this; I've never contributed to the kernel before. If it looks okay to you, I'll submit it.

diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index 5c8e7fe..b11b9f4 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -4686,16 +4686,20 @@ static int __init mtip_init(void)
 	mtip_major = error;
 
 	dfs_parent = debugfs_create_dir("rssd", NULL);
-	if (IS_ERR_OR_NULL(dfs_parent)) {
-		pr_warn("Error creating debugfs parent\n");
+	if (IS_ERR_OR_NULL(dfs_parent) && PTR_ERR(dfs_parent) != -ENODEV) {
+		pr_warn("Error creating debugfs parent, error code: %ld\n",
+			PTR_ERR(dfs_parent));
 		dfs_parent = NULL;
 	}
 	if (dfs_parent) {
 		dfs_device_status = debugfs_create_file("device_status",
 					S_IRUGO, dfs_parent, NULL,
 					&mtip_device_status_fops);
-		if (IS_ERR_OR_NULL(dfs_device_status)) {
-			pr_err("Error creating device_status node\n");
+		if (IS_ERR_OR_NULL(dfs_device_status) &&
+		    PTR_ERR(dfs_device_status) != -ENODEV) {
+			pr_err("Error creating device_status node, " \
+				"error code: %ld\n",
+				PTR_ERR(dfs_device_status));
 			dfs_device_status = NULL;
 		}
 	}

Comment 4 Alan 2014-10-09 15:24:25 UTC

I would only make one change - some of the codingstyle tools like to bitch about > 80 char lines, but they wrongly suggest splitting text strings. That then makes them hard to grep for so I would unsplit the pr_err. The only other thing it then needs is an explanation at the top and a Signed-off-by: line

Comment 5 Daniel Cotton 2014-10-09 16:14:00 UTC

Corrected and patch submitted. Thanks for your help Alan.