Linux-PCI Archive on lore.kernel.org
 help / Atom feed
* [PATCH v2] PCI: endpoint: functions: Use kmemdup instead of duplicating its function
@ 2018-12-06 12:52 Wen Yang
  2019-02-08 12:20 ` Lorenzo Pieralisi
  0 siblings, 1 reply; 5+ messages in thread
From: Wen Yang @ 2018-12-06 12:52 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Lorenzo Pieralisi, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, zhong.weidong, Wen Yang,
	Gustavo Pimentel, Niklas Cassel, Greg Kroah-Hartman,
	Cyrille Pitchen

kmemdup has implemented the function that kmalloc() + memcpy().
We prefer to kmemdup rather than code opened implementation.

This issue was detected with the help of coccinelle.

Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
CC: Kishon Vijay Abraham I <kishon@ti.com>
CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
CC: Niklas Cassel <niklas.cassel@axis.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
CC: linux-pci@vger.kernel.org (open list:PCI ENDPOINT SUBSYSTEM)
CC: linux-kernel@vger.kernel.org (open list)
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 3e86fa3c7da3..8df6c019f8a2 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -169,14 +169,12 @@ static int pci_epf_test_read(struct pci_epf_test *epf_test)
 		goto err_addr;
 	}
 
-	buf = kzalloc(reg->size, GFP_KERNEL);
+	buf = kmemdup(src_addr, reg->size, GFP_KERNEL);
 	if (!buf) {
 		ret = -ENOMEM;
 		goto err_map_addr;
 	}
 
-	memcpy(buf, src_addr, reg->size);
-
 	crc32 = crc32_le(~0, buf, reg->size);
 	if (crc32 != reg->checksum)
 		ret = -EIO;
-- 
2.19.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] PCI: endpoint: functions: Use kmemdup instead of duplicating its function
  2018-12-06 12:52 [PATCH v2] PCI: endpoint: functions: Use kmemdup instead of duplicating its function Wen Yang
@ 2019-02-08 12:20 ` Lorenzo Pieralisi
  2019-02-11  5:48   ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Pieralisi @ 2019-02-08 12:20 UTC (permalink / raw)
  To: Wen Yang, Kishon Vijay Abraham I
  Cc: Bjorn Helgaas, linux-pci, linux-kernel, zhong.weidong,
	Gustavo Pimentel, Niklas Cassel, Greg Kroah-Hartman,
	Cyrille Pitchen

On Thu, Dec 06, 2018 at 08:52:25PM +0800, Wen Yang wrote:
> kmemdup has implemented the function that kmalloc() + memcpy().
> We prefer to kmemdup rather than code opened implementation.
> 
> This issue was detected with the help of coccinelle.
> 
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> CC: Kishon Vijay Abraham I <kishon@ti.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> CC: Bjorn Helgaas <bhelgaas@google.com>
> CC: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> CC: Niklas Cassel <niklas.cassel@axis.com>
> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> CC: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
> CC: linux-pci@vger.kernel.org (open list:PCI ENDPOINT SUBSYSTEM)
> CC: linux-kernel@vger.kernel.org (open list)
> ---
>  drivers/pci/endpoint/functions/pci-epf-test.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Kishon,

this looks OK to me, anything I am missing ?

Lorenzo

> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 3e86fa3c7da3..8df6c019f8a2 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -169,14 +169,12 @@ static int pci_epf_test_read(struct pci_epf_test *epf_test)
>  		goto err_addr;
>  	}
>  
> -	buf = kzalloc(reg->size, GFP_KERNEL);
> +	buf = kmemdup(src_addr, reg->size, GFP_KERNEL);
>  	if (!buf) {
>  		ret = -ENOMEM;
>  		goto err_map_addr;
>  	}
>  
> -	memcpy(buf, src_addr, reg->size);
> -
>  	crc32 = crc32_le(~0, buf, reg->size);
>  	if (crc32 != reg->checksum)
>  		ret = -EIO;
> -- 
> 2.19.1
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] PCI: endpoint: functions: Use kmemdup instead of duplicating its function
  2019-02-08 12:20 ` Lorenzo Pieralisi
@ 2019-02-11  5:48   ` Kishon Vijay Abraham I
  2019-02-11  9:15     ` Gustavo Pimentel
  0 siblings, 1 reply; 5+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-11  5:48 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Wen Yang
  Cc: Bjorn Helgaas, linux-pci, linux-kernel, zhong.weidong,
	Gustavo Pimentel, Niklas Cassel, Greg Kroah-Hartman,
	Cyrille Pitchen

Hi Lorenzo,

On 08/02/19 5:50 PM, Lorenzo Pieralisi wrote:
> On Thu, Dec 06, 2018 at 08:52:25PM +0800, Wen Yang wrote:
>> kmemdup has implemented the function that kmalloc() + memcpy().
>> We prefer to kmemdup rather than code opened implementation.
>>
>> This issue was detected with the help of coccinelle.
>>
>> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
>> CC: Kishon Vijay Abraham I <kishon@ti.com>
>> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> CC: Bjorn Helgaas <bhelgaas@google.com>
>> CC: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> CC: Niklas Cassel <niklas.cassel@axis.com>
>> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> CC: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
>> CC: linux-pci@vger.kernel.org (open list:PCI ENDPOINT SUBSYSTEM)
>> CC: linux-kernel@vger.kernel.org (open list)
>> ---
>>  drivers/pci/endpoint/functions/pci-epf-test.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> Kishon,
> 
> this looks OK to me, anything I am missing ?

For the existing code this might seem the right thing to do but ideally the
memcpy here should be changed to memcpy_fromio/memcpy_toio.

Also later when we plan to use DMA (on the endpoint) for data transfer, we have
to use kzalloc and dma_map_single APIs.

So maybe the right thing would be to just fix it to use memcpy_fromio here.

Thanks
Kishon
> 
> Lorenzo
> 
>> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
>> index 3e86fa3c7da3..8df6c019f8a2 100644
>> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
>> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
>> @@ -169,14 +169,12 @@ static int pci_epf_test_read(struct pci_epf_test *epf_test)
>>  		goto err_addr;
>>  	}
>>  
>> -	buf = kzalloc(reg->size, GFP_KERNEL);
>> +	buf = kmemdup(src_addr, reg->size, GFP_KERNEL);
>>  	if (!buf) {
>>  		ret = -ENOMEM;
>>  		goto err_map_addr;
>>  	}
>>  
>> -	memcpy(buf, src_addr, reg->size);
>> -
>>  	crc32 = crc32_le(~0, buf, reg->size);
>>  	if (crc32 != reg->checksum)
>>  		ret = -EIO;
>> -- 
>> 2.19.1
>>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] PCI: endpoint: functions: Use kmemdup instead of duplicating its function
  2019-02-11  5:48   ` Kishon Vijay Abraham I
