Re: [PATCH 3/3] ASoC: max98090: fix possible race conditions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 21, 2019 at 11:20 AM Pierre-Louis Bossart
<pierre-louis.bossart@xxxxxxxxxxxxxxx> wrote:
> Still you may want to clarify your second point and the commit message.
> The first race condition was clear, the second one not so much.
>
> what did you mean with 'assert ULK == 0' and what does this map to in
> pll_work

The case (2) race condition is based on the previous patch:
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-November/158852.html

After applying the patch, the code would be something like: (in the
case, two threads read 0x01.)
static void max98090_pll_work(struct work_struct *work)
{
[snip]
        snd_soc_component_update_bits(component, M98090_REG_DEVICE_SHUTDOWN,
                            M98090_SHDNN_MASK, 0);
        snd_soc_component_update_bits(component, M98090_REG_DEVICE_SHUTDOWN,
                            M98090_SHDNN_MASK, M98090_SHDNN_MASK);

        for (i = 0; i < 10; ++i) {
                /* Check lock status */
                pll = snd_soc_component_read32(
                                component, M98090_REG_DEVICE_STATUS);
                if (!(pll & M98090_ULK_MASK))
                        break;

                /* Give PLL time to lock */
                usleep_range(1000, 1200);
        }
}

I guess we have 2 choices:
i. stick to the original plan: option B, remove the second thread of execution.
In the case, we would be punished by at most 10~12ms delay in the
interrupt handler thread.  I browsed the max98090_interrupt() and tend
to think it should be fine if we delay jack detection for extra 10ms.

ii. adopt option A, play with the masks; and drop the previous patch
to avoid multiple threads access 0x01
We would be forced to wait >10ms before re-enabling ULK interrupt.
Notably, Documentation/timers/timers-howto.txt states "msleep(1~20)
may not do what the caller intends, and will often sleep
longer".Documentation/timers/timers-howto.txt".

Either way should be fine.  Which one would you suggest?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux