* [PATCH 1/1] Replace the dma_alloc_coherent() with dma_zalloc_coherent()
@ 2014-07-15 18:11 Mundu
2014-07-15 18:28 ` Matthew Wilcox
0 siblings, 1 reply; 6+ messages in thread
From: Mundu @ 2014-07-15 18:11 UTC (permalink / raw)
Signed-off-by: Mundu <mundu2510 at gmail.com>
diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 28aec2d..63f3e19 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1295,7 +1295,7 @@ static struct nvme_queue *nvme_alloc_queue(struct nvme_dev *dev, int qid,
if (!nvmeq->cqes)
goto free_nvmeq;
- nvmeq->sq_cmds = dma_alloc_coherent(dmadev, SQ_SIZE(depth),
+ nvmeq->sq_cmds = dma_zalloc_coherent(dmadev, SQ_SIZE(depth),
&nvmeq->sq_dma_addr, GFP_KERNEL);
if (!nvmeq->sq_cmds)
goto free_cqdma;
@@ -1356,7 +1356,6 @@ static void nvme_init_queue(struct nvme_queue *nvmeq, u16 qid)
nvmeq->cq_phase = 1;
nvmeq->q_db = &dev->dbs[qid * 2 * dev->db_stride];
memset(nvmeq->cmdid_data, 0, extra);
- memset((void *)nvmeq->cqes, 0, CQ_SIZE(nvmeq->q_depth));
nvme_cancel_ios(nvmeq, false);
nvmeq->q_suspended = 0;
dev->online_queues++;
@@ -1652,7 +1651,7 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
goto unmap;
}
- meta_mem = dma_alloc_coherent(&dev->pci_dev->dev, meta_len,
+ meta_mem = dma_zalloc_coherent(&dev->pci_dev->dev, meta_len,
&meta_dma_addr, GFP_KERNEL);
if (!meta_mem) {
status = -ENOMEM;
@@ -2293,7 +2292,7 @@ static int nvme_dev_add(struct nvme_dev *dev)
dma_addr_t dma_addr;
int shift = NVME_CAP_MPSMIN(readq(&dev->bar->cap)) + 12;
- mem = dma_alloc_coherent(&pdev->dev, 8192, &dma_addr, GFP_KERNEL);
+ mem = dma_zalloc_coherent(&pdev->dev, 8192, &dma_addr, GFP_KERNEL);
if (!mem)
return -ENOMEM;
diff --git a/drivers/block/nvme-scsi.c b/drivers/block/nvme-scsi.c
index a4cd6d6..ce08fed 100644
--- a/drivers/block/nvme-scsi.c
+++ b/drivers/block/nvme-scsi.c
@@ -682,7 +682,7 @@ static int nvme_trans_standard_inquiry_page(struct nvme_ns *ns,
u8 cmdque = 0x01 << 1;
u8 fw_offset = sizeof(dev->firmware_rev);
- mem = dma_alloc_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns),
+ mem = dma_zalloc_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns),
&dma_addr, GFP_KERNEL);
if (mem == NULL) {
res = -ENOMEM;
@@ -786,7 +786,7 @@ static int nvme_trans_device_id_page(struct nvme_ns *ns, struct sg_io_hdr *hdr,
int xfer_len;
__be32 tmp_id = cpu_to_be32(ns->ns_id);
- mem = dma_alloc_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns),
+ mem = dma_zalloc_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns),
&dma_addr, GFP_KERNEL);
if (mem == NULL) {
res = -ENOMEM;
@@ -868,7 +868,7 @@ static int nvme_trans_ext_inq_page(struct nvme_ns *ns, struct sg_io_hdr *hdr,
goto out_mem;
}
- mem = dma_alloc_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns),
+ mem = dma_zalloc_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns),
&dma_addr, GFP_KERNEL);
if (mem == NULL) {
res = -ENOMEM;
@@ -1004,7 +1004,7 @@ static int nvme_trans_log_info_exceptions(struct nvme_ns *ns,
goto out_mem;
}
- mem = dma_alloc_coherent(&dev->pci_dev->dev,
+ mem = dma_zalloc_coherent(&dev->pci_dev->dev,
sizeof(struct nvme_smart_log),
&dma_addr, GFP_KERNEL);
if (mem == NULL) {
@@ -1072,7 +1072,7 @@ static int nvme_trans_log_temperature(struct nvme_ns *ns, struct sg_io_hdr *hdr,
goto out_mem;
}
- mem = dma_alloc_coherent(&dev->pci_dev->dev,
+ mem = dma_zalloc_coherent(&dev->pci_dev->dev,
sizeof(struct nvme_smart_log),
&dma_addr, GFP_KERNEL);
if (mem == NULL) {
@@ -1175,7 +1175,7 @@ static int nvme_trans_fill_blk_desc(struct nvme_ns *ns, struct sg_io_hdr *hdr,
else if (llbaa > 0 && len < MODE_PAGE_LLBAA_BLK_DES_LEN)
return SNTI_INTERNAL_ERROR;
- mem = dma_alloc_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns),
+ mem = dma_zalloc_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns),
&dma_addr, GFP_KERNEL);
if (mem == NULL) {
res = -ENOMEM;
@@ -1460,7 +1460,7 @@ static int nvme_trans_power_state(struct nvme_ns *ns, struct sg_io_hdr *hdr,
unsigned ps_desired = 0;
/* NVMe Controller Identify */
- mem = dma_alloc_coherent(&dev->pci_dev->dev,
+ mem = dma_zalloc_coherent(&dev->pci_dev->dev,
sizeof(struct nvme_id_ctrl),
&dma_addr, GFP_KERNEL);
if (mem == NULL) {
@@ -1786,7 +1786,7 @@ static int nvme_trans_fmt_set_blk_size_count(struct nvme_ns *ns,
*/
if (ns->mode_select_num_blocks == 0 || ns->mode_select_block_len == 0) {
- mem = dma_alloc_coherent(&dev->pci_dev->dev,
+ mem = dma_zalloc_coherent(&dev->pci_dev->dev,
sizeof(struct nvme_id_ns), &dma_addr, GFP_KERNEL);
if (mem == NULL) {
res = -ENOMEM;
@@ -1894,7 +1894,7 @@ static int nvme_trans_fmt_send_cmd(struct nvme_ns *ns, struct sg_io_hdr *hdr,
struct nvme_command c;
/* Loop thru LBAF's in id_ns to match reqd lbaf, put in cdw10 */
- mem = dma_alloc_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns),
+ mem = dma_zalloc_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns),
&dma_addr, GFP_KERNEL);
if (mem == NULL) {
res = -ENOMEM;
@@ -2447,7 +2447,7 @@ static int nvme_trans_read_capacity(struct nvme_ns *ns, struct sg_io_hdr *hdr,
resp_size = READ_CAP_16_RESP_SIZE;
}
- mem = dma_alloc_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns),
+ mem = dma_zalloc_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns),
&dma_addr, GFP_KERNEL);
if (mem == NULL) {
res = -ENOMEM;
@@ -2510,7 +2510,7 @@ static int nvme_trans_report_luns(struct nvme_ns *ns, struct sg_io_hdr *hdr,
goto out;
} else {
/* NVMe Controller Identify */
- mem = dma_alloc_coherent(&dev->pci_dev->dev,
+ mem = dma_zalloc_coherent(&dev->pci_dev->dev,
sizeof(struct nvme_id_ctrl),
&dma_addr, GFP_KERNEL);
if (mem == NULL) {
@@ -2876,7 +2876,7 @@ static int nvme_trans_unmap(struct nvme_ns *ns, struct sg_io_hdr *hdr,
goto out;
}
- range = dma_alloc_coherent(&dev->pci_dev->dev, ndesc * sizeof(*range),
+ range = dma_zalloc_coherent(&dev->pci_dev->dev, ndesc * sizeof(*range),
&dma_addr, GFP_KERNEL);
if (!range)
goto out;
@@ -2884,7 +2884,6 @@ static int nvme_trans_unmap(struct nvme_ns *ns, struct sg_io_hdr *hdr,
for (i = 0; i < ndesc; i++) {
range[i].nlb = cpu_to_le32(be32_to_cpu(plist->desc[i].nlb));
range[i].slba = cpu_to_le64(be64_to_cpu(plist->desc[i].slba));
- range[i].cattr = 0;
}
memset(&c, 0, sizeof(c));
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/1] Replace the dma_alloc_coherent() with dma_zalloc_coherent()
2014-07-15 18:11 [PATCH 1/1] Replace the dma_alloc_coherent() with dma_zalloc_coherent() Mundu
@ 2014-07-15 18:28 ` Matthew Wilcox
2014-07-16 5:57 ` mundu agarwal
0 siblings, 1 reply; 6+ messages in thread
From: Matthew Wilcox @ 2014-07-15 18:28 UTC (permalink / raw)
On Tue, Jul 15, 2014@11:41:46PM +0530, Mundu wrote:
> Signed-off-by: Mundu <mundu2510 at gmail.com>
why?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] Replace the dma_alloc_coherent() with dma_zalloc_coherent()
2014-07-15 18:28 ` Matthew Wilcox
@ 2014-07-16 5:57 ` mundu agarwal
2014-07-16 14:16 ` Matthew Wilcox
0 siblings, 1 reply; 6+ messages in thread
From: mundu agarwal @ 2014-07-16 5:57 UTC (permalink / raw)
Willy,
There is patch merged for dma_zalloc_coherent for cqee to avoid the
memset, where as memset still happing for same in nvme_init_queue. I
guess this is not needed, and rest are to maintain the consistancy. I
feel instead of mix use of the apis, should be use as clean/single
one. There are couple of places during initialization variables are
reseting to 0 after allocating the memory using zalloc which we can
avoid. In case this patch ok then my next task will be doing the
cleanup without any impacting the current functionality.
Regards,
Mundu
On Tue, Jul 15, 2014@11:58 PM, Matthew Wilcox <willy@linux.intel.com> wrote:
> On Tue, Jul 15, 2014@11:41:46PM +0530, Mundu wrote:
>> Signed-off-by: Mundu <mundu2510 at gmail.com>
>
> why?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] Replace the dma_alloc_coherent() with dma_zalloc_coherent()
2014-07-16 5:57 ` mundu agarwal
@ 2014-07-16 14:16 ` Matthew Wilcox
2014-07-16 14:52 ` Keith Busch
0 siblings, 1 reply; 6+ messages in thread
From: Matthew Wilcox @ 2014-07-16 14:16 UTC (permalink / raw)
On Wed, Jul 16, 2014@11:27:38AM +0530, mundu agarwal wrote:
> Willy,
>
> There is patch merged for dma_zalloc_coherent for cqee to avoid the
> memset, where as memset still happing for same in nvme_init_queue. I
> guess this is not needed, and rest are to maintain the consistancy. I
> feel instead of mix use of the apis, should be use as clean/single
> one. There are couple of places during initialization variables are
> reseting to 0 after allocating the memory using zalloc which we can
> avoid. In case this patch ok then my next task will be doing the
> cleanup without any impacting the current functionality.
Better not to guess. Joe's quite careful to only do the ones that are
obviously correct. I believe your patches to be incorrect, since some of
the places don't need to be zeroed (eg SQEs are zeroed before each use).
In the specific case of nvme_init_queue() zeroing the memory, I think
it's necessary on that path because that path is called on resume from
D3 and we need to reinitialise the CQ state to all zeroes. Keith should
be able to confirm that.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] Replace the dma_alloc_coherent() with dma_zalloc_coherent()
2014-07-16 14:16 ` Matthew Wilcox
@ 2014-07-16 14:52 ` Keith Busch
2014-07-16 17:08 ` mundu agarwal
0 siblings, 1 reply; 6+ messages in thread
From: Keith Busch @ 2014-07-16 14:52 UTC (permalink / raw)
On Wed, 16 Jul 2014, Matthew Wilcox wrote:
> On Wed, Jul 16, 2014@11:27:38AM +0530, mundu agarwal wrote:
>> Willy,
>>
>> There is patch merged for dma_zalloc_coherent for cqee to avoid the
>> memset, where as memset still happing for same in nvme_init_queue. I
>> guess this is not needed, and rest are to maintain the consistancy. I
>> feel instead of mix use of the apis, should be use as clean/single
>> one. There are couple of places during initialization variables are
>> reseting to 0 after allocating the memory using zalloc which we can
>> avoid. In case this patch ok then my next task will be doing the
>> cleanup without any impacting the current functionality.
>
> Better not to guess. Joe's quite careful to only do the ones that are
> obviously correct. I believe your patches to be incorrect, since some of
> the places don't need to be zeroed (eg SQEs are zeroed before each use).
>
> In the specific case of nvme_init_queue() zeroing the memory, I think
> it's necessary on that path because that path is called on resume from
> D3 and we need to reinitialise the CQ state to all zeroes. Keith should
> be able to confirm that.
Yes, we have to clear the cmdid data and completion queues in init for
the resume and reset cases otherwise we would leak command id's that
were active at the time we shut down the device, or potentially read a
stale cq "phase" and misinterpret that as a new completion.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] Replace the dma_alloc_coherent() with dma_zalloc_coherent()
2014-07-16 14:52 ` Keith Busch
@ 2014-07-16 17:08 ` mundu agarwal
0 siblings, 0 replies; 6+ messages in thread
From: mundu agarwal @ 2014-07-16 17:08 UTC (permalink / raw)
Thanks Keith/Willy for pointing out the missing link.
On Wed, Jul 16, 2014@8:22 PM, Keith Busch <keith.busch@intel.com> wrote:
> On Wed, 16 Jul 2014, Matthew Wilcox wrote:
>>
>> On Wed, Jul 16, 2014@11:27:38AM +0530, mundu agarwal wrote:
>>>
>>> Willy,
>>>
>>> There is patch merged for dma_zalloc_coherent for cqee to avoid the
>>> memset, where as memset still happing for same in nvme_init_queue. I
>>> guess this is not needed, and rest are to maintain the consistancy. I
>>> feel instead of mix use of the apis, should be use as clean/single
>>> one. There are couple of places during initialization variables are
>>> reseting to 0 after allocating the memory using zalloc which we can
>>> avoid. In case this patch ok then my next task will be doing the
>>> cleanup without any impacting the current functionality.
>>
>>
>> Better not to guess. Joe's quite careful to only do the ones that are
>> obviously correct. I believe your patches to be incorrect, since some of
>> the places don't need to be zeroed (eg SQEs are zeroed before each use).
>>
>> In the specific case of nvme_init_queue() zeroing the memory, I think
>> it's necessary on that path because that path is called on resume from
>> D3 and we need to reinitialise the CQ state to all zeroes. Keith should
>> be able to confirm that.
>
>
> Yes, we have to clear the cmdid data and completion queues in init for
> the resume and reset cases otherwise we would leak command id's that
> were active at the time we shut down the device, or potentially read a
> stale cq "phase" and misinterpret that as a new completion.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-16 17:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-15 18:11 [PATCH 1/1] Replace the dma_alloc_coherent() with dma_zalloc_coherent() Mundu
2014-07-15 18:28 ` Matthew Wilcox
2014-07-16 5:57 ` mundu agarwal
2014-07-16 14:16 ` Matthew Wilcox
2014-07-16 14:52 ` Keith Busch
2014-07-16 17:08 ` mundu agarwal
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.