> From: Michael S. Tsirkin <mst@xxxxxxxxxx> > Sent: Tuesday, June 25, 2024 12:51 PM > > On Tue, Jun 25, 2024 at 10:11:26AM +0800, Heng Qi wrote: > > On Mon, 24 Jun 2024 11:16:45 -0400, "Michael S. Tsirkin" > <mst@xxxxxxxxxx> wrote: > > > On Mon, Jun 24, 2024 at 04:51:37PM +0200, Jiri Pirko wrote: > > > > Mon, Jun 24, 2024 at 03:55:53PM CEST, mst@xxxxxxxxxx wrote: > > > > >On Mon, Jun 24, 2024 at 03:46:19PM +0200, Jiri Pirko wrote: > > > > >> Mon, Jun 24, 2024 at 01:23:01PM CEST, mst@xxxxxxxxxx wrote: > > > > >> >On Mon, Jun 24, 2024 at 05:53:52PM +0800, Heng Qi wrote: > > > > >> >> On Mon, 24 Jun 2024 11:04:43 +0200, Jiri Pirko <jiri@xxxxxxxxxxx> > wrote: > > > > >> >> > From: Jiri Pirko <jiri@xxxxxxxxxx> > > > > >> >> > > > > > >> >> > Currently the admin queue command execution is serialized by a > lock. > > > > >> >> > This patchsets lifts this limitation allowing to execute > > > > >> >> > admin queue commands in parallel. To do that, admin queue > > > > >> >> > processing needs to be converted from polling to interrupt based > completion. > > > > >> >> > > > > > >> >> > Patches #1-#6 are preparations, making things a bit smoother as > well. > > > > >> >> > Patch #7 implements interrupt based completion for admin > queue. > > > > >> >> > > > > >> >> Hi, Jiri > > > > >> >> > > > > >> >> Before this set, I pushed the cvq irq set [1], and the > > > > >> >> discussion focused on the fact that the newly added irq > > > > >> >> vector may cause the IO queue to fall back to shared interrupt > mode. > > > > >> >> But it is true that devices implemented according to the > > > > >> >> specification should not encounter this problem. So what do you > think? > > > > >> > > > > >> Wait. Please note that admin queue is only created and used by > > > > >> PF virtio device. And most probably, this is on hypervisor > > > > >> managing the VFs that are passed to guest VMs. These VFs does not > have admin queue. > > > > >> > > > > >> Therefore, this is hardly comparable to control vq. > > > > > > > > > > > > > > >Well Parav recently posted patches adding admin queue to VFs, > > > > >with new "self" group type. > > > > > > > > Right, but even so, when device implementation decides to > > > > implement and enable admin queue, it should also make sure to > > > > provide correct amount of vectors. My point is, there should not > > > > be any breakage in user expectation, or am I missing something? > > > > > > > > > Hmm, I think you are right that cvq is an existing capability and > > > adminq is newer. > > > > admin vq has been supported in the kernel for more than half a year, > > and if at this point you think that the device must provide interrupt > > vectors for it, then I think this is also true for cvq. > > There's a big difference between 15 years and half a year. I don't think any > hardware devices have been released with the capability just yet, and yes > allocating a vector for each queue is a good idea for hardware devices. We have released the hw with admin capability set. And device not supporting the irq for AQ is just inefficient device. So it should be treated as device bug to be fixed by device fw upgrade per say. I agree with Michael's point.
