From: Hans de Goede hdegoede@redhat.com
commit 5c34c0aef185dcd10881847b9ebf20046aa77cb4 upstream.
bq27xxx_battery_update() assumes / requires that it is only run once, not multiple times at the same time. But there are 3 possible callers:
1. bq27xxx_battery_poll() delayed_work item handler 2. bq27xxx_battery_irq_handler_thread() I2C IRQ handler 3. bq27xxx_battery_setup()
And there is no protection against these racing with each other, fix this race condition by making all callers take di->lock:
- Rename bq27xxx_battery_update() to bq27xxx_battery_update_unlocked()
- Add new bq27xxx_battery_update() which takes di->lock and then calls bq27xxx_battery_update_unlocked()
- Make stale cache check code in bq27xxx_battery_get_property(), which already takes di->lock directly to check the jiffies, call bq27xxx_battery_update_unlocked() instead of messing with the delayed_work item
- Make bq27xxx_battery_update_unlocked() mod the delayed-work item so that the next poll is delayed to poll_interval milliseconds after the last update independent of the source of the update
Fixes: 740b755a3b34 ("bq27x00: Poll battery state") Signed-off-by: Hans de Goede hdegoede@redhat.com Signed-off-by: Sebastian Reichel sebastian.reichel@collabora.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- drivers/power/supply/bq27xxx_battery.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
--- a/drivers/power/supply/bq27xxx_battery.c +++ b/drivers/power/supply/bq27xxx_battery.c @@ -1681,7 +1681,7 @@ static int bq27xxx_battery_read_health(s return POWER_SUPPLY_HEALTH_GOOD; }
-void bq27xxx_battery_update(struct bq27xxx_device_info *di) +static void bq27xxx_battery_update_unlocked(struct bq27xxx_device_info *di) { struct bq27xxx_reg_cache cache = {0, }; bool has_ci_flag = di->opts & BQ27XXX_O_HAS_CI; @@ -1732,6 +1732,16 @@ void bq27xxx_battery_update(struct bq27x di->cache = cache;
di->last_update = jiffies; + + if (poll_interval > 0) + mod_delayed_work(system_wq, &di->work, poll_interval * HZ); +} + +void bq27xxx_battery_update(struct bq27xxx_device_info *di) +{ + mutex_lock(&di->lock); + bq27xxx_battery_update_unlocked(di); + mutex_unlock(&di->lock); } EXPORT_SYMBOL_GPL(bq27xxx_battery_update);
@@ -1742,9 +1752,6 @@ static void bq27xxx_battery_poll(struct work.work);
bq27xxx_battery_update(di); - - if (poll_interval > 0) - schedule_delayed_work(&di->work, poll_interval * HZ); }
/* @@ -1919,10 +1926,8 @@ static int bq27xxx_battery_get_property( struct bq27xxx_device_info *di = power_supply_get_drvdata(psy);
mutex_lock(&di->lock); - if (time_is_before_jiffies(di->last_update + 5 * HZ)) { - cancel_delayed_work_sync(&di->work); - bq27xxx_battery_poll(&di->work.work); - } + if (time_is_before_jiffies(di->last_update + 5 * HZ)) + bq27xxx_battery_update_unlocked(di); mutex_unlock(&di->lock);
if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.flags < 0)