19 Sep, 2016

1 commit

  • Variable weight is not being initialized to zero before it is
    used to compute the weight sum. Ensure it is initialized to zero.

    Found with static analysis with cppcheck:
    [lib/sbitmap.c:177]: (error) Uninitialized variable: weight

    Signed-off-by: Colin Ian King
    Signed-off-by: Jens Axboe

    Colin Ian King
     

18 Sep, 2016

1 commit

  • If we have a bunch of high-numbered bits allocated and then we resize
    the struct sbitmap_queue, when those bits get cleared, we'll update the
    hint and then have to re-randomize it repeatedly. Avoid that by checking
    that the cleared bit is still a valid hint. No measurable performance
    difference in the common case.

    Signed-off-by: Omar Sandoval
    Signed-off-by: Jens Axboe

    Omar Sandoval
     

17 Sep, 2016

6 commits

  • After a struct sbitmap_queue is resized smaller, the allocation hints
    may still be set to bits beyond the new depth of the bitmap. This means
    that, for example, if the number of blk-mq tags is reduced through
    sysfs, more requests than the nominal queue depth may be in flight.

    It's tempting to fix this at resize time by doing a one-time
    reinitialization of the hints, but this can race with
    __sbitmap_queue_get() updating the hint. Instead, check the hint before
    we use it. This caused no measurable performance difference in my
    synthetic benchmarks.

    Signed-off-by: Omar Sandoval
    Signed-off-by: Jens Axboe

    Omar Sandoval
     
  • In order to get good cache behavior from a sbitmap, we want each CPU to
    stick to its own cacheline(s) as much as possible. This might happen
    naturally as the bitmap gets filled up and the alloc_hint values spread
    out, but we really want this behavior from the start. blk-mq apparently
    intended to do this, but the code to do this was never wired up. Get rid
    of the dead code and make it part of the sbitmap library.

    Signed-off-by: Omar Sandoval
    Signed-off-by: Jens Axboe

    Omar Sandoval
     
  • Again, there's no point in passing this in every time. Make it part of
    struct sbitmap_queue and clean up the API.

    Signed-off-by: Omar Sandoval
    Signed-off-by: Jens Axboe

    Omar Sandoval
     
  • Allocating your own per-cpu allocation hint separately makes for an
    awkward API. Instead, allocate the per-cpu hint as part of the struct
    sbitmap_queue. There's no point for a struct sbitmap_queue without the
    cache, but you can still use a bare struct sbitmap.

    Signed-off-by: Omar Sandoval
    Signed-off-by: Jens Axboe

    Omar Sandoval
     
  • The original bt_alloc() we converted from was using kzalloc(), not
    kzalloc_node(), to allocate the wait queues. This was probably an
    oversight, so fix it for sbitmap_queue_init_node().

    Signed-off-by: Omar Sandoval
    Signed-off-by: Jens Axboe

    Omar Sandoval
     
  • 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_SBITMAP, which is only
    selected by CONFIG_BLOCK for now.

    This should be a complete noop functionality-wise.

    Signed-off-by: Omar Sandoval
    Signed-off-by: Jens Axboe

    Omar Sandoval