diff mbox series

[v2] mmc: host: use kzalloc instead of kmalloc and memset

Message ID 1576567086-11469-1-git-send-email-zhangpan26@huawei.com (mailing list archive)
State New, archived
Headers show
Series [v2] mmc: host: use kzalloc instead of kmalloc and memset | expand

Commit Message

Pan Zhang Dec. 17, 2019, 7:18 a.m. UTC 
Signed-off-by: Pan Zhang <zhangpan26@huawei.com>
---
 drivers/mmc/host/vub300.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

Comments

David Rientjes Dec. 17, 2019, 7:32 a.m. UTC | #1 
On Tue, 17 Dec 2019, Pan Zhang wrote:

> Signed-off-by: Pan Zhang <zhangpan26@huawei.com>
> ---
>  drivers/mmc/host/vub300.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
> index 6ced1b7..e18931d 100644
> --- a/drivers/mmc/host/vub300.c
> +++ b/drivers/mmc/host/vub300.c
> @@ -1227,12 +1227,10 @@ static void __download_offload_pseudocode(struct vub300_mmc_host *vub300,
>  	size -= 1;
>  	if (interrupt_size < size) {
>  		u16 xfer_length = roundup_to_multiple_of_64(interrupt_size);
> -		u8 *xfer_buffer = kmalloc(xfer_length, GFP_KERNEL);
> +		u8 *xfer_buffer = kzalloc(xfer_length, GFP_KERNEL);
>  		if (xfer_buffer) {
>  			int retval;
>  			memcpy(xfer_buffer, data, interrupt_size);
> -			memset(xfer_buffer + interrupt_size, 0,
> -			       xfer_length - interrupt_size);
>  			size -= interrupt_size;
>  			data += interrupt_size;
>  			retval =
> @@ -1270,12 +1268,10 @@ static void __download_offload_pseudocode(struct vub300_mmc_host *vub300,
>  	size -= 1;
>  	if (ts < size) {
>  		u16 xfer_length = roundup_to_multiple_of_64(ts);
> -		u8 *xfer_buffer = kmalloc(xfer_length, GFP_KERNEL);
> +		u8 *xfer_buffer = kzalloc(xfer_length, GFP_KERNEL);
>  		if (xfer_buffer) {
>  			int retval;
>  			memcpy(xfer_buffer, data, ts);
> -			memset(xfer_buffer + ts, 0,
> -			       xfer_length - ts);
>  			size -= ts;
>  			data += ts;
>  			retval =

I think the previous code is an optimization since the first 
interrupt_size bytes or ts bytes of xfer_buffer would otherwise 
unnecessarily be zeroed and then copied to.
Greg KH Dec. 17, 2019, 7:32 a.m. UTC | #2 
On Tue, Dec 17, 2019 at 03:18:06PM +0800, Pan Zhang wrote:
> Signed-off-by: Pan Zhang <zhangpan26@huawei.com>
> ---
>  drivers/mmc/host/vub300.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)

What changed from v1?  Always put that below the --- line.
Pan Zhang Dec. 17, 2019, 2:17 p.m. UTC | #3 
On Tue, 17 Dec 2019, 15:33 Greg KH <gregkh@linuxfoundation.org> wrote:
>> Signed-off-by: Pan Zhang <zhangpan26@huawei.com>
>> ---
>>  drivers/mmc/host/vub300.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)

>I know I can not take patches without any changelog text, hopefully other maintainers also do the same.

>Please fix,

and 

>> Signed-off-by: Pan Zhang <zhangpan26@huawei.com>
>> ---
>>  drivers/mmc/host/vub300.c | 12 ++++--------
>>  1 file changed, 4 insertions(+), 8 deletions(-)

>What changed from v1?  Always put that below the --- line.

I am so sorry about that I didnot explain this patch clearly.
I complemented patch v2 just after sending the patch v1 because I found that following code has similar problems.

My changelog text can be also my reply to the following question:

On Tue, 17 Dec 2019, 15:32 David Rientjes <rientjes@google.com> wrote:
>I think the previous code is an optimization since the first interrupt_size bytes
>or ts bytes of xfer_buffer would otherwise unnecessarily be zeroed and then copied to.

Part of the memory will be written twice after this change, but that 
should be negligible.
1. xfer_buffer or xfer_buffer - interrupt_size, it won't be big.
2. __download_offload_pseudocode func wouldnot be called frequently.
  

Signed-off-by: Pan Zhang <zhangpan26@huawei.com>
---
 drivers/mmc/host/vub300.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
index 6ced1b7..e18931d 100644
--- a/drivers/mmc/host/vub300.c
+++ b/drivers/mmc/host/vub300.c
@@ -1227,12 +1227,10 @@ static void __download_offload_pseudocode(struct vub300_mmc_host *vub300,
 	size -= 1;
 	if (interrupt_size < size) {
 		u16 xfer_length = roundup_to_multiple_of_64(interrupt_size);
-		u8 *xfer_buffer = kmalloc(xfer_length, GFP_KERNEL);
+		u8 *xfer_buffer = kzalloc(xfer_length, GFP_KERNEL);
 		if (xfer_buffer) {
 			int retval;
 			memcpy(xfer_buffer, data, interrupt_size);
-			memset(xfer_buffer + interrupt_size, 0,
-			       xfer_length - interrupt_size);
 			size -= interrupt_size;
 			data += interrupt_size;
 			retval =
@@ -1270,12 +1268,10 @@ static void __download_offload_pseudocode(struct vub300_mmc_host *vub300,
 	size -= 1;
 	if (ts < size) {
 		u16 xfer_length = roundup_to_multiple_of_64(ts);
-		u8 *xfer_buffer = kmalloc(xfer_length, GFP_KERNEL);
+		u8 *xfer_buffer = kzalloc(xfer_length, GFP_KERNEL);
 		if (xfer_buffer) {
 			int retval;
 			memcpy(xfer_buffer, data, ts);
-			memset(xfer_buffer + ts, 0,
-			       xfer_length - ts);
 			size -= ts;
 			data += ts;
 			retval =
@@ -1465,7 +1461,7 @@ static int __command_read_data(struct vub300_mmc_host *vub300,
 			}
 		}
 	} else {
-		u8 *buf = kmalloc(padded_length, GFP_KERNEL);
+		u8 *buf = kzalloc(padded_length, GFP_KERNEL);
 		if (buf) {
 			int result;
 			unsigned pipe = usb_rcvbulkpipe(vub300->udev,
@@ -2024,7 +2020,7 @@ static void vub300_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		msleep(600);
 		vub300->card_powered = 1;
 	} else if (ios->power_mode == MMC_POWER_ON) {
-		u8 *buf = kmalloc(8, GFP_KERNEL);
+		u8 *buf = kzalloc(8, GFP_KERNEL);
 		if (buf) {
 			__set_clock_speed(vub300, buf, ios);
 			kfree(buf);
diff mbox series

Patch

diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
index 6ced1b7..e18931d 100644
--- a/drivers/mmc/host/vub300.c
+++ b/drivers/mmc/host/vub300.c
@@ -1227,12 +1227,10 @@  static void __download_offload_pseudocode(struct vub300_mmc_host *vub300,
 	size -= 1;
 	if (interrupt_size < size) {
 		u16 xfer_length = roundup_to_multiple_of_64(interrupt_size);
-		u8 *xfer_buffer = kmalloc(xfer_length, GFP_KERNEL);
+		u8 *xfer_buffer = kzalloc(xfer_length, GFP_KERNEL);
 		if (xfer_buffer) {
 			int retval;
 			memcpy(xfer_buffer, data, interrupt_size);
-			memset(xfer_buffer + interrupt_size, 0,
-			       xfer_length - interrupt_size);
 			size -= interrupt_size;
 			data += interrupt_size;
 			retval =
@@ -1270,12 +1268,10 @@  static void __download_offload_pseudocode(struct vub300_mmc_host *vub300,
 	size -= 1;
 	if (ts < size) {
 		u16 xfer_length = roundup_to_multiple_of_64(ts);
-		u8 *xfer_buffer = kmalloc(xfer_length, GFP_KERNEL);
+		u8 *xfer_buffer = kzalloc(xfer_length, GFP_KERNEL);
 		if (xfer_buffer) {
 			int retval;
 			memcpy(xfer_buffer, data, ts);
-			memset(xfer_buffer + ts, 0,
-			       xfer_length - ts);
 			size -= ts;
 			data += ts;
 			retval =
@@ -1465,7 +1461,7 @@  static int __command_read_data(struct vub300_mmc_host *vub300,
 			}
 		}
 	} else {
-		u8 *buf = kmalloc(padded_length, GFP_KERNEL);
+		u8 *buf = kzalloc(padded_length, GFP_KERNEL);
 		if (buf) {
 			int result;
 			unsigned pipe = usb_rcvbulkpipe(vub300->udev,
@@ -2024,7 +2020,7 @@  static void vub300_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		msleep(600);
 		vub300->card_powered = 1;
 	} else if (ios->power_mode == MMC_POWER_ON) {
-		u8 *buf = kmalloc(8, GFP_KERNEL);
+		u8 *buf = kzalloc(8, GFP_KERNEL);
 		if (buf) {
 			__set_clock_speed(vub300, buf, ios);
 			kfree(buf);