[PATCH] rc3-mm1 - /proc/ide/HWIF for modular IDE

[PATCH] rc3-mm1 - /proc/ide/HWIF for modular IDE

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 437 bytes --]

currently /proc/ide/HWIF are created in one shot during initialization
or in ide-generic meaning that for modular IDE you must include
ide-generic.

this adds per-hwif registration currently for PCI only (that is what I
can test); if this is OK I will make create_proc_ide_interfaces static
and replace it with create_proc_ide_interface where appropriate.

this also makes /proc/ide entries for PCI chipset be correctly created

-andrey

[-- Attachment #2: 2.6.2-rc3-mm1-modular_proc_ide.patch --]
[-- Type: text/plain, Size: 2429 bytes --]

--- linux-2.6.2-rc3-mm1/drivers/ide/ide-proc.c.modular	2004-02-03 20:37:32.000000000 +0300
+++ linux-2.6.2-rc3-mm1/drivers/ide/ide-proc.c	2004-02-03 21:13:38.000000000 +0300
@@ -768,23 +768,27 @@ static ide_proc_entry_t hwif_entries[] =
 	{ NULL,	0, NULL, NULL }
 };
 
+void create_proc_ide_interface(ide_hwif_t *hwif)
+{
+	if (!hwif->present)
+		return;
+	if (!hwif->proc) {
+		hwif->proc = proc_mkdir(hwif->name, proc_ide_root);
+		if (!hwif->proc)
+			return;
+		ide_add_proc_entries(hwif->proc, hwif_entries, hwif);
+	}
+	create_proc_ide_drives(hwif);
+}
+
+EXPORT_SYMBOL(create_proc_ide_interface);
+
 void create_proc_ide_interfaces(void)
 {
 	int	h;
 
-	for (h = 0; h < MAX_HWIFS; h++) {
-		ide_hwif_t *hwif = &ide_hwifs[h];
-
-		if (!hwif->present)
-			continue;
-		if (!hwif->proc) {
-			hwif->proc = proc_mkdir(hwif->name, proc_ide_root);
-			if (!hwif->proc)
-				return;
-			ide_add_proc_entries(hwif->proc, hwif_entries, hwif);
-		}
-		create_proc_ide_drives(hwif);
-	}
+	for (h = 0; h < MAX_HWIFS; h++)
+		create_proc_ide_interface(&ide_hwifs[h]);
 }
 
 #ifdef CONFIG_BLK_DEV_IDEPCI
@@ -801,6 +805,12 @@ void ide_pci_register_host_proc (ide_pci
 		tmp->next = p;
 	} else
 		ide_pci_host_proc_list = p;
+
+	if (proc_ide_root) {
+		p->parent = proc_ide_root;
+		create_proc_info_entry(p->name, 0, p->parent, p->get_info);
+		p->set = 2;
+	}
 }
 
 EXPORT_SYMBOL(ide_pci_register_host_proc);
--- linux-2.6.2-rc3-mm1/drivers/ide/setup-pci.c.modular	2004-02-03 20:37:33.000000000 +0300
+++ linux-2.6.2-rc3-mm1/drivers/ide/setup-pci.c	2004-02-03 21:11:22.000000000 +0300
@@ -659,6 +659,10 @@ bypass_legacy_dma:
 			 */
 			d->init_hwif(hwif);
 
+#ifdef CONFIG_PROC_FS
+		create_proc_ide_interface(hwif);
+#endif
+
 		mate = hwif;
 		at_least_one_hwif_enabled = 1;
 	}
--- linux-2.6.2-rc3-mm1/include/linux/ide.h.modular	2004-02-03 20:38:06.000000000 +0300
+++ linux-2.6.2-rc3-mm1/include/linux/ide.h	2004-02-03 21:15:33.000000000 +0300
@@ -1117,6 +1117,7 @@ extern void proc_ide_create(void);
 extern void proc_ide_destroy(void);
 extern void destroy_proc_ide_device(ide_hwif_t *, ide_drive_t *);
 extern void destroy_proc_ide_drives(ide_hwif_t *);
+extern void create_proc_ide_interface(ide_hwif_t *);
 extern void create_proc_ide_interfaces(void);
 extern void ide_add_proc_entries(struct proc_dir_entry *, ide_proc_entry_t *, void *);
 extern void ide_remove_proc_entries(struct proc_dir_entry *, ide_proc_entry_t *);

Re: [PATCH] rc3-mm1 - /proc/ide/HWIF for modular IDE

[Bartlomiej Zolnierkiewicz]

