From: Christoph Hellwig <hch@lst.de>
To: Keith Busch <kbusch@kernel.org>
Cc: sagi@grimberg.me, linux-kernel@vger.kernel.org,
linux-nvme@lists.infradead.org, axboe@fb.com,
Baolin Wang <baolin.wang@linux.alibaba.com>,
baolin.wang7@gmail.com, hch@lst.de
Subject: Re: [PATCH 5/5] nvme-pci: Use standard block status macro
Date: Wed, 8 Jul 2020 08:05:05 +0200 [thread overview]
Message-ID: <20200708060505.GA4919@lst.de> (raw)
In-Reply-To: <20200707190123.GB1997220@dhcp-10-100-145-180.wdl.wdc.com>
On Tue, Jul 07, 2020 at 12:01:23PM -0700, Keith Busch wrote:
> On Fri, Jul 03, 2020 at 10:49:24AM +0800, Baolin Wang wrote:
> > static blk_status_t nvme_map_data(struct nvme_dev *dev, struct request *req,
> > @@ -844,7 +844,7 @@ static blk_status_t nvme_map_metadata(struct nvme_dev *dev, struct request *req,
> > if (dma_mapping_error(dev->dev, iod->meta_dma))
> > return BLK_STS_IOERR;
> > cmnd->rw.metadata = cpu_to_le64(iod->meta_dma);
> > - return 0;
> > + return BLK_STS_OK;
> > }
>
> This is fine, though it takes knowing that this value is 0 for the
> subsequent 'if (!ret)' check to make sense. Maybe those should change to
> 'if (ret != BLK_STS_OK)' so the check uses the same symbol as the
> return, and will always work in the unlikely event that the defines
> are reordered.
If you think this version is inconsistent I'd rather drop this patch.
The assumption that 0 == BLK_STS_OK is inherent all over the code.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@lst.de>
To: Keith Busch <kbusch@kernel.org>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>,
axboe@fb.com, hch@lst.de, sagi@grimberg.me,
baolin.wang7@gmail.com, linux-nvme@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/5] nvme-pci: Use standard block status macro
Date: Wed, 8 Jul 2020 08:05:05 +0200 [thread overview]
Message-ID: <20200708060505.GA4919@lst.de> (raw)
In-Reply-To: <20200707190123.GB1997220@dhcp-10-100-145-180.wdl.wdc.com>
On Tue, Jul 07, 2020 at 12:01:23PM -0700, Keith Busch wrote:
> On Fri, Jul 03, 2020 at 10:49:24AM +0800, Baolin Wang wrote:
> > static blk_status_t nvme_map_data(struct nvme_dev *dev, struct request *req,
> > @@ -844,7 +844,7 @@ static blk_status_t nvme_map_metadata(struct nvme_dev *dev, struct request *req,
> > if (dma_mapping_error(dev->dev, iod->meta_dma))
> > return BLK_STS_IOERR;
> > cmnd->rw.metadata = cpu_to_le64(iod->meta_dma);
> > - return 0;
> > + return BLK_STS_OK;
> > }
>
> This is fine, though it takes knowing that this value is 0 for the
> subsequent 'if (!ret)' check to make sense. Maybe those should change to
> 'if (ret != BLK_STS_OK)' so the check uses the same symbol as the
> return, and will always work in the unlikely event that the defines
> are reordered.
If you think this version is inconsistent I'd rather drop this patch.
The assumption that 0 == BLK_STS_OK is inherent all over the code.
next prev parent reply other threads:[~2020-07-08 6:05 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-03 2:49 [PATCH 0/5] Some cleanups for NVMe-pci driver Baolin Wang
2020-07-03 2:49 ` Baolin Wang
2020-07-03 2:49 ` [PATCH 1/5] nvme-pci: Fix some comments issues Baolin Wang
2020-07-03 2:49 ` Baolin Wang
2020-07-05 6:57 ` Sagi Grimberg
2020-07-05 6:57 ` Sagi Grimberg
2020-07-06 2:10 ` Chaitanya Kulkarni
2020-07-06 2:10 ` Chaitanya Kulkarni
2020-07-03 2:49 ` [PATCH 2/5] nvme-pci: Add a blank line after declarations Baolin Wang
2020-07-03 2:49 ` Baolin Wang
2020-07-05 6:57 ` Sagi Grimberg
2020-07-05 6:57 ` Sagi Grimberg
2020-07-06 2:09 ` Chaitanya Kulkarni
2020-07-06 2:09 ` Chaitanya Kulkarni
2020-07-03 2:49 ` [PATCH 3/5] nvme-pci: Remove redundant segment validation Baolin Wang
2020-07-03 2:49 ` Baolin Wang
2020-07-06 2:23 ` Chaitanya Kulkarni
2020-07-06 2:23 ` Chaitanya Kulkarni
2020-07-03 2:49 ` [PATCH 4/5] nvme-pci: Use the consistent return type of nvme_pci_iod_alloc_size() Baolin Wang
2020-07-03 2:49 ` Baolin Wang
2020-07-05 6:59 ` Sagi Grimberg
2020-07-05 6:59 ` Sagi Grimberg
2020-07-06 2:24 ` Chaitanya Kulkarni
2020-07-06 2:24 ` Chaitanya Kulkarni
2020-07-03 2:49 ` [PATCH 5/5] nvme-pci: Use standard block status macro Baolin Wang
2020-07-03 2:49 ` Baolin Wang
2020-07-05 7:00 ` Sagi Grimberg
2020-07-05 7:00 ` Sagi Grimberg
2020-07-06 2:25 ` Chaitanya Kulkarni
2020-07-06 2:25 ` Chaitanya Kulkarni
2020-07-07 19:01 ` Keith Busch
2020-07-07 19:01 ` Keith Busch
2020-07-08 1:25 ` Baolin Wang
2020-07-08 1:25 ` Baolin Wang
2020-07-08 6:05 ` Christoph Hellwig [this message]
2020-07-08 6:05 ` Christoph Hellwig
2020-07-07 8:50 ` [PATCH 0/5] Some cleanups for NVMe-pci driver Christoph Hellwig
2020-07-07 8:50 ` Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200708060505.GA4919@lst.de \
--to=hch@lst.de \
--cc=axboe@fb.com \
--cc=baolin.wang7@gmail.com \
--cc=baolin.wang@linux.alibaba.com \
--cc=kbusch@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=sagi@grimberg.me \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.