From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751542AbdLNABL (ORCPT ); Wed, 13 Dec 2017 19:01:11 -0500 Received: from mx2.suse.de ([195.135.220.15]:40626 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750785AbdLNABK (ORCPT ); Wed, 13 Dec 2017 19:01:10 -0500 From: NeilBrown To: Oleg Drokin , James Simmons , Andreas Dilger , Greg Kroah-Hartman Date: Thu, 14 Dec 2017 11:00:45 +1100 Cc: linux-kernel@vger.kernel.org, lustre-devel@lists.lustre.org Subject: Re: [PATCH 07/12] staging: lustre: libcfs: simplify memory allocation. In-Reply-To: <151313495497.27582.5451400666802627086.stgit@noble> References: <151313493380.27582.16490205348451477393.stgit@noble> <151313495497.27582.5451400666802627086.stgit@noble> Message-ID: <87d13inpoy.fsf@notabene.neil.brown.name> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable On Wed, Dec 13 2017, NeilBrown wrote: > 1/ Use kvmalloc() instead of kmalloc or vmalloc > 2/ Discard the _GFP() interfaces that are never used. > We only ever do GFP_NOFS and GFP_ATOMIC allocations, > so support each of those explicitly. Hi, I just remembered that posting this patch was, maybe, a little premature. vmalloc() doesn't support GFP_NOFS, so kvmalloc() warns about an attempt to use GFP_NOFS. lustre potentially used vmalloc in GFP_NOFS context, and so now calls kvmalloc() with GFP_NOFS, which triggers a warning. I haven't yet looking into how to remove the warning. Maybe all GFP_NOFS usages should use kmalloc(), not kvmalloc(), and accept the increased chance of failure. I'll dig deeper and hope to have a clearer opinion soon. As it is only a warning, it is probably OK to keep the patch in staging-next, but I wouldn't object if it was dropped. Thanks, NeilBrown =20 > > Signed-off-by: NeilBrown > --- > .../lustre/include/linux/libcfs/libcfs_private.h | 42 ++++++--------= ------ > 1 file changed, 12 insertions(+), 30 deletions(-) > > diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h= b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h > index 2f4ff595fac9..c874f9d15c72 100644 > --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h > +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h > @@ -84,14 +84,6 @@ do { \ > lbug_with_loc(&msgdata); \ > } while (0) >=20=20 > -#ifndef LIBCFS_VMALLOC_SIZE > -#define LIBCFS_VMALLOC_SIZE (2 << PAGE_SHIFT) /* 2 pages */ > -#endif > - > -#define LIBCFS_ALLOC_PRE(size, mask) \ > - LASSERT(!in_interrupt() || ((size) <=3D LIBCFS_VMALLOC_SIZE && \ > - !gfpflags_allow_blocking(mask))) > - > #define LIBCFS_ALLOC_POST(ptr, size) \ > do { \ > if (unlikely(!(ptr))) { \ > @@ -103,46 +95,36 @@ do { \ > } while (0) >=20=20 > /** > - * allocate memory with GFP flags @mask > + * default allocator > */ > -#define LIBCFS_ALLOC_GFP(ptr, size, mask) \ > +#define LIBCFS_ALLOC(ptr, size) \ > do { \ > - LIBCFS_ALLOC_PRE((size), (mask)); \ > - (ptr) =3D (size) <=3D LIBCFS_VMALLOC_SIZE ? \ > - kmalloc((size), (mask)) : vmalloc(size); \ > + LASSERT(!in_interrupt()); \ > + (ptr) =3D kvmalloc((size), GFP_NOFS); \ > LIBCFS_ALLOC_POST((ptr), (size)); \ > } while (0) >=20=20 > -/** > - * default allocator > - */ > -#define LIBCFS_ALLOC(ptr, size) \ > - LIBCFS_ALLOC_GFP(ptr, size, GFP_NOFS) > - > /** > * non-sleeping allocator > */ > -#define LIBCFS_ALLOC_ATOMIC(ptr, size) \ > - LIBCFS_ALLOC_GFP(ptr, size, GFP_ATOMIC) > +#define LIBCFS_ALLOC_ATOMIC(ptr, size) \ > +do { \ > + (ptr) =3D kmalloc((size), GFP_ATOMIC); \ > + LIBCFS_ALLOC_POST(ptr, size); \ > +} while (0) >=20=20 > /** > * allocate memory for specified CPU partition > * \a cptab !=3D NULL, \a cpt is CPU partition id of \a cptab > * \a cptab =3D=3D NULL, \a cpt is HW NUMA node id > */ > -#define LIBCFS_CPT_ALLOC_GFP(ptr, cptab, cpt, size, mask) \ > +#define LIBCFS_CPT_ALLOC(ptr, cptab, cpt, size) \ > do { \ > - LIBCFS_ALLOC_PRE((size), (mask)); \ > - (ptr) =3D (size) <=3D LIBCFS_VMALLOC_SIZE ? \ > - kmalloc_node((size), (mask), cfs_cpt_spread_node(cptab, cpt)) :\ > - vmalloc_node(size, cfs_cpt_spread_node(cptab, cpt)); \ > + LASSERT(!in_interrupt()); \ > + (ptr) =3D kvmalloc_node((size), GFP_NOFS, cfs_cpt_spread_node(cptab, cp= t)); \ > LIBCFS_ALLOC_POST((ptr), (size)); \ > } while (0) >=20=20 > -/** default numa allocator */ > -#define LIBCFS_CPT_ALLOC(ptr, cptab, cpt, size) \ > - LIBCFS_CPT_ALLOC_GFP(ptr, cptab, cpt, size, GFP_NOFS) > - > #define LIBCFS_FREE(ptr, size) \ > do { \ > if (unlikely(!(ptr))) { \ --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEG8Yp69OQ2HB7X0l6Oeye3VZigbkFAloxvzMACgkQOeye3VZi gbk+AQ//UTdjJ/OfMHbXf2Xf/MTuPnXHjYhUGPbMoOmtMtOykeTwlz1RxwUU56nN AxB2+GEMtOMLkAsBV7oUuNnvBiCRmN0x93kPyg2G2Y5lcKDucFjJxA4T+Kbxz7KG vMT7U4CUG49NIvLTeez9+fNbKquQpOZMXxGSy4jCa4DvZCyz3GeNWKT/Ospf8i1s Ac5A1XU5Ka2A0Cpx8zgCZGzDkcFEtk5UWAdpGQl/zBugph9dnSeqkD3ybH/G7Gnr dgO/KMUiatXKvPc3aqZCISxZeVOn2pqb7x82RJRfTOCGarZ3ra6YV/Ve8+JBkyZv OiQilg36/4ZZVZAoVtlBKQXEA8EC63cR34K3XMOMWcp91+ZQz3fqSSOXWx+Jo/TB ByisFk+yw2OWz9AGWsaIffkNBEkqRVPgFBiH1gxZCT3Fdj80Mu5jwU3e7t9klvuM ercIftONEh5B3tmS973K/0DyN4lR/npuyiSe4meLqeCWNVrpPvFOyUejtUXzTUdZ t/HKWsXwgV43OY8Jbbufl9/QnERAPFmpip7/5pfC76QSlfVVudp2HCltfgTo2B3r dN3GLWKRwFm6t3N7QaDF+lwLzjQfupOQWOcnDzbBdoVujRmZR4/I5jonOFWu7Ogd zctxaumR/o3myzY+2r6FAz6fz6lHzxPav7kFTL6oYiaX8h7NJuc= =bYQC -----END PGP SIGNATURE----- --=-=-=--