On Tuesday 03 of February 2004 20:48, Andrey Borzenkov wrote:
> currently /proc/ide/HWIF are created in one shot during initialization
> or in ide-generic meaning that for modular IDE you must include
> ide-generic.
>
> this adds per-hwif registration currently for PCI only (that is what I
> can test); if this is OK I will make create_proc_ide_interfaces static
> and replace it with create_proc_ide_interface where appropriate.
>
> this also makes /proc/ide entries for PCI chipset be correctly created
>
> -andrey

@@ -801,6 +805,12 @@ void ide_pci_register_host_proc (ide_pci
 		tmp->next = p;
 	} else
 		ide_pci_host_proc_list = p;
+
+	if (proc_ide_root) {
+		p->parent = proc_ide_root;
+		create_proc_info_entry(p->name, 0, p->parent, p->get_info);
+		p->set = 2;
+	}
 }
 
You should add p->get_info only _after_ all hwifs of a host are probed,
just like non-modular code does it.  Otherwise you are opening new races.

@@ -659,6 +659,10 @@ bypass_legacy_dma:
 			 */
 			d->init_hwif(hwif);
 
+#ifdef CONFIG_PROC_FS
+		create_proc_ide_interface(hwif);
+#endif
+
 		mate = hwif;
 		at_least_one_hwif_enabled = 1;
 	}

Same problem as above.

ide_setup_pci_device()+ide_setup_pci_devices() are correct places
to add registering of /proc/ide/<chipset> and /proc/ide/<hwif>.

Even better - you may fix every PCI driver to add these entries
itself and remove these silly ide_pci_host_proc_t-s :-).

--bart

Re: [PATCH] rc3-mm1 - /proc/ide/HWIF for modular IDE

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 1790 bytes --]

On Tue, Feb 03, 2004 at 09:39:24PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Tuesday 03 of February 2004 20:48, Andrey Borzenkov wrote:
> > currently /proc/ide/HWIF are created in one shot during initialization
> > or in ide-generic meaning that for modular IDE you must include
> > ide-generic.
> >
> > this adds per-hwif registration currently for PCI only (that is what I
> > can test); if this is OK I will make create_proc_ide_interfaces static
> > and replace it with create_proc_ide_interface where appropriate.
> >
> > this also makes /proc/ide entries for PCI chipset be correctly created
> >
> > -andrey
> 
> @@ -801,6 +805,12 @@ void ide_pci_register_host_proc (ide_pci
>  		tmp->next = p;
>  	} else
>  		ide_pci_host_proc_list = p;
> +
> +	if (proc_ide_root) {
> +		p->parent = proc_ide_root;
> +		create_proc_info_entry(p->name, 0, p->parent, p->get_info);
> +		p->set = 2;
> +	}
>  }
>  
> You should add p->get_info only _after_ all hwifs of a host are probed,
> just like non-modular code does it.  Otherwise you are opening new races.
> 
> @@ -659,6 +659,10 @@ bypass_legacy_dma:
>  			 */
>  			d->init_hwif(hwif);
>  
> +#ifdef CONFIG_PROC_FS
> +		create_proc_ide_interface(hwif);
> +#endif
> +
>  		mate = hwif;
>  		at_least_one_hwif_enabled = 1;
>  	}
> 
> Same problem as above.
> 

oh :( is it possible to do it in probe_hwif_init? it would be most
logical place.

> ide_setup_pci_device()+ide_setup_pci_devices() are correct places
> to add registering of /proc/ide/<chipset> and /proc/ide/<hwif>.
> 

this patch does it for <hwif>

> Even better - you may fix every PCI driver to add these entries
> itself and remove these silly ide_pci_host_proc_t-s :-).
> 

I'll see. what are those races and are they inherently unfixable?

thank you

-andrey

> --bart
> 

