Re: [PATCH] hw/nvme: Use ioeventfd to handle doorbell updates

[Keith Busch]

On Wed, Jun 29, 2022 at 05:04:25PM +0800, Jinhao Fan wrote:
> Ping~
> 
> > @@ -4271,6 +4343,11 @@ static void nvme_init_sq(NvmeSQueue *sq, NvmeCtrl 
> > *n, uint64_t dma_addr,
> >     if (n->dbbuf_enabled) {
> >         sq->db_addr = n->dbbuf_dbs + (sqid << 3);
> >         sq->ei_addr = n->dbbuf_eis + (sqid << 3);
> > +            
> > +        if (n->params.ioeventfd && sq->sqid != 0) {
> > +            ret = nvme_init_sq_ioeventfd(sq);
> > +            sq->ioeventfd_enabled = ret == 0;
> > +        }
> >     }
> > 
> >     assert(n->cq[cqid]);
> 
> Is this “ret == 0” a correct way for error handling?

That looks correct since we don't need the ioevent is an optional optimization.

I would just suggest making this easier to read. For example, in
nvme_init_sq_ioeventfd(), instead of assigning within a conditional:

    if ((ret = event_notifier_init(&cq->notifier, 0))) {

Do each part separately:

    ret = event_notifier_init(&cq->notifier, 0);
    if (ret) {
 
> I’ve also been wondering whether using irqfd for sending interrupts can
> bring some benefits. I’m not familiar with how QEMU emulates interrupts.
> What do you think of irqfd’s?

Not sure about this mechanism, I'll need to look into it.