linux-kernel - Re: [PATCH v2 1/5] blk-mq: abstract tag allocation out into scale

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 7 Sep 2016 17:38:59 -0700
From:   Omar Sandoval <osandov@...ndov.com>
To:     Alexei Starovoitov <ast@...com>
Cc:     Jens Axboe <axboe@...com>, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, kernel-team@...com
Subject: Re: [PATCH v2 1/5] blk-mq: abstract tag allocation out into
 scale_bitmap library

On Wed, Sep 07, 2016 at 05:01:56PM -0700, Alexei Starovoitov wrote:
> On 9/7/16 4:46 PM, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@...com>
> > 
> > This is a generally useful data structure, so make it available to
> > anyone else who might want to use it. It's also a nice cleanup
> > separating the allocation logic from the rest of the tag handling logic.
> > 
> > The code is behind a new Kconfig option, CONFIG_SCALE_BITMAP, which is
> > only selected by CONFIG_BLOCK for now.
> > 
> > This should be a complete noop functionality-wise.
> > 
> > Signed-off-by: Omar Sandoval <osandov@...com>
> > ---
> >   MAINTAINERS                  |   1 +
> >   block/Kconfig                |   1 +
> >   block/blk-mq-tag.c           | 469 ++++++++++---------------------------------
> >   block/blk-mq-tag.h           |  37 +---
> >   block/blk-mq.c               | 113 +++--------
> >   block/blk-mq.h               |   9 -
> >   include/linux/blk-mq.h       |   9 +-
> >   include/linux/scale_bitmap.h | 340 +++++++++++++++++++++++++++++++
> >   lib/Kconfig                  |   3 +
> >   lib/Makefile                 |   2 +
> >   lib/scale_bitmap.c           | 305 ++++++++++++++++++++++++++++
> ...
> > diff --git a/include/linux/scale_bitmap.h b/include/linux/scale_bitmap.h
> > new file mode 100644
> > index 0000000..63f712b
> > --- /dev/null
> > +++ b/include/linux/scale_bitmap.h
> > @@ -0,0 +1,340 @@
> > +/*
> > + * Fast and scalable bitmaps.
> ...
> > +/**
> > + * struct scale_bitmap_word - Word in a &struct scale_bitmap.
> > + */
> > +struct scale_bitmap_word {
> > +/**
> > + * struct scale_bitmap - Scalable bitmap.
> > + *
> > + * A &struct scale_bitmap is spread over multiple cachelines to avoid ping-pong.
> > + * This trades off higher memory usage for better scalability.
> > + */
> > +struct scale_bitmap {
> 
> scale_bitmap sounds odd, since 'scale' is also a verb.
> We also have lib/rhashtable.c:
>  * Resizable, Scalable, Concurrent Hash Table
> everything is 'scalable' nowadays.

Agreed, I'm not a huge fan of the name.

> May be resizable bitmap would be a better name?
> 'struct rbitmap'... lib/rbitmap.c ?
> 

Hm, the resizing operation isn't very well thought-out right now, it's
there because it's okay for the way blk-mq uses it, but it's definitely
not the point of the data structure. It's more of a cache-friendly
bitmap, or a sparse bitmap. `struct sbitmap`? `struct cbitmap`?

-- 
Omar