rc3-mm1 - /proc/ide/HWIF for modular IDE

PATCH In review Andrey Borzenkov 2 participants 6 messages Feb 3, 2004 – Feb 22, 2004

Share & tools

Andrey Borzenkov

author

2004-02-03 19:53 UTC #0 lore raw

PATCH 1/1 rc3-mm1 - /proc/ide/HWIF for modular IDE

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

drivers/ide/ide-proc.c renamed from drivers/ide/ide-proc.c.modular +23 ~~-13~~

@@ -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);

drivers/ide/setup-pci.c renamed from drivers/ide/setup-pci.c.modular +4 -0

@@ -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;
 	}

include/linux/ide.h renamed from include/linux/ide.h.modular +1 -0

@@ -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 *);

Bartlomiej Zolnierkiewicz

2004-02-03 20:35 UTC #1 lore raw

Quoted 11 lines · Andrey Borzenkov

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

Andrey Borzenkov

author

2004-02-04 20:29 UTC #2 lore raw

Quoted 40 lines · Bartlomiej Zolnierkiewicz

Quoted 11 lines · Andrey Borzenkov
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
(quoted diff) +6 -0
@@ -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.
(quoted diff) +4 -0
@@ -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;
 	}

Quoted 2 lines

Same problem as above.

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

Quoted 3 lines

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>

Quoted 3 lines

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

Quoted 2 lines

--bart

drivers/ide/setup-pci.c renamed from drivers/ide/setup-pci.c.modular +8 -0

@@ -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);

Bartlomiej Zolnierkiewicz

2004-02-04 21:54 UTC #3 lore raw

Quoted 26 lines · Andrey Borzenkov

Quoted 25 lines · Bartlomiej Zolnierkiewicz
Quoted 11 lines · Andrey Borzenkov
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
(quoted diff) +6 -0
@@ -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;
+	}
 }
 

Quoted 2 lines

Quoted 2 lines
(quoted diff) +0 -0
 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.

Quoted 12 lines

(quoted diff) +4 -0

@@ -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;
 	}

Quoted 4 lines

Quoted 1 line
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.

Quoted 9 lines

Quoted 2 lines
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>
Quoted 2 lines
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

Andrey Borzenkov

author

2004-02-05 21:01 UTC #4 lore raw

Quoted 19 lines · Bartlomiej Zolnierkiewicz

Quoted 3 lines
Quoted 3 lines
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.
Quoted 12 lines
Quoted 12 lines
(quoted diff) +4 -0
@@ -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;
 	}

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
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

Quoted 8 lines

Quoted 8 lines
Quoted 2 lines
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>
Quoted 2 lines
Even better - you may fix every PCI driver to add these entries itself and remove these silly ide_pci_host_proc_t-s :-).

Quoted 8 lines

Quoted 1 line
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

drivers/ide/ide-proc.c renamed from drivers/ide/ide-proc.c.modular +7 -0

@@ -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);

drivers/ide/setup-pci.c renamed from drivers/ide/setup-pci.c.modular +8 -0

@@ -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);

Andrey Borzenkov

author

2004-02-22 09:51 UTC #5 lore raw

PATCH 1/1 /proc/ide/HWIF for modular IDE

In reply to Bartlomiej Zolnierkiewicz

Quoted 6 lines · Bartlomiej Zolnierkiewicz

Quoted 6 lines
Quoted 3 lines
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

drivers/ide/setup-pci.c renamed from drivers/ide/setup-pci.c.modular +8 -0

@@ -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);