Re: [PATCH] mctp i2c: check packet length before marking flow active

[Jeremy Kerr <jk@codeconstruct.com.au>]

Hi William,

> > Just to clarify my understanding of the state: "being held by two
> > owners" would indicate a violation of the lock itself. Or is it that
> > there are two threads blocked waiting to acquire the mutex?
> I think it’s actually this, 2 threads are waiting on acquiring the lock. 

OK, that's good news!

> There was a theory that it was a lock underflow that allowed 2 threads 
> to acquire the lock that lead to this patch.
>
> > For NVMe-MI, you're likely using manual tag allocation, where the tag
> > allocation (and hence flow state) is entirely controlled by userspace.
> > It may be that the NVMe protocol-level errors are causing that tags to
> > be held for long durations, perhaps?
> 
> Yeah, this is very plausible given the device(s) stop responding 
> correctly. I imagine we are getting stuck with manual allocations and
> not releasing locks. Can we reset the state machine back to NEW instead 
> of holding the lock?

Not sure what you're referring to here; if the userspace application is
not releasing the tag, we have to keep the i2c bus locked, otherwise we
may not receive a response from the device.

The one case I can think of (in upstream infrastructure, at least) is
that this might be triggered by the device reporting a long MPRT value,
and then a response gets lost. libnvme is respecting the MPRT, and not
releasing the tag for that (excessive) duration.

However, the tag -> i2c lock associations are only useful if you have
muxes in the i2c topology. Is that the case on your platform? If not,
perhaps we could elide all the bus locking when we can detect that...

Cheers,


Jeremy