[PATCH] dm: reduce the number of processes per dm device

[PATCH] dm: reduce the number of processes per dm device

[Mikulas Patocka]

The patch 54efd50bfd873e2dbf784e0b21a8027ba4299a3e ("block: make
generic_make_request handle arbitrarily sized bios") makes it possible for
block devices to process large bios. The patch allocates a new bio set
queue->bio_split for each device, this bio set is used for allocating bios
when the driver needs to split large bios.

Each bio_set allocates a workqueue process, thus the above patch increases
the number of processes allocated per block device.

Device mapper doesn't need the queue->bio_split bio_set, thus we can
deallocate it. This reduces the number of allocated processes per
dm-device from 3 to 2.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux-4.3-rc4/drivers/md/dm.c
===================================================================
--- linux-4.3-rc4.orig/drivers/md/dm.c	2015-10-08 16:23:09.000000000 +0200
+++ linux-4.3-rc4/drivers/md/dm.c	2015-10-08 16:30:40.000000000 +0200
@@ -2839,6 +2839,12 @@ int dm_setup_md_queue(struct mapped_devi
 	case DM_TYPE_BIO_BASED:
 		dm_init_old_md_queue(md);
 		blk_queue_make_request(md->queue, dm_make_request);
+		/*
+		 * We don't need the bioset, so we free it to save one process
+		 * per device.
+		 */
+		bioset_free(md->queue->bio_split);
+		md->queue->bio_split = NULL;
 		break;
 	}
 

Re: dm: reduce the number of processes per dm device

[Mike Snitzer]

On Thu, Oct 08 2015 at 12:15pm -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> The patch 54efd50bfd873e2dbf784e0b21a8027ba4299a3e ("block: make
> generic_make_request handle arbitrarily sized bios") makes it possible for
> block devices to process large bios. The patch allocates a new bio set
> queue->bio_split for each device, this bio set is used for allocating bios
> when the driver needs to split large bios.
> 
> Each bio_set allocates a workqueue process, thus the above patch increases
> the number of processes allocated per block device.
> 
> Device mapper doesn't need the queue->bio_split bio_set, thus we can
> deallocate it. This reduces the number of allocated processes per
> dm-device from 3 to 2.

This header needs more context added, specifically we need to tell the
reader the answer to: why doesn't DM need queue->bio_split?

Is this a resource that only the lowest layer's request_queue would
need?  And given DM's stacking nature it doesn't need it simply because
it'll never be the lowest layer?

Re: dm: reduce the number of processes per dm device

[Mikulas Patocka]

On Thu, 8 Oct 2015, Mike Snitzer wrote:

> On Thu, Oct 08 2015 at 12:15pm -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > The patch 54efd50bfd873e2dbf784e0b21a8027ba4299a3e ("block: make
> > generic_make_request handle arbitrarily sized bios") makes it possible for
> > block devices to process large bios. The patch allocates a new bio set
> > queue->bio_split for each device, this bio set is used for allocating bios
> > when the driver needs to split large bios.
> > 
> > Each bio_set allocates a workqueue process, thus the above patch increases
> > the number of processes allocated per block device.
> > 
> > Device mapper doesn't need the queue->bio_split bio_set, thus we can
> > deallocate it. This reduces the number of allocated processes per
> > dm-device from 3 to 2.
> 
> This header needs more context added, specifically we need to tell the
> reader the answer to: why doesn't DM need queue->bio_split?

Dm doesn't need queue->bio_split because it has its own bioset md->bs. We 
can't use queue->bio_split instead of md->bs because md->bs has non-zero 
front pad depending on targets loaded in the table.

> Is this a resource that only the lowest layer's request_queue would 
> need? And given DM's stacking nature it doesn't need it simply because 
> it'll never be the lowest layer?

All request-based drivers need queue->bio_split, but some non-dm bio-based 
drivers need it too.

I thought about removing bio_split for for all bio-based devices, but 
declined that because it could generate regressions in drivers that we are 
not able to test.

Mikulas

Re: dm: reduce the number of processes per dm device

[Mike Snitzer]

On Thu, Oct 08 2015 at  3:59pm -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> 
> 
> On Thu, 8 Oct 2015, Mike Snitzer wrote:
> 
> > On Thu, Oct 08 2015 at 12:15pm -0400,
> > Mikulas Patocka <mpatocka@redhat.com> wrote:
> > 
> > > The patch 54efd50bfd873e2dbf784e0b21a8027ba4299a3e ("block: make
> > > generic_make_request handle arbitrarily sized bios") makes it possible for
> > > block devices to process large bios. The patch allocates a new bio set
> > > queue->bio_split for each device, this bio set is used for allocating bios
> > > when the driver needs to split large bios.
> > > 
> > > Each bio_set allocates a workqueue process, thus the above patch increases
> > > the number of processes allocated per block device.
> > > 
> > > Device mapper doesn't need the queue->bio_split bio_set, thus we can
> > > deallocate it. This reduces the number of allocated processes per
> > > dm-device from 3 to 2.
> > 
> > This header needs more context added, specifically we need to tell the
> > reader the answer to: why doesn't DM need queue->bio_split?
> 
> Dm doesn't need queue->bio_split because it has its own bioset md->bs. We 
> can't use queue->bio_split instead of md->bs because md->bs has non-zero 
> front pad depending on targets loaded in the table.

Sure but my point was that bio-based DM targets by definition aren't the
last device in a stack.
 
> > Is this a resource that only the lowest layer's request_queue would 
> > need? And given DM's stacking nature it doesn't need it simply because 
> > it'll never be the lowest layer?
> 
> All request-based drivers need queue->bio_split, but some non-dm bio-based 
> drivers need it too.

Not all request-based drivers right?  request-based DM (aka DM mpath)
shouldn't _need_ queue->bio_split either right?

Re: dm: reduce the number of processes per dm device

[Mikulas Patocka]

On Thu, 8 Oct 2015, Mike Snitzer wrote:

> On Thu, Oct 08 2015 at  3:59pm -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > 
> > 
> > On Thu, 8 Oct 2015, Mike Snitzer wrote:
> > 
> > > On Thu, Oct 08 2015 at 12:15pm -0400,
> > > Mikulas Patocka <mpatocka@redhat.com> wrote:
> > > 
> > > > The patch 54efd50bfd873e2dbf784e0b21a8027ba4299a3e ("block: make
> > > > generic_make_request handle arbitrarily sized bios") makes it possible for
> > > > block devices to process large bios. The patch allocates a new bio set
> > > > queue->bio_split for each device, this bio set is used for allocating bios
> > > > when the driver needs to split large bios.
> > > > 
> > > > Each bio_set allocates a workqueue process, thus the above patch increases
> > > > the number of processes allocated per block device.
> > > > 
> > > > Device mapper doesn't need the queue->bio_split bio_set, thus we can
> > > > deallocate it. This reduces the number of allocated processes per
> > > > dm-device from 3 to 2.
> > > 
> > > This header needs more context added, specifically we need to tell the
> > > reader the answer to: why doesn't DM need queue->bio_split?
> > 
> > Dm doesn't need queue->bio_split because it has its own bioset md->bs. We 
> > can't use queue->bio_split instead of md->bs because md->bs has non-zero 
> > front pad depending on targets loaded in the table.
> 
> Sure but my point was that bio-based DM targets by definition aren't the
> last device in a stack.
>  
> > > Is this a resource that only the lowest layer's request_queue would 
> > > need? And given DM's stacking nature it doesn't need it simply because 
> > > it'll never be the lowest layer?
> > 
> > All request-based drivers need queue->bio_split, but some non-dm bio-based 
> > drivers need it too.
> 
> Not all request-based drivers right?  request-based DM (aka DM mpath)
> shouldn't _need_ queue->bio_split either right?

It uses queue->bio_split because it is used at the begining of 
blk_queue_bio. All request-based drivers use blk_queue_bio.

Mikulas

[PATCH v2] dm: reduce the number of processes per dm device

[Mikulas Patocka]

This is updated patch to reduce the number of processes. When we free 
q->bio_split bioset, we must also avoid the call to blk_queue_split that 
uses it. The call to blk_queue_split is useless because device mapper does 
its own splitting.


From: Mikulas Patocka <mpatocka@redhat.com>

The patch 54efd50bfd873e2dbf784e0b21a8027ba4299a3e ("block: make
generic_make_request handle arbitrarily sized bios") makes it possible for
block devices to process large bios. The patch allocates a new bio set
queue->bio_split for each device, this bio set is used for allocating bios
when the driver needs to split large bios.

Each bio_set allocates a workqueue process, thus the above patch increases
the number of processes allocated per block device.

Device mapper doesn't need the queue->bio_split bio_set, thus we can
deallocate it. This reduces the number of allocated processes per
dm-device from 3 to 2. We remove the call to blk_queue_split, it is not 
needed because device mapper does its own splitting.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Index: linux-4.3-rc6/drivers/md/dm.c
===================================================================
--- linux-4.3-rc6.orig/drivers/md/dm.c	2015-10-21 19:29:04.000000000 +0200
+++ linux-4.3-rc6/drivers/md/dm.c	2015-10-21 19:29:58.000000000 +0200
@@ -1742,8 +1742,6 @@ static void dm_make_request(struct reque
 
 	map = dm_get_live_table(md, &srcu_idx);
 
-	blk_queue_split(q, &bio, q->bio_split);
-
 	generic_start_io_acct(rw, bio_sectors(bio), &dm_disk(md)->part0);
 
 	/* if we're suspended, we have to queue this io for later */
@@ -2770,6 +2768,12 @@ int dm_setup_md_queue(struct mapped_devi
 	case DM_TYPE_BIO_BASED:
 		dm_init_old_md_queue(md);
 		blk_queue_make_request(md->queue, dm_make_request);
+		/*
+		 * We don't need the bioset, so we free it to save one process
+		 * per device.
+		 */
+		bioset_free(md->queue->bio_split);
+		md->queue->bio_split = NULL;
 		break;
 	}