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


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-comment message

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

Subject: Re: [virtio-comment] [PATCH v6] virtio-i2c: add the device specification

On 2020/12/31 20:42, Stefan Hajnoczi wrote:

This approach causes a bottleneck when there are multiple I2C devices.
It requires exclusive access to the virtqueue and virtual I2C bus,
slowing down applications that are accessing separate I2C devices.

This is inefficient, especially if the I2C devices are emulated or on
separate physical I2C busses where request concurrent processing is

This also creates starvation/denial-of-service problems in the driver
because an application can submit many requests and needs exclusive
access to the virtqueue until all requests have completed.

I suggest defining a VIRTIO_I2C_FLAGS_CONTINUATION bit in struct
virtio_i2c_out_hdr::flags. When a request fails and this flag is set
then the device fails all later requests until it reaches a request
where the flag is zero. This allows the driver to enqueue a sequence of
requests that fail as a group without waiting for an empty virtqueue.
The flags is designed to be a "driver->device" field for extensibility. It
is read-onlyï to the device.
So when a request fails, the device can't set this read-only field.
Sorry, "and this flag is set" means "and this flag was set by the
driver". My intention was that the driver marks a request so the device
knows that it needs to fail the next request too when this request
No. A device doesn't need to be told by a driver if it needs to fail the next request. Actually just the opposite, a driver may need to be notified. A driver can't know which one is the "next" because only a device know which request is the first failed one.

Once a device fails to handle a request, it shall fail the existing remaining requests
since these requests needs to be handled in order.
Thinking about it again, VIRTIO_I2C_FLAGS_FAIL_NEXT would be a clearer
name for this flag.

We just need to make sure that once the driver adds some requests to the
it should complete them (either success or fail) before adding new requests.
I think this
is a behavior of physical I2C adapter drivers and we can follow.
Existing VIRTIO devices do not have special empty queue semantics and I
don't think virtio-i2c should introduce a special case here. The
VIRTIO_I2C_FLAGS_FAIL_NEXT flag is simple to implement, allows drivers
more freedom to enqueue requests, and eliminates the race condition with
polled device implementations (they could complete failed request #1
before request #2 is enqueued and then request #2 would not be
automatically failed).
The I2C core is using a synchronization semantics. It means once a I2C bus driver send some messages, it should wait for the device to complete them (either success or fail) before sending new messages. The virtio-i2c driver also have to follow this to register it into I2C subsystem. So the freedom is already limited by the I2C core, and as a result, we don't have
such race conditions.


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