[-- Attachment #2: 2.6.2-rc3-mm1-modular_proc_ide.patch --]
[-- Type: text/plain, Size: 814 bytes --]

--- linux-2.6.2-rc3-mm1/drivers/ide/setup-pci.c.modular	2004-02-04 22:14:43.000000000 +0300
+++ linux-2.6.2-rc3-mm1/drivers/ide/setup-pci.c	2004-02-04 22:23:24.000000000 +0300
@@ -746,6 +746,10 @@ void ide_setup_pci_device (struct pci_de
 		probe_hwif_init(&ide_hwifs[index_list.b.low]);
 	if ((index_list.b.high & 0xf0) != 0xf0)
 		probe_hwif_init(&ide_hwifs[index_list.b.high]);
+
+#ifdef CONFIG_PROC_FS
+	create_proc_ide_interfaces();
+#endif
 }
 
 EXPORT_SYMBOL_GPL(ide_setup_pci_device);
@@ -763,6 +767,10 @@ void ide_setup_pci_devices (struct pci_d
 		probe_hwif_init(&ide_hwifs[index_list2.b.low]);
 	if ((index_list2.b.high & 0xf0) != 0xf0)
 		probe_hwif_init(&ide_hwifs[index_list2.b.high]);
+
+#ifdef CONFIG_PROC_FS
+	create_proc_ide_interfaces();
+#endif
 }
 
 EXPORT_SYMBOL_GPL(ide_setup_pci_devices);

Re: [PATCH] rc3-mm1 - /proc/ide/HWIF for modular IDE

[Bartlomiej Zolnierkiewicz]

On Wednesday 04 of February 2004 20:44, Andrey Borzenkov wrote:
> On Tue, Feb 03, 2004 at 09:39:24PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Tuesday 03 of February 2004 20:48, Andrey Borzenkov wrote:
> > > currently /proc/ide/HWIF are created in one shot during initialization
> > > or in ide-generic meaning that for modular IDE you must include
> > > ide-generic.
> > >
> > > this adds per-hwif registration currently for PCI only (that is what I
> > > can test); if this is OK I will make create_proc_ide_interfaces static
> > > and replace it with create_proc_ide_interface where appropriate.
> > >
> > > this also makes /proc/ide entries for PCI chipset be correctly created
> > >
> > > -andrey
> >
> > @@ -801,6 +805,12 @@ void ide_pci_register_host_proc (ide_pci
> >  		tmp->next = p;
> >  	} else
> >  		ide_pci_host_proc_list = p;
> > +
> > +	if (proc_ide_root) {
> > +		p->parent = proc_ide_root;
> > +		create_proc_info_entry(p->name, 0, p->parent, p->get_info);
> > +		p->set = 2;
> > +	}
> >  }
> >
> > You should add p->get_info only _after_ all hwifs of a host are probed,
> > just like non-modular code does it.  Otherwise you are opening new races.

My previous comment is (probably) wrong :-).
I've just checked all PCI drivers and don't see anything preventing this.

> > @@ -659,6 +659,10 @@ bypass_legacy_dma:
> >  			 */
> >  			d->init_hwif(hwif);
> >
> > +#ifdef CONFIG_PROC_FS
> > +		create_proc_ide_interface(hwif);
> > +#endif
> > +
> >  		mate = hwif;
> >  		at_least_one_hwif_enabled = 1;
> >  	}
> >
> > Same problem as above.
>
> oh :( is it possible to do it in probe_hwif_init? it would be most
> logical place.

It is not logical place - you got 1 <chipset> /proc entry per PCI device(s).
ide_pci_register_host_proc() (as done in first patch) is more logical.

> > ide_setup_pci_device()+ide_setup_pci_devices() are correct places
> > to add registering of /proc/ide/<chipset> and /proc/ide/<hwif>.
>
> this patch does it for <hwif>
>
> > Even better - you may fix every PCI driver to add these entries
> > itself and remove these silly ide_pci_host_proc_t-s :-).
>
> I'll see. what are those races and are they inherently unfixable?

ie. if <hwif> entry is registered before second (serialized) port is probed,
see proc_ide_write_config() for details.

Thanks,
--bart

Re: [PATCH] rc3-mm1 - /proc/ide/HWIF for modular IDE

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 1665 bytes --]

On Wed, Feb 04, 2004 at 10:55:33PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > >
> > > You should add p->get_info only _after_ all hwifs of a host are probed,
> > > just like non-modular code does it.  Otherwise you are opening new races.
> 
> My previous comment is (probably) wrong :-).
> I've just checked all PCI drivers and don't see anything preventing this.
> 
> > > @@ -659,6 +659,10 @@ bypass_legacy_dma:
> > >  			 */
> > >  			d->init_hwif(hwif);
> > >
> > > +#ifdef CONFIG_PROC_FS
> > > +		create_proc_ide_interface(hwif);
> > > +#endif
> > > +
> > >  		mate = hwif;
> > >  		at_least_one_hwif_enabled = 1;
> > >  	}
> > >
> > > Same problem as above.
> >
> > oh :( is it possible to do it in probe_hwif_init? it would be most
> > logical place.
> 
> It is not logical place - you got 1 <chipset> /proc entry per PCI device(s).
> ide_pci_register_host_proc() (as done in first patch) is more logical.
> 

I actually meant  hwif no chipset  but if you say it is racy anyway ...

so what about this version? it gives me both hwif and chipset and does
not require messing with all pci drivers.

thank you

-andrey

> > > ide_setup_pci_device()+ide_setup_pci_devices() are correct places
> > > to add registering of /proc/ide/<chipset> and /proc/ide/<hwif>.
> >
> > this patch does it for <hwif>
> >
> > > Even better - you may fix every PCI driver to add these entries
> > > itself and remove these silly ide_pci_host_proc_t-s :-).
> >
> > I'll see. what are those races and are they inherently unfixable?
> 
> ie. if <hwif> entry is registered before second (serialized) port is probed,
> see proc_ide_write_config() for details.
> 
> Thanks,
> --bart
> 

