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.
