* [PATCH] wlcore: consolidate kmalloc + memset 0 into kzalloc @ 2015-12-21 16:47 Nicholas Mc Guire 2015-12-21 22:56 ` Julian Calaby 2015-12-22 7:06 ` Souptick Joarder 0 siblings, 2 replies; 5+ messages in thread From: Nicholas Mc Guire @ 2015-12-21 16:47 UTC (permalink / raw) To: Kalle Valo Cc: Eliad Peller, Johannes Berg, Arik Nemtsov, Tony Lindgren, Luciano Coelho, Eyal Reizer, Emmanuel Grumbach, linux-wireless, netdev, linux-kernel, Nicholas Mc Guire This is an API consolidation only. The use of kmalloc + memset to 0 is equivalent to kzalloc. Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> --- Found by coccinelle script (relaxed version of scripts/coccinelle/api/alloc/kzalloc-simple.cocci) Patch was compile tested with: x86_64_defconfig + CONFIG_WL12XX=m (implies CONFIG_WLCORE=m) Patch is against linux-next (localversion-next is -next-20151221) drivers/net/wireless/ti/wlcore/main.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c index ec7f6af..dfc49bf 100644 --- a/drivers/net/wireless/ti/wlcore/main.c +++ b/drivers/net/wireless/ti/wlcore/main.c @@ -838,7 +838,7 @@ static void wl12xx_read_fwlog_panic(struct wl1271 *wl) wl1271_info("Reading FW panic log"); - block = kmalloc(wl->fw_mem_block_size, GFP_KERNEL); + block = kzalloc(wl->fw_mem_block_size, GFP_KERNEL); if (!block) return; @@ -885,7 +885,6 @@ static void wl12xx_read_fwlog_panic(struct wl1271 *wl) goto out; } - memset(block, 0, wl->fw_mem_block_size); ret = wlcore_read_hwaddr(wl, addr, block, wl->fw_mem_block_size, false); -- 2.1.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] wlcore: consolidate kmalloc + memset 0 into kzalloc 2015-12-21 16:47 [PATCH] wlcore: consolidate kmalloc + memset 0 into kzalloc Nicholas Mc Guire @ 2015-12-21 22:56 ` Julian Calaby 2015-12-22 7:29 ` Nicholas Mc Guire 2015-12-22 7:06 ` Souptick Joarder 1 sibling, 1 reply; 5+ messages in thread From: Julian Calaby @ 2015-12-21 22:56 UTC (permalink / raw) To: Nicholas Mc Guire Cc: Kalle Valo, Eliad Peller, Johannes Berg, Arik Nemtsov, Tony Lindgren, Luciano Coelho, Eyal Reizer, Emmanuel Grumbach, linux-wireless, netdev, linux-kernel Hi, On Tue, Dec 22, 2015 at 3:47 AM, Nicholas Mc Guire <hofrat@osadl.org> wrote: > This is an API consolidation only. The use of kmalloc + memset to 0 > is equivalent to kzalloc. > > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> > --- > > Found by coccinelle script (relaxed version of > scripts/coccinelle/api/alloc/kzalloc-simple.cocci) > > Patch was compile tested with: x86_64_defconfig + > CONFIG_WL12XX=m (implies CONFIG_WLCORE=m) > > Patch is against linux-next (localversion-next is -next-20151221) > > drivers/net/wireless/ti/wlcore/main.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c > index ec7f6af..dfc49bf 100644 > --- a/drivers/net/wireless/ti/wlcore/main.c > +++ b/drivers/net/wireless/ti/wlcore/main.c > @@ -838,7 +838,7 @@ static void wl12xx_read_fwlog_panic(struct wl1271 *wl) > > wl1271_info("Reading FW panic log"); > > - block = kmalloc(wl->fw_mem_block_size, GFP_KERNEL); > + block = kzalloc(wl->fw_mem_block_size, GFP_KERNEL); > if (!block) > return; > > @@ -885,7 +885,6 @@ static void wl12xx_read_fwlog_panic(struct wl1271 *wl) > goto out; > } > - memset(block, 0, wl->fw_mem_block_size); I don't think you can't remove this line. It appears that the loop this is part of resets block to be all zero, reads a chunk of data in, then operates on it. I'm guessing that the code after the following line expects that there isn't any data left over from previous runs through the loop. > ret = wlcore_read_hwaddr(wl, addr, block, > wl->fw_mem_block_size, false); > > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] wlcore: consolidate kmalloc + memset 0 into kzalloc 2015-12-21 22:56 ` Julian Calaby @ 2015-12-22 7:29 ` Nicholas Mc Guire 2015-12-22 7:52 ` Julian Calaby 0 siblings, 1 reply; 5+ messages in thread From: Nicholas Mc Guire @ 2015-12-22 7:29 UTC (permalink / raw) To: Julian Calaby Cc: Nicholas Mc Guire, Kalle Valo, Eliad Peller, Johannes Berg, Arik Nemtsov, Tony Lindgren, Luciano Coelho, Eyal Reizer, Emmanuel Grumbach, linux-wireless, netdev, linux-kernel On Tue, Dec 22, 2015 at 09:56:10AM +1100, Julian Calaby wrote: > Hi, > > On Tue, Dec 22, 2015 at 3:47 AM, Nicholas Mc Guire <hofrat@osadl.org> wrote: > > This is an API consolidation only. The use of kmalloc + memset to 0 > > is equivalent to kzalloc. > > > > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> > > --- > > > > Found by coccinelle script (relaxed version of > > scripts/coccinelle/api/alloc/kzalloc-simple.cocci) > > > > Patch was compile tested with: x86_64_defconfig + > > CONFIG_WL12XX=m (implies CONFIG_WLCORE=m) > > > > Patch is against linux-next (localversion-next is -next-20151221) > > > > drivers/net/wireless/ti/wlcore/main.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c > > index ec7f6af..dfc49bf 100644 > > --- a/drivers/net/wireless/ti/wlcore/main.c > > +++ b/drivers/net/wireless/ti/wlcore/main.c > > @@ -838,7 +838,7 @@ static void wl12xx_read_fwlog_panic(struct wl1271 *wl) > > > > wl1271_info("Reading FW panic log"); > > > > - block = kmalloc(wl->fw_mem_block_size, GFP_KERNEL); > > + block = kzalloc(wl->fw_mem_block_size, GFP_KERNEL); > > if (!block) > > return; > > > > @@ -885,7 +885,6 @@ static void wl12xx_read_fwlog_panic(struct wl1271 *wl) > > goto out; > > } > > - memset(block, 0, wl->fw_mem_block_size); > > I don't think you can't remove this line. It appears that the loop > this is part of resets block to be all zero, reads a chunk of data in, > then operates on it. I'm guessing that the code after the following > line expects that there isn't any data left over from previous runs > through the loop. > the rational for this being ok is thta the copy operation into block is: ret = wlcore_read_hwaddr(wl, addr, block, wl->fw_mem_block_size, false); this will end up in the .read methods where block should be completely overwritten (length == full block size), so within the loop if successful this should be correct - if not successful it would "goto out" witout using the content of block. Am I overlooking something here ? thx! hofrat ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] wlcore: consolidate kmalloc + memset 0 into kzalloc 2015-12-22 7:29 ` Nicholas Mc Guire @ 2015-12-22 7:52 ` Julian Calaby 0 siblings, 0 replies; 5+ messages in thread From: Julian Calaby @ 2015-12-22 7:52 UTC (permalink / raw) To: Nicholas Mc Guire Cc: Nicholas Mc Guire, Kalle Valo, Eliad Peller, Johannes Berg, Arik Nemtsov, Tony Lindgren, Luciano Coelho, Eyal Reizer, Emmanuel Grumbach, linux-wireless, netdev, linux-kernel Hi Nicholas, On Tue, Dec 22, 2015 at 6:29 PM, Nicholas Mc Guire <der.herr@hofr.at> wrote: > On Tue, Dec 22, 2015 at 09:56:10AM +1100, Julian Calaby wrote: >> Hi, >> >> On Tue, Dec 22, 2015 at 3:47 AM, Nicholas Mc Guire <hofrat@osadl.org> wrote: >> > This is an API consolidation only. The use of kmalloc + memset to 0 >> > is equivalent to kzalloc. >> > >> > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> >> > --- >> > >> > Found by coccinelle script (relaxed version of >> > scripts/coccinelle/api/alloc/kzalloc-simple.cocci) >> > >> > Patch was compile tested with: x86_64_defconfig + >> > CONFIG_WL12XX=m (implies CONFIG_WLCORE=m) >> > >> > Patch is against linux-next (localversion-next is -next-20151221) >> > >> > drivers/net/wireless/ti/wlcore/main.c | 3 +-- >> > 1 file changed, 1 insertion(+), 2 deletions(-) >> > >> > diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c >> > index ec7f6af..dfc49bf 100644 >> > --- a/drivers/net/wireless/ti/wlcore/main.c >> > +++ b/drivers/net/wireless/ti/wlcore/main.c >> > @@ -838,7 +838,7 @@ static void wl12xx_read_fwlog_panic(struct wl1271 *wl) >> > >> > wl1271_info("Reading FW panic log"); >> > >> > - block = kmalloc(wl->fw_mem_block_size, GFP_KERNEL); >> > + block = kzalloc(wl->fw_mem_block_size, GFP_KERNEL); >> > if (!block) >> > return; >> > >> > @@ -885,7 +885,6 @@ static void wl12xx_read_fwlog_panic(struct wl1271 *wl) >> > goto out; >> > } >> > - memset(block, 0, wl->fw_mem_block_size); >> >> I don't think you can't remove this line. It appears that the loop >> this is part of resets block to be all zero, reads a chunk of data in, >> then operates on it. I'm guessing that the code after the following >> line expects that there isn't any data left over from previous runs >> through the loop. >> > the rational for this being ok is thta the copy operation into block is: > ret = wlcore_read_hwaddr(wl, addr, block, > wl->fw_mem_block_size, false); > > this will end up in the .read methods where block should be completely > overwritten (length == full block size), so within the loop if successful > this should be correct - if not successful it would "goto out" witout > using the content of block. > > Am I overlooking something here ? It's quite possible I am. I didn't look at the implementation of wlcore_read_hwaddr() so I'm only guessing that it could do partial reads. That said, if it does overwrite the entire buffer each time, then there's no need to use kzalloc() to allocate the buffer in the first place. Either way, it needs a review from someone more familiar with the code. Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] wlcore: consolidate kmalloc + memset 0 into kzalloc 2015-12-21 16:47 [PATCH] wlcore: consolidate kmalloc + memset 0 into kzalloc Nicholas Mc Guire 2015-12-21 22:56 ` Julian Calaby @ 2015-12-22 7:06 ` Souptick Joarder 1 sibling, 0 replies; 5+ messages in thread From: Souptick Joarder @ 2015-12-22 7:06 UTC (permalink / raw) To: Nicholas Mc Guire Cc: Kalle Valo, Eliad Peller, Johannes Berg, Arik Nemtsov, Tony Lindgren, Luciano Coelho, Eyal Reizer, Emmanuel Grumbach, linux-wireless, netdev, linux-kernel On Mon, Dec 21, 2015 at 10:17 PM, Nicholas Mc Guire <hofrat@osadl.org> wrote: > This is an API consolidation only. The use of kmalloc + memset to 0 > is equivalent to kzalloc. > > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> > --- > > Found by coccinelle script (relaxed version of > scripts/coccinelle/api/alloc/kzalloc-simple.cocci) > > Patch was compile tested with: x86_64_defconfig + > CONFIG_WL12XX=m (implies CONFIG_WLCORE=m) > > Patch is against linux-next (localversion-next is -next-20151221) > > drivers/net/wireless/ti/wlcore/main.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c > index ec7f6af..dfc49bf 100644 > --- a/drivers/net/wireless/ti/wlcore/main.c > +++ b/drivers/net/wireless/ti/wlcore/main.c > @@ -838,7 +838,7 @@ static void wl12xx_read_fwlog_panic(struct wl1271 *wl) > > wl1271_info("Reading FW panic log"); > > - block = kmalloc(wl->fw_mem_block_size, GFP_KERNEL); > + block = kzalloc(wl->fw_mem_block_size, GFP_KERNEL); > if (!block) > return; > > @@ -885,7 +885,6 @@ static void wl12xx_read_fwlog_panic(struct wl1271 *wl) > goto out; > } > > - memset(block, 0, wl->fw_mem_block_size); Current code is reseting the block value to 0 in each iterations and perform some operations. But if you are using kzalloc as mentioned above it will memset 0 to block pointer only once. So both are not equivalent operations. > ret = wlcore_read_hwaddr(wl, addr, block, > wl->fw_mem_block_size, false); > > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -Souptick ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-12-22 7:52 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-21 16:47 [PATCH] wlcore: consolidate kmalloc + memset 0 into kzalloc Nicholas Mc Guire 2015-12-21 22:56 ` Julian Calaby 2015-12-22 7:29 ` Nicholas Mc Guire 2015-12-22 7:52 ` Julian Calaby 2015-12-22 7:06 ` Souptick Joarder
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).