OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: Re: [PATCH v6 07/17] firmware: arm_scmi: Handle concurrent and out-of-order messages


On 19.07.21 11:14, Cristian Marussi wrote:
On Thu, Jul 15, 2021 at 06:36:03PM +0200, Peter Hilber wrote:
On 12.07.21 16:18, Cristian Marussi wrote:

[snip]

@@ -608,6 +755,7 @@ static int do_xfer(const struct scmi_protocol_handle *ph,
   			      xfer->hdr.protocol_id, xfer->hdr.seq,
   			      xfer->hdr.poll_completion);
+	xfer->state = SCMI_XFER_SENT_OK;

To be completely safe, this assignment could also be protected by the
xfer->lock.


In fact this would be true being xfer->lock meant to protect the state but it
seemed to me unnecessary here given that this is a brand new xfer with a
brand new (monotonic) seq number so that any possibly late-received msg will
carry an old stale seq number certainly different from this such that cannot be
possibly mapped to this same xfer. (but just discarded on xfer lookup in
xfer_command_acquire)

The issue indeed could still exist only for do_xfer loops (as you pointed out
already early on) where the seq_num is used, but in that case on a timeout we
would have already bailed out of the loop and reported an error so any timed-out
late received response would have been anyway discarded; so at the end I thought
I could avoid spinlocking here.

Thanks,
Cristian


I mostly meant to refer to the possibility of a very fast response not seeing this assignment, since the next line is

 	ret = info->desc->ops->send_message(cinfo, xfer);

and during that a regular scmi_rx_callback(), reading xfer->state, can already arrive. But maybe this is too theoretical.

Best regards,

Peter


[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]