linux-kernel - Re: [PATCH v3 07/10] lib/memregion: Uplevel the pmem "region" ida to a global allocator

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 26 Aug 2019 22:48:50 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Keith Busch <keith.busch@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Vishal L Verma <vishal.l.verma@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        linux-nvdimm <linux-nvdimm@...ts.01.org>,
        X86 ML <x86@...nel.org>, linux-efi <linux-efi@...r.kernel.org>
Subject: Re: [PATCH v3 07/10] lib/memregion: Uplevel the pmem "region" ida to
 a global allocator

On Fri, Jun 7, 2019 at 1:23 PM Matthew Wilcox <willy@...radead.org> wrote:
>
> On Fri, Jun 07, 2019 at 12:27:50PM -0700, Dan Williams wrote:
> > diff --git a/lib/memregion.c b/lib/memregion.c
> > new file mode 100644
> > index 000000000000..f6c6a94c7921
> > --- /dev/null
> > +++ b/lib/memregion.c
> > @@ -0,0 +1,15 @@
> > +#include <linux/idr.h>
> > +
> > +static DEFINE_IDA(region_ids);
> > +
> > +int memregion_alloc(gfp_t gfp)
> > +{
> > +     return ida_alloc(&region_ids, gfp);
> > +}
> > +EXPORT_SYMBOL(memregion_alloc);
> > +
> > +void memregion_free(int id)
> > +{
> > +     ida_free(&region_ids, id);
> > +}
> > +EXPORT_SYMBOL(memregion_free);
>
> Does this trivial abstraction have to live in its own file?  I'd make
> memregion_alloc/free static inlines that live in a header file, then
> all you need do is find a suitable .c file to store memregion_ids in,
> and export that one symbol instead of two.

It turns out yes, this needs to live in its own file. It's an optional
library that does not fit anywhere else, everywhere I've thought to
stash it has either triggered obscure build errors based on idr.h
include dependencies or does not fit due to build dependencies.