[RFC, PATCH] sysdev suspend/resume order?

[RFC, PATCH] sysdev suspend/resume order?

[Pallipadi, Venkatesh]

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


 
Problem:
Most of the sysdev_register calls in various devices
are being done using device_initcalls. This method of 
initialization has a side-effect, as we loose the 
ordering requirements for various resume calls. 
Order in which _resume() functions gets called during 
a resume, just depends on the order device_initcalls 
were called on these devices.

Shaohua's mail below describes one issue that is a 
result of this problem, where in pci link device get 
initialized before PIC. There can be more issues 
like this.

One solution to this problem:
Call the sysdev_registers during the actual device 
initialization. Say, i8259 sysdev_register should be 
called when i8259 device is getting registered and 
not with a device_initcall 'sometime' later.

With this, we will be able to save the original 
device initialization order (bootup order), and call 
the resume methods in the same order.

Attached are the couple of sample patches that does 
this in particular files.

Questions:
Is this solution proper?
Is there any better solution to this problem? Say 
introducing some sort of levels in device_initcall.
If the above soultion is proper, is it 
practical/feasible to audit all usages of 
sysdev_register() and try to move it to the 
device initialization function?

Thanks,
Venki

>-----Original Message-----
>From: Pallipadi, Venkatesh 
>Sent: Friday, August 13, 2004 11:58 AM
>To: Li, Shaohua; 'Patrick Mochel'
>Cc: 'acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org'; Brown, Len
>Subject: RE: sysdev suspend/resume order?
>
>
>
>I think, there should be a relation between the order
>in which actual device initialization happens during 
>the boot time and the order in which device initcalls 
>are called (at boot time).
>
>In this example, actual PIC initialization happens 
>before pci_link device initialization. So, device 
>initcall should have a similar ordering. That will 
>take care of this after S3 ordering in a natural manner.
>
>Thanks,
>Venki 
>
>>-----Original Message-----
>>From: Li, Shaohua 
>>Sent: Friday, August 13, 2004 12:37 AM
>>To: Patrick Mochel
>>Cc: acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Brown, Len; Pallipadi, Venkatesh
>>Subject: sysdev suspend/resume order?
>>
>>Patrick,
>>My sound card dies after S3 sometimes. Basically, if it dies 
>>depends on the order of sysdev's resume. If IRQ router resume 
>>before PIC (I use a tricky method to change order. It's 
>>changing the initcall level of the sysfs initialization for 
>>the device), the sound card die. Otherwise, it works. Is it 
>>possible that we add some levels for sysdev? I'd like some 
>>sysdevs resume before other sysdevs. Does this make sense? 
>>
>>Thanks,
>>Shaohua
>>
>

