Re: [PATCH 08/17] hw/block/nvme: refactor aio submission

[Klaus Jensen]

On Sep  4 14:15, Keith Busch wrote:
> On Fri, Sep 04, 2020 at 10:38:39PM +0200, Klaus Jensen wrote:
> > On Sep  4 12:47, Keith Busch wrote:
> > > On Fri, Sep 04, 2020 at 04:19:47PM +0200, Klaus Jensen wrote:
> > > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > > > index bfac3385cb64..3e32f39c7c1d 100644
> > > > --- a/hw/block/nvme.c
> > > > +++ b/hw/block/nvme.c
> > > > @@ -110,6 +110,7 @@ static const uint32_t 
> > > > nvme_feature_cap[NVME_FID_MAX] = {
> > > >  };
> > > >  
> > > >  static void nvme_process_sq(void *opaque);
> > > > +static void nvme_aio_cb(void *opaque, int ret);
> > > 
> > > You don't need the forward declaration here. Just move the
> > > implementation above where it's used. It looks safe: nvme_aio_cb()
> > > doesn't have any circular dependencies.
> > 
> > You are right, of course. But it is getting a circular dependency in a
> > later patch. I left it there to reduce code movement later.
> 
> Is that coming in a future patch? Not finding it in this series.
> 
> About the whole patch in general, are multiple aio's per-request coming
> in later patch as well? I didn't see any use for it here, and the
> overhead of dynamic allocation and zeroing a new struct in the IO path
> is a bit concerning for performance. I'd like to see your intended use
> for this.

Intended use-case was parallel aios. There are a lot of use cases for
this, DSM, metadata, block allocation tracking and zns zoneinfo.

But I'll rip it out of the series and repost so we can focus on multiple
namespaces.