rather than using kmalloc_array + memset it seems cleaner to simply use
kcalloc which will deliver memory set to zero.
Signed-off-by: Nicholas Mc Guire <[email protected]>
---
Patch was compile tested with: x86_64_defconfig
CONFIG_INFINIBAND=m, CONFIG_STAGING=y, CONFIG_STAGING_RDMA=m
Patch is against linux-next (localversion-next is -next-20151214)
drivers/staging/rdma/hfi1/chip.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/rdma/hfi1/chip.c b/drivers/staging/rdma/hfi1/chip.c
index dc69159..31eec8a 100644
--- a/drivers/staging/rdma/hfi1/chip.c
+++ b/drivers/staging/rdma/hfi1/chip.c
@@ -10128,8 +10128,7 @@ static void init_qos(struct hfi1_devdata *dd, u32 first_ctxt)
goto bail;
if (num_vls * qpns_per_vl > dd->chip_rcv_contexts)
goto bail;
- rsmmap = kmalloc_array(NUM_MAP_REGS, sizeof(u64), GFP_KERNEL);
- memset(rsmmap, rxcontext, NUM_MAP_REGS * sizeof(u64));
+ rsmmap = kcalloc(NUM_MAP_REGS, sizeof(u64), GFP_KERNEL);
/* init the local copy of the table */
for (i = 0, ctxt = first_ctxt; i < num_vls; i++) {
unsigned tctxt;
--
1.7.10.4
Add a null check after the kcalloc call as proposed by
Mike Marciniszyn <[email protected]>.
Signed-off-by: Nicholas Mc Guire <[email protected]>
---
Patch was compile tested with: x86_64_defconfig
CONFIG_INFINIBAND=m, CONFIG_STAGING=y, CONFIG_STAGING_RDMA=m
Patch is against linux-next (localversion-next is -next-20151214)
drivers/staging/rdma/hfi1/chip.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/rdma/hfi1/chip.c b/drivers/staging/rdma/hfi1/chip.c
index 31eec8a..52d2bd7 100644
--- a/drivers/staging/rdma/hfi1/chip.c
+++ b/drivers/staging/rdma/hfi1/chip.c
@@ -10129,6 +10129,9 @@ static void init_qos(struct hfi1_devdata *dd, u32 first_ctxt)
if (num_vls * qpns_per_vl > dd->chip_rcv_contexts)
goto bail;
rsmmap = kcalloc(NUM_MAP_REGS, sizeof(u64), GFP_KERNEL);
+ if (!rsmmap)
+ goto bail;
+
/* init the local copy of the table */
for (i = 0, ctxt = first_ctxt; i < num_vls; i++) {
unsigned tctxt;
--
1.7.10.4
fix the following build warning
drivers/staging/rdma/hfi1/chip.c: In function 'init_qos':
drivers/staging/rdma/hfi1/chip.c:10110:6: warning: unused variable
'rxcontext' [-Wunused-variable]
Signed-off-by: Nicholas Mc Guire <[email protected]>
---
Patch was compile tested with: x86_64_defconfig
CONFIG_INFINIBAND=m, CONFIG_STAGING=y, CONFIG_STAGING_RDMA=m
Patch is against linux-next (localversion-next is -next-20151214)
drivers/staging/rdma/hfi1/chip.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/rdma/hfi1/chip.c b/drivers/staging/rdma/hfi1/chip.c
index 52d2bd7..ec368a8 100644
--- a/drivers/staging/rdma/hfi1/chip.c
+++ b/drivers/staging/rdma/hfi1/chip.c
@@ -10107,7 +10107,6 @@ static void init_qos(struct hfi1_devdata *dd, u32 first_ctxt)
unsigned qpns_per_vl, ctxt, i, qpn, n = 1, m;
u64 *rsmmap;
u64 reg;
- u8 rxcontext = is_a0(dd) ? 0 : 0xff; /* 0 is default if a0 ver. */
/* validate */
if (dd->n_krcv_queues <= MIN_KERNEL_KCTXTS ||
--
1.7.10.4
> @@ -10129,6 +10129,9 @@ static void init_qos(struct hfi1_devdata *dd,
> u32 first_ctxt)
> if (num_vls * qpns_per_vl > dd->chip_rcv_contexts)
> goto bail;
> rsmmap = kcalloc(NUM_MAP_REGS, sizeof(u64), GFP_KERNEL);
> + if (!rsmmap)
> + goto bail;
> +
I checked out a linux-next remote at the next-20151214 tag.
The allocation method is clearly kmalloc_array() not kcalloc().
Where are you seeing the kcalloc()?
While it is tempting to allocate and zero, there is a chip rev specific difference.
Mike
> --- a/drivers/staging/rdma/hfi1/chip.c
> +++ b/drivers/staging/rdma/hfi1/chip.c
> @@ -10128,8 +10128,7 @@ static void init_qos(struct hfi1_devdata *dd,
> u32 first_ctxt)
> goto bail;
> if (num_vls * qpns_per_vl > dd->chip_rcv_contexts)
> goto bail;
> - rsmmap = kmalloc_array(NUM_MAP_REGS, sizeof(u64),
> GFP_KERNEL);
> - memset(rsmmap, rxcontext, NUM_MAP_REGS * sizeof(u64));
> + rsmmap = kcalloc(NUM_MAP_REGS, sizeof(u64), GFP_KERNEL);
> /* init the local copy of the table */
> for (i = 0, ctxt = first_ctxt; i < num_vls; i++) {
> unsigned tctxt;
> --
I'm NAKing this.
There is a chip specific difference that accounts for the current code.
Mike
On Mon, Dec 14, 2015 at 03:21:24PM +0000, Marciniszyn, Mike wrote:
> > @@ -10129,6 +10129,9 @@ static void init_qos(struct hfi1_devdata *dd,
> > u32 first_ctxt)
> > if (num_vls * qpns_per_vl > dd->chip_rcv_contexts)
> > goto bail;
> > rsmmap = kcalloc(NUM_MAP_REGS, sizeof(u64), GFP_KERNEL);
> > + if (!rsmmap)
> > + goto bail;
> > +
>
> I checked out a linux-next remote at the next-20151214 tag.
>
> The allocation method is clearly kmalloc_array() not kcalloc().
>
> Where are you seeing the kcalloc()?
>
> While it is tempting to allocate and zero, there is a chip rev specific difference.
>
x = kmalloc_array(...)
if(!x)
...
memset(x...)
should be equivalent to
kcalloc - include/linux/slab.h
static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
{
return kmalloc_array(n, size, flags | __GFP_ZERO);
}
if the assumption that this is equvalent is wrong I appologize
the intent was simply API consolidation as the patch description
stated.
thx!
hofrta
On Mon, Dec 14, 2015 at 03:28:46PM +0000, Marciniszyn, Mike wrote:
> > --- a/drivers/staging/rdma/hfi1/chip.c
> > +++ b/drivers/staging/rdma/hfi1/chip.c
> > @@ -10128,8 +10128,7 @@ static void init_qos(struct hfi1_devdata *dd,
> > u32 first_ctxt)
> > goto bail;
> > if (num_vls * qpns_per_vl > dd->chip_rcv_contexts)
> > goto bail;
> > - rsmmap = kmalloc_array(NUM_MAP_REGS, sizeof(u64),
> > GFP_KERNEL);
> > - memset(rsmmap, rxcontext, NUM_MAP_REGS * sizeof(u64));
> > + rsmmap = kcalloc(NUM_MAP_REGS, sizeof(u64), GFP_KERNEL);
> > /* init the local copy of the table */
> > for (i = 0, ctxt = first_ctxt; i < num_vls; i++) {
> > unsigned tctxt;
> > --
>
> I'm NAKing this.
>
> There is a chip specific difference that accounts for the current code.
>
I obviously made a real mess here.
I incorrectly concluded that rxcontext is 0 which it is not in some cases
sorry for the noise.
thx!
hofrat
On Mon, Dec 14, 2015 at 05:41:23PM +0000, Nicholas Mc Guire wrote:
> I obviously made a real mess here.
> I incorrectly concluded that rxcontext is 0 which it is not in some cases
Yep. Plus you build tested it but assumed that the unused variable
warning must have been there in the original... I've done that for
static checker warnings. Lesson learned, hopefully.
regards,
dan carpenter