@ 2019-02-11  9:15     ` Gustavo Pimentel
  2019-02-11 12:37       ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 5+ messages in thread
From: Gustavo Pimentel @ 2019-02-11  9:15 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Lorenzo Pieralisi, Wen Yang
  Cc: Bjorn Helgaas, linux-pci, linux-kernel, zhong.weidong,
	Gustavo Pimentel, Niklas Cassel, Greg Kroah-Hartman,
	Cyrille Pitchen

On 11/02/2019 05:48, Kishon Vijay Abraham I wrote:
> Hi Lorenzo,
> 
> On 08/02/19 5:50 PM, Lorenzo Pieralisi wrote:
>> On Thu, Dec 06, 2018 at 08:52:25PM +0800, Wen Yang wrote:
>>> kmemdup has implemented the function that kmalloc() + memcpy().
>>> We prefer to kmemdup rather than code opened implementation.
>>>
>>> This issue was detected with the help of coccinelle.
>>>
>>> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
>>> CC: Kishon Vijay Abraham I <kishon@ti.com>
>>> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>> CC: Bjorn Helgaas <bhelgaas@google.com>
>>> CC: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>> CC: Niklas Cassel <niklas.cassel@axis.com>
>>> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> CC: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
>>> CC: linux-pci@vger.kernel.org (open list:PCI ENDPOINT SUBSYSTEM)
>>> CC: linux-kernel@vger.kernel.org (open list)
>>> ---
>>>  drivers/pci/endpoint/functions/pci-epf-test.c | 4 +---
>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> Kishon,
>>
>> this looks OK to me, anything I am missing ?
> 
> For the existing code this might seem the right thing to do but ideally the
> memcpy here should be changed to memcpy_fromio/memcpy_toio.
> 
> Also later when we plan to use DMA (on the endpoint) for data transfer, we have
> to use kzalloc and dma_map_single APIs.

Are you considering to use the eDMA driver that I'm developing?

> 
> So maybe the right thing would be to just fix it to use memcpy_fromio here.
> 
> Thanks
> Kishon
>>
>> Lorenzo
>>
>>> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
>>> index 3e86fa3c7da3..8df6c019f8a2 100644
>>> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
>>> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
>>> @@ -169,14 +169,12 @@ static int pci_epf_test_read(struct pci_epf_test *epf_test)
>>>  		goto err_addr;
>>>  	}
>>>  
>>> -	buf = kzalloc(reg->size, GFP_KERNEL);
>>> +	buf = kmemdup(src_addr, reg->size, GFP_KERNEL);
>>>  	if (!buf) {
>>>  		ret = -ENOMEM;
>>>  		goto err_map_addr;
>>>  	}
>>>  
>>> -	memcpy(buf, src_addr, reg->size);
>>> -
>>>  	crc32 = crc32_le(~0, buf, reg->size);
>>>  	if (crc32 != reg->checksum)
>>>  		ret = -EIO;
>>> -- 
>>> 2.19.1
>>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] PCI: endpoint: functions: Use kmemdup instead of duplicating its function
  2019-02-11  9:15     ` Gustavo Pimentel
@ 2019-02-11 12:37       ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 5+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-11 12:37 UTC (permalink / raw)
  To: Gustavo Pimentel, Lorenzo Pieralisi, Wen Yang
  Cc: Bjorn Helgaas, linux-pci, linux-kernel, zhong.weidong,
	Niklas Cassel, Greg Kroah-Hartman, Cyrille Pitchen

Hi Gustavo,

On 11/02/19 2:45 PM, Gustavo Pimentel wrote:
> On 11/02/2019 05:48, Kishon Vijay Abraham I wrote:
>> Hi Lorenzo,
>>
>> On 08/02/19 5:50 PM, Lorenzo Pieralisi wrote:
>>> On Thu, Dec 06, 2018 at 08:52:25PM +0800, Wen Yang wrote:
>>>> kmemdup has implemented the function that kmalloc() + memcpy().
>>>> We prefer to kmemdup rather than code opened implementation.
>>>>
>>>> This issue was detected with the help of coccinelle.
>>>>
>>>> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
>>>> CC: Kishon Vijay Abraham I <kishon@ti.com>
>>>> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>> CC: Bjorn Helgaas <bhelgaas@google.com>
>>>> CC: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>> CC: Niklas Cassel <niklas.cassel@axis.com>
>>>> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> CC: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
>>>> CC: linux-pci@vger.kernel.org (open list:PCI ENDPOINT SUBSYSTEM)
>>>> CC: linux-kernel@vger.kernel.org (open list)
>>>> ---
>>>>  drivers/pci/endpoint/functions/pci-epf-test.c | 4 +---
>>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> Kishon,
>>>
>>> this looks OK to me, anything I am missing ?
>>
>> For the existing code this might seem the right thing to do but ideally the
>> memcpy here should be changed to memcpy_fromio/memcpy_toio.
>>
>> Also later when we plan to use DMA (on the endpoint) for data transfer, we have
>> to use kzalloc and dma_map_single APIs.
> 
> Are you considering to use the eDMA driver that I'm developing?

I used system DMA for my testing. But in the DesignWare driver, we should be
able to choose between system DMA or any other DMA based on the platforms
capabilities.

Thanks
Kishon

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-06 12:52 [PATCH v2] PCI: endpoint: functions: Use kmemdup instead of duplicating its function Wen Yang
2019-02-08 12:20 ` Lorenzo Pieralisi
2019-02-11  5:48   ` Kishon Vijay Abraham I
2019-02-11  9:15     ` Gustavo Pimentel
2019-02-11 12:37       ` Kishon Vijay Abraham I

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org linux-pci@archiver.kernel.org
	public-inbox-index linux-pci


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/ public-inbox