| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 21 Nov 2016 20:20:30 +0000
From: Salil Mehta <salil.mehta@...wei.com>
To: Leon Romanovsky <leon@...nel.org>
CC: "dledford@...hat.com" <dledford@...hat.com>,
"Huwei (Xavier)" <xavier.huwei@...wei.com>,
oulijun <oulijun@...wei.com>,
"mehta.salil.lnk@...il.com" <mehta.salil.lnk@...il.com>,
"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Linuxarm <linuxarm@...wei.com>,
"Zhangping (ZP)" <zhangping5@...wei.com>
Subject: RE: [PATCH for-next 03/11] IB/hns: Optimize the logic of allocating
memory using APIs
> -----Original Message-----
> From: netdev-owner@...r.kernel.org [mailto:netdev-
> owner@...r.kernel.org] On Behalf Of Leon Romanovsky
> Sent: Monday, November 21, 2016 5:14 PM
> To: Salil Mehta
> Cc: dledford@...hat.com; Huwei (Xavier); oulijun;
> mehta.salil.lnk@...il.com; linux-rdma@...r.kernel.org;
> netdev@...r.kernel.org; linux-kernel@...r.kernel.org; Linuxarm;
> Zhangping (ZP)
> Subject: Re: [PATCH for-next 03/11] IB/hns: Optimize the logic of
> allocating memory using APIs
>
> On Mon, Nov 21, 2016 at 04:12:38PM +0000, Salil Mehta wrote:
> > > -----Original Message-----
> > > From: Leon Romanovsky [mailto:leon@...nel.org]
> > > Sent: Wednesday, November 16, 2016 8:36 AM
> > > To: Salil Mehta
> > > Cc: dledford@...hat.com; Huwei (Xavier); oulijun;
> > > mehta.salil.lnk@...il.com; linux-rdma@...r.kernel.org;
> > > netdev@...r.kernel.org; linux-kernel@...r.kernel.org; Linuxarm;
> > > Zhangping (ZP)
> > > Subject: Re: [PATCH for-next 03/11] IB/hns: Optimize the logic of
> > > allocating memory using APIs
> > >
> > > On Tue, Nov 15, 2016 at 03:52:46PM +0000, Salil Mehta wrote:
> > > > > -----Original Message-----
> > > > > From: Leon Romanovsky [mailto:leon@...nel.org]
> > > > > Sent: Wednesday, November 09, 2016 7:22 AM
> > > > > To: Salil Mehta
> > > > > Cc: dledford@...hat.com; Huwei (Xavier); oulijun;
> > > > > mehta.salil.lnk@...il.com; linux-rdma@...r.kernel.org;
> > > > > netdev@...r.kernel.org; linux-kernel@...r.kernel.org; Linuxarm;
> > > > > Zhangping (ZP)
> > > > > Subject: Re: [PATCH for-next 03/11] IB/hns: Optimize the logic
> of
> > > > > allocating memory using APIs
> > > > >
> > > > > On Fri, Nov 04, 2016 at 04:36:25PM +0000, Salil Mehta wrote:
> > > > > > From: "Wei Hu (Xavier)" <xavier.huwei@...wei.com>
> > > > > >
> > > > > > This patch modified the logic of allocating memory using APIs
> in
> > > > > > hns RoCE driver. We used kcalloc instead of kmalloc_array and
> > > > > > bitmap_zero. And When kcalloc failed, call vzalloc to alloc
> > > > > > memory.
> > > > > >
> > > > > > Signed-off-by: Wei Hu (Xavier) <xavier.huwei@...wei.com>
> > > > > > Signed-off-by: Ping Zhang <zhangping5@...wei.com>
> > > > > > Signed-off-by: Salil Mehta <salil.mehta@...wei.com>
> > > > > > ---
> > > > > > drivers/infiniband/hw/hns/hns_roce_mr.c | 15 ++++++++-----
> --
> > > > > > 1 file changed, 8 insertions(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/infiniband/hw/hns/hns_roce_mr.c
> > > > > b/drivers/infiniband/hw/hns/hns_roce_mr.c
> > > > > > index fb87883..d3dfb5f 100644
> > > > > > --- a/drivers/infiniband/hw/hns/hns_roce_mr.c
> > > > > > +++ b/drivers/infiniband/hw/hns/hns_roce_mr.c
> > > > > > @@ -137,11 +137,12 @@ static int hns_roce_buddy_init(struct
> > > > > hns_roce_buddy *buddy, int max_order)
> > > > > >
> > > > > > for (i = 0; i <= buddy->max_order; ++i) {
> > > > > > s = BITS_TO_LONGS(1 << (buddy->max_order - i));
> > > > > > - buddy->bits[i] = kmalloc_array(s, sizeof(long),
> > > > > GFP_KERNEL);
> > > > > > - if (!buddy->bits[i])
> > > > > > - goto err_out_free;
> > > > > > -
> > > > > > - bitmap_zero(buddy->bits[i], 1 << (buddy->max_order -
> > > i));
> > > > > > + buddy->bits[i] = kcalloc(s, sizeof(long),
> > > GFP_KERNEL);
> > > > > > + if (!buddy->bits[i]) {
> > > > > > + buddy->bits[i] = vzalloc(s * sizeof(long));
> > > > >
> > > > > I wonder, why don't you use directly vzalloc instead of kcalloc
> > > > > fallback?
> > > > As we know we will have physical contiguous pages if the kcalloc
> > > > call succeeds. This will give us a chance to have better
> performance
> > > > over the allocations which are just virtually contiguous through
> the
> > > > function vzalloc(). Therefore, later has only been used as a
> fallback
> > > > when our memory request cannot be entertained through kcalloc.
> > > >
> > > > Are you suggesting that there will not be much performance
> penalty
> > > > if we use just vzalloc ?
> > >
> > > Not exactly,
> > > I asked it, because we have similar code in our drivers and this
> > > construction looks strange to me.
> > >
> > > 1. If performance is critical, we will use kmalloc.
> > > 2. If performance is not critical, we will use vmalloc.
> > >
> > > But in this case, such construction shows me that we can live with
> > > vmalloc performance and kmalloc allocation are not really needed.
> > >
> > > In your specific case, I'm not sure that kcalloc will ever fail.
> > Performance is definitely critical here. Though, I agree this is bit
> > unusual way of memory allocation. In actual, we were encountering
> > memory alloc failures using kmalloc (if you see allocation amount
> > is on the higher side and is exponential) so we ended up using
> > vmalloc as fall back - It is very naïve allocation scheme.
>
> I understand it, we did the same, see our mlx5_vzalloc call.
> BTW, we used __GFP_NOWARN flag, which you should consider to use
> in your case too.
Ok. Will add this flag and refloat patch V3.
Thanks
>
> >
> > Maybe we need to rethink this allocation scheme part? Also, I can
> pull
> > back this particular patch for now or just live with vzalloc() till
> > we figure out proper solution to this?
>
> It is up to you, I don't think that you should drop it, AFAIK, there is
> no other proper solution.
Ok we will live with it for now and later maybe we can see how we can optimize
pre-allocation of physically contiguous memory.
Thanks for your suggestions!
Salil
>
> >
> > >
> > > Thanks
> > >
> > >
> > > >
> > > > >
> > > > > > + if (!buddy->bits[i])
> > > > > > + goto err_out_free;
> > > > > > + }
> > > > > > }
Powered by blists - more mailing lists