re: ehci: fix error handling?
> Ok, thanks, I've fixed it and the machine shuts down correctly now. i saw the commit - thanks for making it better. > However there seem to be other problems in the attach/detach. ehci_init() > and ehci_detach() are not symmetrical, the former initializes more stuff > than the latter deinitializes. ehci_pci.c deinitializes the extra stuff > correctly, but the others (cardbus, arm, mips) don't. yup. it's kind of a wreck, but we're slowly making all of this better. for a long time many of the host controllers didn't need to support detach, but we've been moving more and more towards everything is capable of detaching the last few years, and sometimes things crash annoyingly when you are trying to reboot. thanks. .mrg.
Re: ehci: fix error handling?
Le 25/05/2019 à 08:53, Iain Hibbert a écrit : On Sat, 25 May 2019, Maxime Villard wrote: To fix this problem, we need to fix the error handling in ehci_pci_attach(), so that goto fail does not leave 'sc' in an inconsistent state. What is the best way to do that? Should I somehow forcibly unregister the device, or should I add a flag in 'sc' to indicate whether it is initialized? the latter It looks like there is no clean way to handle attach failures. attach always 'succeeds' in the sense that after attach has been called, detach will always be called. The detach routine should tear down everything that needs tearing down and not do things that will fail. Perhaps the init could simply be done before the attach routine gets the chance to fail? iain Ok, thanks, I've fixed it and the machine shuts down correctly now. However there seem to be other problems in the attach/detach. ehci_init() and ehci_detach() are not symmetrical, the former initializes more stuff than the latter deinitializes. ehci_pci.c deinitializes the extra stuff correctly, but the others (cardbus, arm, mips) don't.
Re: ehci: fix error handling?
Hi all, > attach always 'succeeds' in the sense that after attach has been called, > detach will always be called. The detach routine should tear down > everything that needs tearing down and not do things that will fail. > Perhaps the init could simply be done before the attach routine gets the > chance to fail? Some drivers mark the progress through attach so that detach will work correctly in all cases. Can we do the same here? For example, see sc->sc_att_stage in: https://nxr.netbsd.org/xref/src/sys/dev/ic/gem.c and also how gem_partial_detach() is called when the attach fails part way through. Regards, Julian
Re: ehci: fix error handling?
On Sat, 25 May 2019, Maxime Villard wrote: > To fix this problem, we need to fix the error handling in ehci_pci_attach(), > so that goto fail does not leave 'sc' in an inconsistent state. > > What is the best way to do that? Should I somehow forcibly unregister the > device, or should I add a flag in 'sc' to indicate whether it is initialized? the latter > It looks like there is no clean way to handle attach failures. attach always 'succeeds' in the sense that after attach has been called, detach will always be called. The detach routine should tear down everything that needs tearing down and not do things that will fail. Perhaps the init could simply be done before the attach routine gets the chance to fail? iain
