| Message ID | 20190703155944.9637-3-mlevitsk@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Few fixes for userspace NVME driver | expand |
On 03.07.19 17:59, Maxim Levitsky wrote: > Fix the math involving non standard doorbell stride > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > block/nvme.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block/nvme.c b/block/nvme.c > index 6d4e7f3d83..52798081b2 100644 > --- a/block/nvme.c > +++ b/block/nvme.c > @@ -217,7 +217,7 @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs, > error_propagate(errp, local_err); > goto fail; > } > - q->cq.doorbell = &s->regs->doorbells[idx * 2 * s->doorbell_scale + 1]; > + q->cq.doorbell = &s->regs->doorbells[(idx * 2 + 1) * s->doorbell_scale]; > > return q; > fail: Hm. How has this ever worked? Reviewed-by: Max Reitz <mreitz@redhat.com>
On 05.07.19 13:09, Max Reitz wrote: > On 03.07.19 17:59, Maxim Levitsky wrote: >> Fix the math involving non standard doorbell stride >> >> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> >> --- >> block/nvme.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/block/nvme.c b/block/nvme.c >> index 6d4e7f3d83..52798081b2 100644 >> --- a/block/nvme.c >> +++ b/block/nvme.c >> @@ -217,7 +217,7 @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs, >> error_propagate(errp, local_err); >> goto fail; >> } >> - q->cq.doorbell = &s->regs->doorbells[idx * 2 * s->doorbell_scale + 1]; >> + q->cq.doorbell = &s->regs->doorbells[(idx * 2 + 1) * s->doorbell_scale]; >> >> return q; >> fail: > > Hm. How has this ever worked? (Ah, because CAP.DSTRD has probably been 0 in most devices.)
On Fri, 2019-07-05 at 13:10 +0200, Max Reitz wrote: > On 05.07.19 13:09, Max Reitz wrote: > > On 03.07.19 17:59, Maxim Levitsky wrote: > > > Fix the math involving non standard doorbell stride > > > > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > > --- > > > block/nvme.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/block/nvme.c b/block/nvme.c > > > index 6d4e7f3d83..52798081b2 100644 > > > --- a/block/nvme.c > > > +++ b/block/nvme.c > > > @@ -217,7 +217,7 @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs, > > > error_propagate(errp, local_err); > > > goto fail; > > > } > > > - q->cq.doorbell = &s->regs->doorbells[idx * 2 * s->doorbell_scale + 1]; > > > + q->cq.doorbell = &s->regs->doorbells[(idx * 2 + 1) * s->doorbell_scale]; > > > > > > return q; > > > fail: > > > > Hm. How has this ever worked? > > (Ah, because CAP.DSTRD has probably been 0 in most devices.) > Exactly, and I used cache line stride in my nvme-mdev, which broke this and I spend an evening figuring out what is going on. I was sure that there is some memory ordering bug or something even weirder before (as usual) finding that this is a very simple bug. I tested nvme-mdev pretty much with everything I could get my hands on, including this driver. Best regards, Maxim Levitsky
diff --git a/block/nvme.c b/block/nvme.c index 6d4e7f3d83..52798081b2 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -217,7 +217,7 @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs, error_propagate(errp, local_err); goto fail; } - q->cq.doorbell = &s->regs->doorbells[idx * 2 * s->doorbell_scale + 1]; + q->cq.doorbell = &s->regs->doorbells[(idx * 2 + 1) * s->doorbell_scale]; return q; fail:
Fix the math involving non standard doorbell stride Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- block/nvme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)