Re: usbtmc: stb race condition introduced by 4f3c8d6eddc272b386464524235440a418ed2029

[George McCollister <george.mccollister@gmail.com>]

On Tue, Sep 1, 2020 at 1:59 PM <guido@kiener-muenchen.de> wrote:
>
>
> George,
>
> Thanks for your question.
> > Since 4f3c8d6eddc272b386464524235440a418ed2029 I'm experiencing this
> > STB race condition. TL;DR an old, cached STB value can be used after a
> > new one is reported in reply to a control message. Hacking up the
> > latest driver code to ignore the cached stb value gets around the
> > issue.
>
> The SRQ notification is an important message and must not be missed
> in userspace applications. The new implementation does not miss the
> SRQ event anymore, even when multiple threads are waiting for the SRQ event.
>
> Your coding relies on the previous behavior and did not fail for your
> use case and timing. However we could not develop reliable applications with
> the previous implementation.
>
> There are now two ways to wait for the SRQ event:
> 1. Using the poll/select method
> 2. Using the new ioctl USBTMC488_IOCTL_WAIT_SRQ (preferred way)
>
> When receiving the SRQ event, you should immediately read the stb with
> USBTMC488_IOCTL_READ_STB within the same thread to prevent races or
> reading stale status byte information.

I do. I call it as soon as poll indicates there is an SRQ.

>
> More info see:
> https://github.com/GuidoKiener/linux-usbtmc/blob/master/README.md
>
> > My USBTMC device has an interrupt endpoint with a 1ms interval.
> >
> > 1) A query is sent to the USBTMC device.
> >
> > 2) An SRQ is reported via the interrupt endpoint with MAV set.
> >
> > 3) Userspace performs a read to get the reply since MAV is set after
> > being notified by poll().
>
> Did you start reading (read()) without checking the Status Byte after
> poll() event?

I check the status byte after poll(). The problem seems to be that I
also check the status byte in a loop (until there is nothing to
service) before calling poll again. As long as you only check the
status byte when there is a cached value available it should be no
problem. However if you call USBTMC488_IOCTL_READ_STB when there is
not a cached SRQ value an SRQ can occur after the lock is released in
usbtmc488_ioctl_read_stb() and cache an older value (which will be
read by the next USBTMC488_IOCTL_READ_STB) than the one returned. This
is a race condition and sounds broken to me but if this is the
intended behavior I can adjust my userspace code to only do
USBTMC488_IOCTL_READ_STB once after poll indicates an SRQ and live
with it.

It doesn't seem right for USBTMC488_IOCTL_READ_STB to ever report an
older value than what was reported on the previous call.

>
> Regards,
>
> Guido
>
>

Thanks for your reply.