[-- Attachment #2: sysfsinit_i8259.patch --]
[-- Type: application/octet-stream, Size: 865 bytes --]

--- linux-2.6.8.1/arch/i386/kernel/i8259.c.org	2004-08-16 13:44:08.000000000 -0700
+++ linux-2.6.8.1/arch/i386/kernel/i8259.c	2004-08-16 13:54:43.000000000 -0700
@@ -262,8 +262,6 @@ static int __init i8259A_init_sysfs(void
 	return error;
 }
 
-device_initcall(i8259A_init_sysfs);
-
 void init_8259A(int auto_eoi)
 {
 	unsigned long flags;
@@ -303,6 +301,8 @@ void init_8259A(int auto_eoi)
 	outb(cached_slave_mask, PIC_SLAVE_IMR);	  /* restore slave IRQ mask */
 
 	spin_unlock_irqrestore(&i8259A_lock, flags);
+	i8259A_init_sysfs();
+
 }
 
 /*
@@ -400,8 +400,6 @@ static int __init init_timer_sysfs(void)
 	return error;
 }
 
-device_initcall(init_timer_sysfs);
-
 void __init init_IRQ(void)
 {
 	int i;
@@ -432,6 +430,7 @@ void __init init_IRQ(void)
 	 * vector now:
 	 */
 	setup_timer();
+	init_timer_sysfs();
 
 	/*
 	 * External FPU? Set up irq13 if so, for

[-- Attachment #3: sysfsinit_apic.patch --]
[-- Type: application/octet-stream, Size: 801 bytes --]

--- linux-2.6.8.1/arch/i386/kernel/apic.c.org	2004-08-16 13:43:20.000000000 -0700
+++ linux-2.6.8.1/arch/i386/kernel/apic.c	2004-08-16 14:12:01.000000000 -0700
@@ -40,6 +40,7 @@
 #include "io_ports.h"
 
 static void apic_pm_activate(void);
+static int init_lapic_sysfs(void);
 
 void __init apic_intr_init(void)
 {
@@ -477,6 +478,7 @@ void __init setup_local_APIC (void)
 	if (nmi_watchdog == NMI_LOCAL_APIC)
 		setup_apic_nmi_watchdog();
 	apic_pm_activate();
+	init_lapic_sysfs();
 }
 
 #ifdef CONFIG_PM
@@ -602,11 +604,11 @@ static int __init init_lapic_sysfs(void)
 		error = sysdev_register(&device_lapic);
 	return error;
 }
-device_initcall(init_lapic_sysfs);
 
 #else	/* CONFIG_PM */
 
 static void apic_pm_activate(void) { }
+static int init_lapic_sysfs(void) { }
 
 #endif	/* CONFIG_PM */
 

Re: [RFC, PATCH] sysdev suspend/resume order?

[Nathan Bryant]

Pallipadi, Venkatesh wrote:
>  
> Problem:
> Most of the sysdev_register calls in various devices
> are being done using device_initcalls. This method of 
> initialization has a side-effect, as we loose the 
> ordering requirements for various resume calls. 
> Order in which _resume() functions gets called during 
> a resume, just depends on the order device_initcalls 
> were called on these devices.
> 
> Shaohua's mail below describes one issue that is a 
> result of this problem, where in pci link device get 
> initialized before PIC. There can be more issues 
> like this.

Actually, the PCI links are initialized from a subsys_initcall, which is 
not to be confused with a device_initcall, and anyway the PCI links 
don't have their own suspend/resume callbacks unless you're using a 
specific patch that I posted.

Maybe the problem you guys are thinking of has to do with PIC/APIC and 
PIT resume order?

> 
> One solution to this problem:
> Call the sysdev_registers during the actual device 
> initialization. Say, i8259 sysdev_register should be 
> called when i8259 device is getting registered and 
> not with a device_initcall 'sometime' later.
> 
> With this, we will be able to save the original 
> device initialization order (bootup order), and call 
> the resume methods in the same order.
> 
> Attached are the couple of sample patches that does 
> this in particular files.
> 
> Questions:
> Is this solution proper?

I think that's dubious. Haven't looked at your patch yet, but... if you 
call sysdev_register from with an ACPI subsys_initcall you will run into 
some sort of data structure corruption and your kernel will fail to boot.

Also, some of the devices you refer to are initialized from 
subsys_initcall's anyway, and I assume that the order of those is also 
somewhat undefined.

> Is there any better solution to this problem? Say 
> introducing some sort of levels in device_initcall.
> If the above soultion is proper, is it 
> practical/feasible to audit all usages of 
> sysdev_register() and try to move it to the 
> device initialization function?
> 
> Thanks,
> Venki
> 
> 
>>-----Original Message-----
>>From: Pallipadi, Venkatesh 
>>Sent: Friday, August 13, 2004 11:58 AM
>>To: Li, Shaohua; 'Patrick Mochel'
>>Cc: 'acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org'; Brown, Len
>>Subject: RE: sysdev suspend/resume order?
>>
>>
>>
>>I think, there should be a relation between the order
>>in which actual device initialization happens during 
>>the boot time and the order in which device initcalls 
>>are called (at boot time).
>>
>>In this example, actual PIC initialization happens 
>>before pci_link device initialization. So, device 
>>initcall should have a similar ordering. That will 
>>take care of this after S3 ordering in a natural manner.
>>
>>Thanks,
>>Venki 
>>
>>
>>>-----Original Message-----
>>>From: Li, Shaohua 
>>>Sent: Friday, August 13, 2004 12:37 AM
>>>To: Patrick Mochel
>>>Cc: acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Brown, Len; Pallipadi, Venkatesh
>>>Subject: sysdev suspend/resume order?
>>>
>>>Patrick,
>>>My sound card dies after S3 sometimes. Basically, if it dies 
>>>depends on the order of sysdev's resume. If IRQ router resume 
>>>before PIC (I use a tricky method to change order. It's 
>>>changing the initcall level of the sysfs initialization for 
>>>the device), the sound card die. Otherwise, it works. Is it 
>>>possible that we add some levels for sysdev? I'd like some 
>>>sysdevs resume before other sysdevs. Does this make sense? 
>>>
>>>Thanks,
>>>Shaohua
>>>
>>



-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

Re: [RFC, PATCH] sysdev suspend/resume order?

[Nathan Bryant]

Nathan Bryant wrote:
> Pallipadi, Venkatesh wrote:
> 
>>  
>> Problem:
>> Most of the sysdev_register calls in various devices
>> are being done using device_initcalls. This method of initialization 
>> has a side-effect, as we loose the ordering requirements for various 
>> resume calls. Order in which _resume() functions gets called during a 
>> resume, just depends on the order device_initcalls were called on 
>> these devices.
>>
>> Shaohua's mail below describes one issue that is a result of this 
>> problem, where in pci link device get initialized before PIC. There 
>> can be more issues like this.
> 
> 
> Actually, the PCI links are initialized from a subsys_initcall, which is 
> not to be confused with a device_initcall, and anyway the PCI links 
> don't have their own suspend/resume callbacks unless you're using a 
> specific patch that I posted.

Ok, I take that back. Looks like my patch was merged to mainline. Well I 
suppose I can help restructure that. As I mentioned a week or two ago in 
a separate thread, I think it may be a good idea to control these 
init-order issues by combining these IRQrouting-related sys_device 
drivers into a single logical driver, then you handle the ordering 
issues within the driver. I'd like to get some feedback on that idea. If 
Shaohua can tell me precisely what order he wants to see, that would help.

> 
> Maybe the problem you guys are thinking of has to do with PIC/APIC and 
> PIT resume order?
> 
>>
>> One solution to this problem:
>> Call the sysdev_registers during the actual device initialization. 
>> Say, i8259 sysdev_register should be called when i8259 device is 
>> getting registered and not with a device_initcall 'sometime' later.
>>
>> With this, we will be able to save the original device initialization 
>> order (bootup order), and call the resume methods in the same order.
>>
>> Attached are the couple of sample patches that does this in particular 
>> files.
>>
>> Questions:
>> Is this solution proper?
> 
> 
> I think that's dubious. Haven't looked at your patch yet, but... if you 
> call sysdev_register from with an ACPI subsys_initcall you will run into 
> some sort of data structure corruption and your kernel will fail to boot.
> 
> Also, some of the devices you refer to are initialized from 
> subsys_initcall's anyway, and I assume that the order of those is also 
> somewhat undefined.
> 
>> Is there any better solution to this problem? Say introducing some 
>> sort of levels in device_initcall.
>> If the above soultion is proper, is it practical/feasible to audit all 
>> usages of sysdev_register() and try to move it to the device 
>> initialization function?
>>
>> Thanks,
>> Venki
>>
>>
>>> -----Original Message-----
>>> From: Pallipadi, Venkatesh Sent: Friday, August 13, 2004 11:58 AM
>>> To: Li, Shaohua; 'Patrick Mochel'
>>> Cc: 'acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org'; Brown, Len
>>> Subject: RE: sysdev suspend/resume order?
>>>
>>>
>>>
>>> I think, there should be a relation between the order
>>> in which actual device initialization happens during the boot time 
>>> and the order in which device initcalls are called (at boot time).
>>>
>>> In this example, actual PIC initialization happens before pci_link 
>>> device initialization. So, device initcall should have a similar 
>>> ordering. That will take care of this after S3 ordering in a natural 
>>> manner.
>>>
>>> Thanks,
>>> Venki
>>>
>>>> -----Original Message-----
>>>> From: Li, Shaohua Sent: Friday, August 13, 2004 12:37 AM
>>>> To: Patrick Mochel
>>>> Cc: acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Brown, Len; Pallipadi, Venkatesh
>>>> Subject: sysdev suspend/resume order?
>>>>
>>>> Patrick,
>>>> My sound card dies after S3 sometimes. Basically, if it dies depends 
>>>> on the order of sysdev's resume. If IRQ router resume before PIC (I 
>>>> use a tricky method to change order. It's changing the initcall 
>>>> level of the sysfs initialization for the device), the sound card 
>>>> die. Otherwise, it works. Is it possible that we add some levels for 
>>>> sysdev? I'd like some sysdevs resume before other sysdevs. Does this 
>>>> make sense?
>>>> Thanks,
>>>> Shaohua
>>>>
>>>
> 
> 