[-- Attachment #2: 2.6.2-mm1-modular_proc_ide.patch --]
[-- Type: text/plain, Size: 1280 bytes --]

--- linux-2.6.2-mm1/drivers/ide/ide-proc.c.modular	2004-02-04 23:30:42.000000000 +0300
+++ linux-2.6.2-mm1/drivers/ide/ide-proc.c	2004-02-05 23:02:54.000000000 +0300
@@ -801,6 +801,13 @@ void ide_pci_register_host_proc (ide_pci
 		tmp->next = p;
 	} else
 		ide_pci_host_proc_list = p;
+
+	if (proc_ide_root) {
+		p->parent = proc_ide_root;
+		create_proc_info_entry(p->name, 0, p->parent, p->get_info);
+		p->set = 2;
+	}
+
 }
 
 EXPORT_SYMBOL(ide_pci_register_host_proc);
--- linux-2.6.2-mm1/drivers/ide/setup-pci.c.modular	2004-02-05 22:59:48.000000000 +0300
+++ linux-2.6.2-mm1/drivers/ide/setup-pci.c	2004-02-05 22:59:54.000000000 +0300
@@ -746,6 +746,10 @@ void ide_setup_pci_device (struct pci_de
 		probe_hwif_init(&ide_hwifs[index_list.b.low]);
 	if ((index_list.b.high & 0xf0) != 0xf0)
 		probe_hwif_init(&ide_hwifs[index_list.b.high]);
+
+#ifdef CONFIG_PROC_FS
+	create_proc_ide_interfaces();
+#endif
 }
 
 EXPORT_SYMBOL_GPL(ide_setup_pci_device);
@@ -763,6 +767,10 @@ void ide_setup_pci_devices (struct pci_d
 		probe_hwif_init(&ide_hwifs[index_list2.b.low]);
 	if ((index_list2.b.high & 0xf0) != 0xf0)
 		probe_hwif_init(&ide_hwifs[index_list2.b.high]);
+
+#ifdef CONFIG_PROC_FS
+	create_proc_ide_interfaces();
+#endif
 }
 
 EXPORT_SYMBOL_GPL(ide_setup_pci_devices);

[PATCH][2.6.3-mm2] /proc/ide/HWIF for modular IDE

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 381 bytes --]

On Wed, Feb 04, 2004 at 10:55:33PM +0100, Bartlomiej Zolnierkiewicz wrote:
> 
> > > ide_setup_pci_device()+ide_setup_pci_devices() are correct places
> > > to add registering of /proc/ide/<chipset> and /proc/ide/<hwif>.
> >
> > this patch does it for <hwif>
> >

2.6.3-mm2 now makes /proc/ide/CHIPSET appear but /proc/ide/HWIF are
still missing.

Updated patch attached.

-andrey 

[-- Attachment #2: 2.6.3-mm2-modular_proc_ide.patch --]
[-- Type: text/plain, Size: 806 bytes --]

--- linux-2.6.2-mm1/drivers/ide/setup-pci.c.modular	2004-02-05 22:59:48.000000000 +0300
+++ linux-2.6.2-mm1/drivers/ide/setup-pci.c	2004-02-05 22:59:54.000000000 +0300
@@ -746,6 +746,10 @@ void ide_setup_pci_device (struct pci_de
 		probe_hwif_init(&ide_hwifs[index_list.b.low]);
 	if ((index_list.b.high & 0xf0) != 0xf0)
 		probe_hwif_init(&ide_hwifs[index_list.b.high]);
+
+#ifdef CONFIG_PROC_FS
+	create_proc_ide_interfaces();
+#endif
 }
 
 EXPORT_SYMBOL_GPL(ide_setup_pci_device);
@@ -763,6 +767,10 @@ void ide_setup_pci_devices (struct pci_d
 		probe_hwif_init(&ide_hwifs[index_list2.b.low]);
 	if ((index_list2.b.high & 0xf0) != 0xf0)
 		probe_hwif_init(&ide_hwifs[index_list2.b.high]);
+
+#ifdef CONFIG_PROC_FS
+	create_proc_ide_interfaces();
+#endif
 }
 
 EXPORT_SYMBOL_GPL(ide_setup_pci_devices);