Re: usbtmc: stb race condition introduced by 4f3c8d6eddc272b386464524235440a418ed2029

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

On Wed, Sep 2, 2020 at 10:25 AM <guido@kiener-muenchen.de> wrote:
>
> >> 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.
>
> This is not a problem. You can check the status byte several times
> as long as you like. When RQS Bit (0x40) is set, then you know it was
> an intermediate SRQ, sent according to chapter 3.4.1
> (USBTMC_usb488_subclass_1_00.pdf). Otherwise it is a requested status
> byte according to chapter 3.4.2.

I wouldn't call myself a USBTMC or 488 expert. I read the USBTMC spec,
488.1 and 488.2 but it's been three years since I implemented most of
my device firmware and wrote the userspace code. It runs in a
contained environment so I haven't done any interoperability testing.
However, my understanding is that MSS shares the same bit with RQS and
if something is still needing service the MSS bit will be set in the
status byte given in reply to a READ_STATUS_BYTE request. If that is
the case how would you tell a status byte sent as an SRQ vs one with
MSS sent in reply to READ_STATUS_BYTE? See IEEE Std 488.2-1992 Figure
11-3—Service Request Enabling.

>
> > 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.
>
> Yes, interrupts (here SRQ) can happen at any time. Therefore the user
> can enable/disable interrupts (e.g. with SCPI command SRE) and
> postpone interrupt handling.

This wasn't necessary before but perhaps it could be used to work
around the issue.

>
> The SRQ sends a status byte with RQS bit set (chapter 3.4.1) and the
> request initiated by the client returns a status byte without RQS bit
> (chapter 3.4.2).

Again maybe I'm confused by how MSS is supposed to work.

>
> > 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.
>
> I agree that this can result in an odd behavior (E.g. the MAV bit
> is unset with reading the subsequent cached status byte).
> I was not aware about this problem and I need to check whether this is
> a common problem in VISA implementations or whether we just need to fix
> the kernel (e.g. drop requested status byte and return older cached value).
>
> A workaround to avoid this odd behavior is to read the status byte
> again with USBTMC488_IOCTL_READ_STB when your user application detects
> the RQS bit.
>
> BTW when you look at the old implementation
> https://elixir.bootlin.com/linux/v4.7.10/source/drivers/usb/class/usbtmc.c#L1332
> then you can see that you will never get a status byte with RQS bit set.
> The status byte was never stored in data->bNotify2
>
> Regards,
>
> Guido
>
>
>