-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

Re: [RFC, PATCH] sysdev suspend/resume order?

[Pavel Machek]

Hi!

> One solution to this problem:
> Call the sysdev_registers during the actual device 
> initialization. Say, i8259 sysdev_register should be 
> called when i8259 device is getting registered and 
> not with a device_initcall 'sometime' later.

Okay, nice trick. At the very least, we should document somewhere
that order of sysdev_register matters and that we depend on it.

> Questions:
> Is this solution proper?
> Is there any better solution to this problem? Say 
> introducing some sort of levels in device_initcall.
> If the above soultion is proper, is it 
> practical/feasible to audit all usages of 
> sysdev_register() and try to move it to the 
> device initialization function?

I'd hope that for most system devices, order does not matter and
we can only audit/fix those that matter.

Patrick may have nicer solution in his sleeve, through.
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         



-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

RE: [RFC, PATCH] sysdev suspend/resume order?

[Pallipadi, Venkatesh]

 

>-----Original Message-----
>From: Nathan Bryant [mailto:nbryant-p32f3XyCuykqcZcGjlUOXw@public.gmane.org] 
>Sent: Monday, August 16, 2004 12:40 PM
>To: Nathan Bryant
>Cc: Pallipadi, Venkatesh; Li, Shaohua; Patrick Mochel; Pavel 
>Machek; acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Brown, Len
>Subject: Re: [ACPI] [RFC, PATCH] sysdev suspend/resume order?
>
>Nathan Bryant wrote:
>> Pallipadi, Venkatesh wrote:
>> 
>>>  
>>> Problem:
>>> Most of the sysdev_register calls in various devices
>>> are being done using device_initcalls. This method of 
>initialization 
>>> has a side-effect, as we loose the ordering requirements 
>for various 
>>> resume calls. Order in which _resume() functions gets 
>called during a 
>>> resume, just depends on the order device_initcalls were called on 
>>> these devices.
>>>
>>> Shaohua's mail below describes one issue that is a result of this 
>>> problem, where in pci link device get initialized before PIC. There 
>>> can be more issues like this.
>> 
>> 
>> Actually, the PCI links are initialized from a 
>subsys_initcall, which is 
>> not to be confused with a device_initcall, and anyway the PCI links 
>> don't have their own suspend/resume callbacks unless you're using a 
>> specific patch that I posted.
>
>Ok, I take that back. Looks like my patch was merged to 
>mainline. Well I 
>suppose I can help restructure that. As I mentioned a week or 
>two ago in 
>a separate thread, I think it may be a good idea to control these 
>init-order issues by combining these IRQrouting-related sys_device 
>drivers into a single logical driver, then you handle the ordering 
>issues within the driver. I'd like to get some feedback on 
>that idea. If 
>Shaohua can tell me precisely what order he wants to see, that 
>would help.

