On Thu, 10 Mar 2022 10:35:25 +0100,
S.J. Wang wrote:
>
> Thanks.
>
> >
> > The xrun/suspend may happen at any time and we should check it right
> > before the slave hwptr update. Otherwise the hwptr value may be screwed
>
> I think should be "after the slave hwptr update". If hwptr is screwed, means
> Suspend happened then check_xrun() can return.
>
> > and get unexpected large read/write.
> >
> > Reported-by: S.J. Wang <shengjiu.wang@xxxxxxx>
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > ---
> > src/pcm/pcm_dmix.c | 4 ++--
> > src/pcm/pcm_dshare.c | 4 ++--
> > src/pcm/pcm_dsnoop.c | 6 +++---
> > 3 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index
> > d00d53bef604..111fea157228 100644
> > --- a/src/pcm/pcm_dmix.c
> > +++ b/src/pcm/pcm_dmix.c
> > @@ -426,11 +426,11 @@ static int snd_pcm_dmix_sync_ptr(snd_pcm_t
> > *pcm)
> > snd_pcm_direct_t *dmix = pcm->private_data;
> > int err;
> >
> > + if (dmix->slowptr)
> > + snd_pcm_hwsync(dmix->spcm);
> > err = snd_pcm_direct_check_xrun(dmix, pcm);
> > if (err < 0)
> > return err;
> > - if (dmix->slowptr)
> > - snd_pcm_hwsync(dmix->spcm);
> >
> > return snd_pcm_dmix_sync_ptr0(pcm, *dmix->spcm->hw.ptr); } diff --
>
> Better to get slave_hw_ptr before check_xrun(), like this:
>
> --- a/src/pcm/pcm_dmix.c
> +++ b/src/pcm/pcm_dmix.c
> @@ -424,15 +424,17 @@ static int snd_pcm_dmix_sync_ptr0(snd_pcm_t *pcm, snd_pcm_uframes_t slave_hw_ptr
> static int snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm)
> {
> snd_pcm_direct_t *dmix = pcm->private_data;
> + snd_pcm_uframes_t slave_hw_ptr;
> int err;
>
> if (dmix->slowptr)
> snd_pcm_hwsync(dmix->spcm);
> + slave_hw_ptr = *dmix->spcm->hw.ptr;
> err = snd_pcm_direct_check_xrun(dmix, pcm);
> if (err < 0)
> return err;
>
> - return snd_pcm_dmix_sync_ptr0(pcm, *dmix->spcm->hw.ptr);
> + return snd_pcm_dmix_sync_ptr0(pcm, slave_hw_ptr);
> }
Makes sense. I'll respin v2 with those changes.
thanks,
Takashi
