Messages in this thread | | | Date | Tue, 18 Aug 2009 17:29:28 +0400 | From | Anton Vorontsov <> | Subject | Re: [PATCH] pcf50633: use a dedicated workqueue for irq processing |
| |
On Fri, Jul 31, 2009 at 01:23:13PM +0400, Paul Fertser wrote: > On Thu, Jul 30, 2009 at 03:34:25PM -0700, Andrew Morton wrote: > > On Tue, 28 Jul 2009 00:41:15 +0400 > > Paul Fertser <fercerpav@gmail.com> wrote: > > > > > Using the default kernel "events" workqueue causes problems with > > > synchronous adc readings if initiated from some task on the same > > > workqueue. > > > > > > I had a deadlock trying to use pcf50633_adc_sync_read from a > > > power_supply class driver because the reading was initiated from the > > > workqueue and it waited for the irq processing to complete (to get the > > > result) and that was put on the same workqueue. > > > > I don't get it. > > > > Do you meant that pcf50633_adc_sync_read() was called via a > > schedule_work() handler? If so, wasn't that a bug? > > Andrew, i think it is called this way: > > power_supply_changed -> schedule_work(&psy->changed_work) -> > power_supply_changed_work -> kobject_uevent(&psy->dev->kobj, KOBJ_CHANGE) > -> power_supply_uevent -> power_supply_show_property -> get_property -> > bat->pdata->get_voltage -> pcf50633_adc_sync_read > > I assumed that since power_supply class is well-established and widely used > that's an intended behaviour. CCing relevant maintainers to get their > opinions.
The thing is that power_supply_changed() is supposed to be callable from any context (including hard IRQs), so we use schedule_work().
I see the deadlock problem, and it could be fixed by a dedicated workqueue either in the pcf50633 driver, or power supply class. I don't mind either way.
Another option is to implement power_supply_changed_can_sleep(), which won't use schedule_work(), and so we won't need another thread.
Thanks,
-- Anton Vorontsov email: cbouatmailru@gmail.com irc://irc.freenode.net/bd2
| |