Patchwork WARNINGs because of ide-proc handling

login
register
mail settings
Submitter Wolfram Sang
Date Sept. 2, 2010, 9:28 p.m.
Message ID <20100902212839.GA20070@pengutronix.de>
Download mbox | patch
Permalink /patch/63557/
State Accepted
Delegated to: David Miller
Headers show

Comments

Wolfram Sang - Sept. 2, 2010, 9:28 p.m.
On Wed, Sep 01, 2010 at 07:33:32PM -0700, David Miller wrote:

Adding Bartlomiej to CC, maybe he has something to add :)

> From: Wolfram Sang <w.sang@pengutronix.de>
> Date: Tue, 31 Aug 2010 22:56:05 +0200
> 
> > Hello,
> > 
> > I get the following warning (and some similar ones) on my laptop running
> > 2.6.36-rc3 when removing a CF card via pcmcia:
> > 
> > [ 1087.602129] WARNING: at fs/proc/generic.c:816 remove_proc_entry+0x185/0x1d0()
> > [ 1087.602137] Hardware name: Amilo M14255
> > [ 1087.602142] name 'cache'
> > ...
> > 
> > It turned out that none of the entries in ide_disk_proc[] gets created when
> > inserting the card, so trying to remove them triggers this warning. Adding
> > some debug printouts
> 
> Hmmm, at some point ide_host_add() or ide_host_register() should be invoked,
> which should create the drive->proc and hwif->proc PROCFS directories.
> 
> What driver is your pcmcia IDE card using?

ide-cs.

I did some more debugging and have a function_graph-trace which proves the
following callchain:

ide_host_add -> ... -> ide_host_register

This calls _first_ hwif_register_devices() and only _later_
ide_proc_register_port() and ide_proc_port_register_devices() which are
responsible for creating the needed directories. In hwif_register_devices(), a
device gets created and immediately attached to the driver, ide-gd in this
case. The probe-function calls disk_ops->setup() which wants to create the
proc-entries, but the directories are not there yet. This patch helps,
combining two loops and moving hwif_register_devices() to the end:


Dunno, if it is correct, though. The strange thing is that the code is like
this for two years and, surprisingly, I don't see the problem with 2.6.33.
Also, the proc-entry for my hard-disk is fine. I can't see any suspicious
commits in the ide-directory except maybe the BKL pushdown had a side-effect
regarding the serialization of the sequence? Ah, too late today to seriously
think about such issues...

Thanks for the input,

   Wolfram
David Miller - Sept. 2, 2010, 10:14 p.m.
From: Wolfram Sang <w.sang@pengutronix.de>
Date: Thu, 2 Sep 2010 23:28:39 +0200

> Dunno, if it is correct, though. The strange thing is that the code is like
> this for two years and, surprisingly, I don't see the problem with 2.6.33.
> Also, the proc-entry for my hard-disk is fine. I can't see any suspicious
> commits in the ide-directory except maybe the BKL pushdown had a side-effect
> regarding the serialization of the sequence? Ah, too late today to seriously
> think about such issues...

I don't think it's the BKL pushdown, it seems it would be due to something
else to me.

Thanks for your analysis and suggested patch, I'll take a look at this.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang - Sept. 3, 2010, 5:53 a.m.
> Thanks for your analysis and suggested patch, I'll take a look at this.

Thanks for your help. If we can't find something, I'll just do a bisect this
weekend.
David Miller - Sept. 3, 2010, 12:34 p.m.
From: Wolfram Sang <w.sang@pengutronix.de>
Date: Fri, 3 Sep 2010 07:53:28 +0200

> 
>> Thanks for your analysis and suggested patch, I'll take a look at this.
> 
> Thanks for your help. If we can't find something, I'll just do a bisect this
> weekend.

Frankly, at this point, I'm beginning suspect that the procfs stuff
simply never warned at some point in the past and that the IDE
code has had this problem for a while.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartlomiej Zolnierkiewicz - Sept. 3, 2010, 2:21 p.m.
On Friday 03 September 2010 02:34:37 pm David Miller wrote:
> From: Wolfram Sang <w.sang@pengutronix.de>
> Date: Fri, 3 Sep 2010 07:53:28 +0200
> 
> > 
> >> Thanks for your analysis and suggested patch, I'll take a look at this.
> > 
> > Thanks for your help. If we can't find something, I'll just do a bisect this
> > weekend.
> 
> Frankly, at this point, I'm beginning suspect that the procfs stuff
> simply never warned at some point in the past and that the IDE
> code has had this problem for a while.

JFYI deprecated procfs stuff has been long gone from atang tree..

Thanks.
--
Bartlomiej Zolnierkiewicz
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
index 4c3d1bf..068cef0 100644
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -1448,19 +1448,13 @@  int ide_host_register(struct ide_host *host, const struct ide_port_info *d,
 		if (hwif == NULL)
 			continue;
 
-		if (hwif->present)
-			hwif_register_devices(hwif);
-	}
-
-	ide_host_for_each_port(i, hwif, host) {
-		if (hwif == NULL)
-			continue;
-
 		ide_sysfs_register_port(hwif);
 		ide_proc_register_port(hwif);
 
-		if (hwif->present)
+		if (hwif->present) {
 			ide_proc_port_register_devices(hwif);
+			hwif_register_devices(hwif);
+		}
 	}
 
 	return j ? 0 : -1;