I think the basic resume ordering should be 
- APIC (if enabled)
- IOAPIC (or PIC)
- Link device
- All drivers

I somehow feel doing it in a single driver may increase the complexity
of this whole thing. Especially with various APIC/IOAPIC configs and 
boot time parameters.

Thanks,
Venki



-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

Re: [RFC, PATCH] sysdev suspend/resume order?

[Nathan Bryant]

Pallipadi, Venkatesh wrote:

> I think the basic resume ordering should be 
> - APIC (if enabled)

You mean Local APIC?

> - IOAPIC (or PIC)

We still use the i8259 in APIC mode for some things. (Some motherboards 
  don't connect certain IOAPIC pins, like IRQ0... I think there may be 
other uses) Does that need to be before or after the local apic? Before 
or after the IO-APIC?

> - Link device
> - All drivers

Makes sense

But where does PIT fit into the order? It has its own sys_device, and 
was, I think, specifically reported by one user to be causing a problem. 
If the following message in the archives is accurate, it might help, but 
it's a little short on details of how he tracked this down: 
http://marc.theaimsgroup.com/?l=acpi4linux&m=109180681500139&w=2

Matthew, care to weigh in? I'm fuzzy on the details of what patches 
broke what, on your system. (That is to say, who did what to who.)

> 
> I somehow feel doing it in a single driver may increase the complexity
> of this whole thing. Especially with various APIC/IOAPIC configs and 
> boot time parameters.

