Re: [PATCH] firewire-sbp2: Initialise sbp2_orb->rcode for management ORBs

[Stefan Richter <stefanr@s5r6.in-berlin.de>]

On Mar 04 Stefan Richter wrote:
> On Feb 18 Chris Boot wrote:
> > When sending ORBs the struct sbp2_orb->rcode field should be initialised
> > to -1 otherwise complete_transaction() assumes the request is successful
> > (RCODE_COMPLETE is 0). When sending managament ORBs, such as LOGIN or
> > LOGOUT, this was not done and so the initiator would wait for the
> > request to time out before trying again.
> > 
> > Without this, LOGINs are only retried when the management ORB times out,
> > rather than the initiator noticing an error occurred and retrying soon
> > after. For targets that advertise more than one LUN per unit, and can
> > only accept one management request at a time, this means LUNs are only
> > logged in one per timeout period.
[...]
> I left this hanging in my inbox for too long, sorry...
> 
> While I agree that the current initialization of orb->base.rcode with 0 is
> wrong, I don't think your change alone is sufficient:
> 
> Consider the case that a login request to LU 0 causes the target to pull
> out the hardware behind that LU out of a powered-down state --- which may
> take a very long time --- and login requests to LU 1 would be aborted by
> the target with resp_conflict_error on any Management_Agent write
> request.  Of course a reasonably clever target would accept login before
> full power-up, but you never now.
> 
> We retry login 5 times in 0.2 seconds intervals, and this 1 s in total may
> not be enough.
[...]

Chris, I obviously haven't done anything about this potentially too short
retry period yet; it is still on my list.

Perhaps we should not count the number of retries but watch the time that
retries take.  I.e. accumulate the time that each try takes; break out of
the retry loop after a maximum time; but reset the accumulated time at a
bus reset as a precaution for buses with many nodes coming online at
different times.
-- 
Stefan Richter
-=====-===-- -=-= =--==
http://arcgraph.de/sr/