From 5095d47320c36e2346d41eb7e35dcbd215886557 Mon Sep 17 00:00:00 2001 From: Kimplul Date: Mon, 3 Nov 2025 20:17:32 +0200 Subject: minimize critical section in work handler + Effects handlers were being called in a locked context, which in this case happened to work due to how `hid_hw_request` is implemented. Now, only the selected state is updated and the heavy lifting of making the wheel aware of the requested changes is done after the critical section. --- src/hid-tmff2.c | 78 +++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 27 deletions(-) (limited to 'src/hid-tmff2.c') diff --git a/src/hid-tmff2.c b/src/hid-tmff2.c index b6bb834..ee3188a 100644 --- a/src/hid-tmff2.c +++ b/src/hid-tmff2.c @@ -302,15 +302,22 @@ static void tmff2_work_handler(struct work_struct *w) return; for (effect_id = 0; effect_id < tmff2->max_effects; ++effect_id) { - spin_lock_irqsave(&tmff2->lock, lock_flags); + unsigned long actions = 0; + struct tmff2_effect_state effect; time_now = JIFFIES2MS(jiffies); + state = &tmff2->states[effect_id]; + /* critical section for updating state flags, keep log of what + * actions to take after the critical section with actions */ + spin_lock_irqsave(&tmff2->lock, lock_flags); + effect_delay = state->effect.replay.delay; effect_length = state->effect.replay.length; if (test_bit(FF_EFFECT_PLAYING, &state->flags) && effect_length) { - if ((time_now - state->start_time) >= (effect_delay + effect_length) * state->count) { + if ((time_now - state->start_time) >= + (effect_delay + effect_length) * state->count) { __clear_bit(FF_EFFECT_PLAYING, &state->flags); __clear_bit(FF_EFFECT_QUEUE_UPDATE, &state->flags); @@ -319,47 +326,64 @@ static void tmff2_work_handler(struct work_struct *w) } if (test_bit(FF_EFFECT_QUEUE_UPLOAD, &state->flags)) { - if (tmff2->upload_effect(tmff2->data, state)) { - hid_warn(tmff2->hdev, "failed uploading effect\n"); - } else { - __clear_bit(FF_EFFECT_QUEUE_UPLOAD, &state->flags); - /* if we're uploading an effect, it's bound to be the up - * to date */ - __clear_bit(FF_EFFECT_QUEUE_UPDATE, &state->flags); - } + __set_bit(FF_EFFECT_QUEUE_UPLOAD, &actions); + __clear_bit(FF_EFFECT_QUEUE_UPLOAD, &state->flags); + /* if we're uploading an effect, it's bound to be up + * to date */ + __clear_bit(FF_EFFECT_QUEUE_UPDATE, &state->flags); } if (test_bit(FF_EFFECT_QUEUE_UPDATE, &state->flags)) { - if (tmff2->update_effect(tmff2->data, state)) - hid_warn(tmff2->hdev, "failed updating effect\n"); - else - __clear_bit(FF_EFFECT_QUEUE_UPDATE, &state->flags); + __set_bit(FF_EFFECT_QUEUE_UPDATE, &actions); + __clear_bit(FF_EFFECT_QUEUE_UPDATE, &state->flags); } if (test_bit(FF_EFFECT_QUEUE_START, &state->flags)) { - if (tmff2->play_effect(tmff2->data, state)) { - hid_warn(tmff2->hdev, "failed starting effect\n"); - } else { - __clear_bit(FF_EFFECT_QUEUE_START, &state->flags); - __set_bit(FF_EFFECT_PLAYING, &state->flags); - } - + __set_bit(FF_EFFECT_QUEUE_START, &actions); + __clear_bit(FF_EFFECT_QUEUE_START, &state->flags); + /* effect is playing since we're started it right now */ + __set_bit(FF_EFFECT_PLAYING, &state->flags); } if (test_bit(FF_EFFECT_QUEUE_STOP, &state->flags)) { - if (tmff2->stop_effect(tmff2->data, state)) { - hid_warn(tmff2->hdev, "failed stopping effect\n"); - } else { - __clear_bit(FF_EFFECT_PLAYING, &state->flags); - __clear_bit(FF_EFFECT_QUEUE_STOP, &state->flags); - } + __set_bit(FF_EFFECT_QUEUE_STOP, &actions); + __clear_bit(FF_EFFECT_QUEUE_STOP, &state->flags); + + /* the effect can't be playing if we're stopped, aye? */ + __clear_bit(FF_EFFECT_PLAYING, &state->flags); } if (state->count > max_count) max_count = state->count; + /* copy effect state to local variable so we can pass it around + * after the atomic section */ + effect = *state; + spin_unlock_irqrestore(&tmff2->lock, lock_flags); + /* perform the identified actions */ + if (test_bit(FF_EFFECT_QUEUE_UPLOAD, &actions) + && tmff2->upload_effect(tmff2->data, &effect)) { + hid_warn(tmff2->hdev, "failed uploading effect\n"); + } + + if (test_bit(FF_EFFECT_QUEUE_UPDATE, &actions) + && tmff2->update_effect(tmff2->data, &effect)) { + hid_warn(tmff2->hdev, "failed updating effect\n"); + } + + if (test_bit(FF_EFFECT_QUEUE_START, &actions) + && tmff2->play_effect(tmff2->data, &effect)) { + hid_warn(tmff2->hdev, "failed starting effect\n"); + } + + if (test_bit(FF_EFFECT_QUEUE_STOP, &actions) + && tmff2->stop_effect(tmff2->data, &effect)) { + hid_warn(tmff2->hdev, "failed stopping effect\n"); + } + + /* wait for each effect update to actually be sent out to avoid * filling up usb output queue */ hid_hw_wait(tmff2->hdev); -- cgit v1.3