You may be right. Part of the problem is that some of the affected code 
lives in arch/... and some of it lives in drivers/acpi/... That 
certainly makes cleaning up require a little more forethought.

Nathan

> 
> Thanks,
> Venki
> 
> 



-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

RE: [RFC, PATCH] sysdev suspend/resume order?

[Pallipadi, Venkatesh]

 
>-----Original Message-----
>From: Nathan Bryant [mailto:nbryant-p32f3XyCuykqcZcGjlUOXw@public.gmane.org] 
>Sent: Monday, August 16, 2004 1:48 PM
>To: Pallipadi, Venkatesh; Matthew Garrett
>Cc: Li, Shaohua; Patrick Mochel; Pavel Machek; 
>acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Brown, Len
>Subject: Re: [ACPI] [RFC, PATCH] sysdev suspend/resume order?
>
>Pallipadi, Venkatesh wrote:
>
>> I think the basic resume ordering should be 
>> - APIC (if enabled)
>
>You mean Local APIC?
>
>> - IOAPIC (or PIC)
>
>We still use the i8259 in APIC mode for some things. (Some 
>motherboards 
>  don't connect certain IOAPIC pins, like IRQ0... I think there may be 
>other uses) Does that need to be before or after the local 
>apic? Before 
>or after the IO-APIC?

OK. Another attempt at listing all the devices that need to have 
some ordering during resume

- Local APIC
- i8259
- IOAPIC
- PIT
- Interrupt link device
- All drivers

Thanks,
Venki


-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

RE: [RFC, PATCH] sysdev suspend/resume order?

[Li, Shaohua]

Pallipadi, Venkatesh wrote:
>To: Li, Shaohua; 'Patrick Mochel'; Pavel Machek
>Cc: 'acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org'; Brown, Len
>Subject: [RFC, PATCH] sysdev suspend/resume order?
>
>One solution to this problem:
>Call the sysdev_registers during the actual device
>initialization. Say, i8259 sysdev_register should be
>called when i8259 device is getting registered and
>not with a device_initcall 'sometime' later.
>
>With this, we will be able to save the original
>device initialization order (bootup order), and call
>the resume methods in the same order.
>
>Attached are the couple of sample patches that does
>this in particular files.
>
>Questions:
>Is this solution proper?
>Is there any better solution to this problem? Say
>introducing some sort of levels in device_initcall.
>If the above soultion is proper, is it
>practical/feasible to audit all usages of
>sysdev_register()
I think we should check 'sysdev_class_register', when sysdev_register is
called doesn't affect sysdev's order. But maybe my previous email
misleads you.

>and try to move it to the
>device initialization function?
I have the same concern as Nathan: does this cause some data corrupt?
sysdev_class_register must be called after device driver core is
initialized (after 'driver_init', which is invoked at 'do_basic_setup').
Some sysdev are initialized before that.

Thanks,
Shaohua


-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285