A patchset to limit the number of outstanding bios

A patchset to limit the number of outstanding bios

[Mikulas Patocka]

Hi

I'm announcing the release of a patchset that limits the number of 
outstanding bios in the device mapper.

The patchset is at 
http://people.redhat.com/~mpatocka/patches/kernel/dm-limit-outstanding-bios/

The default limit is 1024, the minimum limit is 4 (but I verified that the 
testsuite works even with limit 1). The limit can be set in 
/sys/block/dm-*/queue/nr_requests

The limit for newly created dm devices can be set in 
/sys/module/dm_mod/parameters/default_nr_requests (for example, you can 
set it before running the testsuite or some benchmark).

The patchset could possibly fix bug 1010466.

Note that the patch "punt-to-workqueue.patch" is required, otherwise we 
get lockups in the testsuite at low nr_requests values. The patch 
"fix-counting.patch" isn't required, but it is included to avoid patch 
conflicts.

Mikulas

Re: A patchset to limit the number of outstanding bios

[Joe Thornber]

On Mon, Oct 13, 2014 at 11:27:20AM -0400, Mikulas Patocka wrote:
> Hi
> 
> I'm announcing the release of a patchset that limits the number of 
> outstanding bios in the device mapper.

Interesting; Mike and I were considering this for throttling io to
thinp a couple of weeks ago.  The reason we didn't go with this was,
for thinp, there are cheap bios (eg, a quick remap) and expensive bios
(eg, one that triggers a copy on write exception).  This means it's
very hard to come up with a single number that represents too many
bios.  Instead I throttle by looking at how long it's taking the
worker thread to get through it's work queue.

Could you make this limit optional please?  eg, setting the limit to 0
turns it off, and I'd prefer a default of 0/off.

- Joe

Re: A patchset to limit the number of outstanding bios

[Mike Snitzer]

On Mon, Oct 13 2014 at 11:27am -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> Hi
> 
> I'm announcing the release of a patchset that limits the number of 
> outstanding bios in the device mapper.
> 
> The patchset is at 
> http://people.redhat.com/~mpatocka/patches/kernel/dm-limit-outstanding-bios/

You should probably just send the patchset to the list (and cc Jens on
the block changes).

Patch 1 looks good as is.  But Jens suggested that Patch 2 should hook
off the on stack plugging rather than adding yet another hook to
kernel/sched/core.c

And I agree with Joe, we want this DM-only limit to be opt-in per-target
(so disabled by default).

Mike

Re: A patchset to limit the number of outstanding bios

[Mikulas Patocka]

On Mon, 13 Oct 2014, Mike Snitzer wrote:

> On Mon, Oct 13 2014 at 11:27am -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > Hi
> > 
> > I'm announcing the release of a patchset that limits the number of 
> > outstanding bios in the device mapper.
> > 
> > The patchset is at 
> > http://people.redhat.com/~mpatocka/patches/kernel/dm-limit-outstanding-bios/
> 
> You should probably just send the patchset to the list (and cc Jens on
> the block changes).
> 
> Patch 1 looks good as is.  But Jens suggested that Patch 2 should hook
> off the on stack plugging rather than adding yet another hook to
> kernel/sched/core.c

I think that adding some generic schedule callbacks because of *two* hooks 
is overengineering. If there were five schedule hooks, it would be 
justified to generalize that.

> And I agree with Joe, we want this DM-only limit to be opt-in per-target
> (so disabled by default).

... if it were opt-in, people will still get out of memory dm-crypt 
crashes (like bug 1010466) or tasks stuck in "D" state for too long time, 
because they don't know that they need to this knob to avoid this problem.

I'd select the default value high enough so that it doesn't degrade any 
benchmarks.

Mikulas

> Mike

device-mapper

[lilofile]

what does btree_insert_raw function means?   how can I understand dm-btree.c, any infomation will be helpful.

dm-thin

[lilofile]

In dm-thin.c,
/*
 * The block size of the device holding pool data must be
 * between 64KB and 1GB.
 */
#define DATA_DEV_BLOCK_SIZE_MIN_SECTORS (64 * 1024 >> SECTOR_SHIFT)
#define DATA_DEV_BLOCK_SIZE_MAX_SECTORS (1024 * 1024 * 1024 >> SECTOR_SHIFT)

why the block size must be between 64KB and 1GB, can I change  the value of DATA_DEV_BLOCK_SIZE_MIN_SECTORS? what effect  if the value of DATA_DEV_BLOCK_SIZE_MIN_SECTORS<128?

Re: dm-thin

[Zdenek Kabelac]

Dne 21.10.2014 v 03:36 lilofile napsal(a):
> In dm-thin.c,
> /*
>   * The block size of the device holding pool data must be
>   * between 64KB and 1GB.
>   */
> #define DATA_DEV_BLOCK_SIZE_MIN_SECTORS (64 * 1024 >> SECTOR_SHIFT)
> #define DATA_DEV_BLOCK_SIZE_MAX_SECTORS (1024 * 1024 * 1024 >> SECTOR_SHIFT)
>
> why the block size must be between 64KB and 1GB, can I change  the value of DATA_DEV_BLOCK_SIZE_MIN_SECTORS? what effect  if the value of DATA_DEV_BLOCK_SIZE_MIN_SECTORS<128?


Performance reasons mainly - updating metadata much more frequently is 
lowering the throughput considerable.

The other issue is you would also scale up required metadata size.

Zdenek