Re: [dm-devel] [PATCH] dm: Make MIN_IOS, et al, tunable via sysctl.
On Tue, 2013-08-20 at 17:47 -0400, Mikulas Patocka wrote: > > On Tue, 20 Aug 2013, Frank Mayhar wrote: > > > On Tue, 2013-08-20 at 17:22 -0400, Mikulas Patocka wrote: > > > On Fri, 16 Aug 2013, Frank Mayhar wrote: > > > > The device mapper and some of its modules allocate memory pools at > > > > various points when setting up a device. In some cases, these pools are > > > > fairly large, for example the multipath module allocates a 256-entry > > > > pool and the dm itself allocates three of that size. In a > > > > memory-constrained environment where we're creating a lot of these > > > > devices, the memory use can quickly become significant. Unfortunately, > > > > there's currently no way to change the size of the pools other than by > > > > changing a constant and rebuilding the kernel. > > > I think it would be better to set the values to some low value (1 should > > > be enough, there is 16 in some places that is already low enough). There > > > is no need to make it user-configurable, because the user will see no > > > additional benefit from bigger mempools. > > > > > > Maybe multipath is the exception - but other dm targets don't really need > > > big mempools so there is no need to make the size configurable. > > > > The point is not to make the mempools bigger, the point is to be able to > > make them _smaller_ for memory-constrained environments. In some cases, > > even 16 can be too big, especially when creating a large number of > > devices. > > > > In any event, it seems unfortunate to me that these values are > > hard-coded. One shouldn't have to rebuild the kernel to change them, > > IMHO. > > So make a patch that changes 16 to 1 if it helps on your system (I'd like > to ask - what is the configuration where this kind of change helps?). > There is no need for that to be tunable, anyone can live with mempool size > being 1. I reiterate: It seems unfortunate that these values are hard-coded. It seems to me that a sysadmin should be able to tune them according to his or her needs. Particularly when the same kernel is being run on many machines that may have different constraints; building a special kernel for each set of constraints doesn't scale. And as I said, it's a memory-constrained environment. -- Frank Mayhar 310-460-4042 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [dm-devel] [PATCH] dm: Make MIN_IOS, et al, tunable via sysctl.
On Tue, 20 Aug 2013, Frank Mayhar wrote: > On Tue, 2013-08-20 at 17:22 -0400, Mikulas Patocka wrote: > > On Fri, 16 Aug 2013, Frank Mayhar wrote: > > > The device mapper and some of its modules allocate memory pools at > > > various points when setting up a device. In some cases, these pools are > > > fairly large, for example the multipath module allocates a 256-entry > > > pool and the dm itself allocates three of that size. In a > > > memory-constrained environment where we're creating a lot of these > > > devices, the memory use can quickly become significant. Unfortunately, > > > there's currently no way to change the size of the pools other than by > > > changing a constant and rebuilding the kernel. > > I think it would be better to set the values to some low value (1 should > > be enough, there is 16 in some places that is already low enough). There > > is no need to make it user-configurable, because the user will see no > > additional benefit from bigger mempools. > > > > Maybe multipath is the exception - but other dm targets don't really need > > big mempools so there is no need to make the size configurable. > > The point is not to make the mempools bigger, the point is to be able to > make them _smaller_ for memory-constrained environments. In some cases, > even 16 can be too big, especially when creating a large number of > devices. > > In any event, it seems unfortunate to me that these values are > hard-coded. One shouldn't have to rebuild the kernel to change them, > IMHO. So make a patch that changes 16 to 1 if it helps on your system (I'd like to ask - what is the configuration where this kind of change helps?). There is no need for that to be tunable, anyone can live with mempool size being 1. Mikulas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [dm-devel] [PATCH] dm: Make MIN_IOS, et al, tunable via sysctl.
On Tue, 2013-08-20 at 17:22 -0400, Mikulas Patocka wrote: > On Fri, 16 Aug 2013, Frank Mayhar wrote: > > The device mapper and some of its modules allocate memory pools at > > various points when setting up a device. In some cases, these pools are > > fairly large, for example the multipath module allocates a 256-entry > > pool and the dm itself allocates three of that size. In a > > memory-constrained environment where we're creating a lot of these > > devices, the memory use can quickly become significant. Unfortunately, > > there's currently no way to change the size of the pools other than by > > changing a constant and rebuilding the kernel. > I think it would be better to set the values to some low value (1 should > be enough, there is 16 in some places that is already low enough). There > is no need to make it user-configurable, because the user will see no > additional benefit from bigger mempools. > > Maybe multipath is the exception - but other dm targets don't really need > big mempools so there is no need to make the size configurable. The point is not to make the mempools bigger, the point is to be able to make them _smaller_ for memory-constrained environments. In some cases, even 16 can be too big, especially when creating a large number of devices. In any event, it seems unfortunate to me that these values are hard-coded. One shouldn't have to rebuild the kernel to change them, IMHO. -- Frank Mayhar 310-460-4042 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [dm-devel] [PATCH] dm: Make MIN_IOS, et al, tunable via sysctl.
On Fri, 16 Aug 2013, Frank Mayhar wrote:
> The device mapper and some of its modules allocate memory pools at
> various points when setting up a device. In some cases, these pools are
> fairly large, for example the multipath module allocates a 256-entry
> pool and the dm itself allocates three of that size. In a
> memory-constrained environment where we're creating a lot of these
> devices, the memory use can quickly become significant. Unfortunately,
> there's currently no way to change the size of the pools other than by
> changing a constant and rebuilding the kernel.
>
> This patch fixes that by changing the hardcoded MIN_IOS (and certain
> other) #defines in dm-crypt, dm-io, dm-mpath, dm-snap and dm itself to
> sysctl-modifiable values. This lets us change the size of these pools
> on the fly, we can reduce the size of the pools and reduce memory
> pressure.
>
> We tested performance of dm-mpath with smaller MIN_IOS sizes for both dm
> and dm-mpath, from a value of 32 all the way down to zero. Bearing in
> mind that the underlying devices were network-based, we saw essentially
> no performance degradation; if there was any, it was down in the noise.
> One might wonder why these sizes are the way they are; I investigated
> and they've been unchanged since at least 2006.
I think it would be better to set the values to some low value (1 should
be enough, there is 16 in some places that is already low enough). There
is no need to make it user-configurable, because the user will see no
additional benefit from bigger mempools.
Maybe multipath is the exception - but other dm targets don't really need
big mempools so there is no need to make the size configurable.
Mikulas
> Signed-off-by: Frank Mayhar
> ---
> drivers/md/dm-crypt.c | 63
> +++
> drivers/md/dm-io.c| 58 +--
> drivers/md/dm-mpath.c | 48 ++-
> drivers/md/dm-snap.c | 45 +++-
> drivers/md/dm.c | 41 ++---
> 5 files changed, 244 insertions(+), 11 deletions(-)
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 6d2d41a..d68f10a 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -178,12 +178,55 @@ struct crypt_config {
> #define MIN_IOS16
> #define MIN_POOL_PAGES 32
>
> +static int sysctl_dm_crypt_min_ios = MIN_IOS;
> +static int sysctl_dm_crypt_min_pool_pages = MIN_POOL_PAGES;
> +
> static struct kmem_cache *_crypt_io_pool;
>
> static void clone_init(struct dm_crypt_io *, struct bio *);
> static void kcryptd_queue_crypt(struct dm_crypt_io *io);
> static u8 *iv_of_dmreq(struct crypt_config *cc, struct dm_crypt_request
> *dmreq);
>
> +static struct ctl_table_header *dm_crypt_table_header;
> +static ctl_table dm_crypt_ctl_table[] = {
> + {
> + .procname = "min_ios",
> + .data = _dm_crypt_min_ios,
> + .maxlen = sizeof(int),
> + .mode = S_IRUGO|S_IWUSR,
> + .proc_handler = proc_dointvec,
> + },
> + {
> + .procname = "min_pool_pages",
> + .data = _dm_crypt_min_pool_pages,
> + .maxlen = sizeof(int),
> + .mode = S_IRUGO|S_IWUSR,
> + .proc_handler = proc_dointvec,
> + },
> + { }
> +};
> +
> +static ctl_table dm_crypt_dir_table[] = {
> + { .procname = "crypt",
> + .mode = 0555,
> + .child= dm_crypt_ctl_table },
> + { }
> +};
> +
> +static ctl_table dm_crypt_dm_dir_table[] = {
> + { .procname = "dm",
> + .mode = 0555,
> + .child= dm_crypt_dir_table },
> + { }
> +};
> +
> +static ctl_table dm_crypt_root_table[] = {
> + { .procname = "dev",
> + .mode = 0555,
> + .child= dm_crypt_dm_dir_table },
> + { }
> +};
> +
> static struct crypt_cpu *this_crypt_config(struct crypt_config *cc)
> {
> return this_cpu_ptr(cc->cpu);
> @@ -1571,7 +1614,8 @@ static int crypt_ctr(struct dm_target *ti, unsigned int
> argc, char **argv)
> goto bad;
>
> ret = -ENOMEM;
> - cc->io_pool = mempool_create_slab_pool(MIN_IOS, _crypt_io_pool);
> + cc->io_pool = mempool_create_slab_pool(sysctl_dm_crypt_min_ios,
> +_crypt_io_pool);
> if (!cc->io_pool) {
> ti->error = "Cannot allocate crypt io mempool";
> goto bad;
> @@ -1583,20 +1627,22 @@ static int crypt_ctr(struct dm_target *ti, unsigned
> int argc, char **argv)
> cc->dmreq_start += crypto_ablkcipher_alignmask(any_tfm(cc)) &
> ~(crypto_tfm_ctx_alignment() - 1);
>
> - cc->req_pool = mempool_create_kmalloc_pool(MIN_IOS, cc->dmreq_start +
> + cc->req_pool =
Re: [dm-devel] [PATCH] dm: Make MIN_IOS, et al, tunable via sysctl.
On Fri, 16 Aug 2013, Frank Mayhar wrote:
The device mapper and some of its modules allocate memory pools at
various points when setting up a device. In some cases, these pools are
fairly large, for example the multipath module allocates a 256-entry
pool and the dm itself allocates three of that size. In a
memory-constrained environment where we're creating a lot of these
devices, the memory use can quickly become significant. Unfortunately,
there's currently no way to change the size of the pools other than by
changing a constant and rebuilding the kernel.
This patch fixes that by changing the hardcoded MIN_IOS (and certain
other) #defines in dm-crypt, dm-io, dm-mpath, dm-snap and dm itself to
sysctl-modifiable values. This lets us change the size of these pools
on the fly, we can reduce the size of the pools and reduce memory
pressure.
We tested performance of dm-mpath with smaller MIN_IOS sizes for both dm
and dm-mpath, from a value of 32 all the way down to zero. Bearing in
mind that the underlying devices were network-based, we saw essentially
no performance degradation; if there was any, it was down in the noise.
One might wonder why these sizes are the way they are; I investigated
and they've been unchanged since at least 2006.
I think it would be better to set the values to some low value (1 should
be enough, there is 16 in some places that is already low enough). There
is no need to make it user-configurable, because the user will see no
additional benefit from bigger mempools.
Maybe multipath is the exception - but other dm targets don't really need
big mempools so there is no need to make the size configurable.
Mikulas
Signed-off-by: Frank Mayhar fmay...@google.com
---
drivers/md/dm-crypt.c | 63
+++
drivers/md/dm-io.c| 58 +--
drivers/md/dm-mpath.c | 48 ++-
drivers/md/dm-snap.c | 45 +++-
drivers/md/dm.c | 41 ++---
5 files changed, 244 insertions(+), 11 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 6d2d41a..d68f10a 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -178,12 +178,55 @@ struct crypt_config {
#define MIN_IOS16
#define MIN_POOL_PAGES 32
+static int sysctl_dm_crypt_min_ios = MIN_IOS;
+static int sysctl_dm_crypt_min_pool_pages = MIN_POOL_PAGES;
+
static struct kmem_cache *_crypt_io_pool;
static void clone_init(struct dm_crypt_io *, struct bio *);
static void kcryptd_queue_crypt(struct dm_crypt_io *io);
static u8 *iv_of_dmreq(struct crypt_config *cc, struct dm_crypt_request
*dmreq);
+static struct ctl_table_header *dm_crypt_table_header;
+static ctl_table dm_crypt_ctl_table[] = {
+ {
+ .procname = min_ios,
+ .data = sysctl_dm_crypt_min_ios,
+ .maxlen = sizeof(int),
+ .mode = S_IRUGO|S_IWUSR,
+ .proc_handler = proc_dointvec,
+ },
+ {
+ .procname = min_pool_pages,
+ .data = sysctl_dm_crypt_min_pool_pages,
+ .maxlen = sizeof(int),
+ .mode = S_IRUGO|S_IWUSR,
+ .proc_handler = proc_dointvec,
+ },
+ { }
+};
+
+static ctl_table dm_crypt_dir_table[] = {
+ { .procname = crypt,
+ .mode = 0555,
+ .child= dm_crypt_ctl_table },
+ { }
+};
+
+static ctl_table dm_crypt_dm_dir_table[] = {
+ { .procname = dm,
+ .mode = 0555,
+ .child= dm_crypt_dir_table },
+ { }
+};
+
+static ctl_table dm_crypt_root_table[] = {
+ { .procname = dev,
+ .mode = 0555,
+ .child= dm_crypt_dm_dir_table },
+ { }
+};
+
static struct crypt_cpu *this_crypt_config(struct crypt_config *cc)
{
return this_cpu_ptr(cc-cpu);
@@ -1571,7 +1614,8 @@ static int crypt_ctr(struct dm_target *ti, unsigned int
argc, char **argv)
goto bad;
ret = -ENOMEM;
- cc-io_pool = mempool_create_slab_pool(MIN_IOS, _crypt_io_pool);
+ cc-io_pool = mempool_create_slab_pool(sysctl_dm_crypt_min_ios,
+_crypt_io_pool);
if (!cc-io_pool) {
ti-error = Cannot allocate crypt io mempool;
goto bad;
@@ -1583,20 +1627,22 @@ static int crypt_ctr(struct dm_target *ti, unsigned
int argc, char **argv)
cc-dmreq_start += crypto_ablkcipher_alignmask(any_tfm(cc))
~(crypto_tfm_ctx_alignment() - 1);
- cc-req_pool = mempool_create_kmalloc_pool(MIN_IOS, cc-dmreq_start +
+ cc-req_pool = mempool_create_kmalloc_pool(sysctl_dm_crypt_min_ios,
+ cc-dmreq_start +
sizeof(struct
Re: [dm-devel] [PATCH] dm: Make MIN_IOS, et al, tunable via sysctl.
On Tue, 2013-08-20 at 17:22 -0400, Mikulas Patocka wrote: On Fri, 16 Aug 2013, Frank Mayhar wrote: The device mapper and some of its modules allocate memory pools at various points when setting up a device. In some cases, these pools are fairly large, for example the multipath module allocates a 256-entry pool and the dm itself allocates three of that size. In a memory-constrained environment where we're creating a lot of these devices, the memory use can quickly become significant. Unfortunately, there's currently no way to change the size of the pools other than by changing a constant and rebuilding the kernel. I think it would be better to set the values to some low value (1 should be enough, there is 16 in some places that is already low enough). There is no need to make it user-configurable, because the user will see no additional benefit from bigger mempools. Maybe multipath is the exception - but other dm targets don't really need big mempools so there is no need to make the size configurable. The point is not to make the mempools bigger, the point is to be able to make them _smaller_ for memory-constrained environments. In some cases, even 16 can be too big, especially when creating a large number of devices. In any event, it seems unfortunate to me that these values are hard-coded. One shouldn't have to rebuild the kernel to change them, IMHO. -- Frank Mayhar 310-460-4042 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [dm-devel] [PATCH] dm: Make MIN_IOS, et al, tunable via sysctl.
On Tue, 20 Aug 2013, Frank Mayhar wrote: On Tue, 2013-08-20 at 17:22 -0400, Mikulas Patocka wrote: On Fri, 16 Aug 2013, Frank Mayhar wrote: The device mapper and some of its modules allocate memory pools at various points when setting up a device. In some cases, these pools are fairly large, for example the multipath module allocates a 256-entry pool and the dm itself allocates three of that size. In a memory-constrained environment where we're creating a lot of these devices, the memory use can quickly become significant. Unfortunately, there's currently no way to change the size of the pools other than by changing a constant and rebuilding the kernel. I think it would be better to set the values to some low value (1 should be enough, there is 16 in some places that is already low enough). There is no need to make it user-configurable, because the user will see no additional benefit from bigger mempools. Maybe multipath is the exception - but other dm targets don't really need big mempools so there is no need to make the size configurable. The point is not to make the mempools bigger, the point is to be able to make them _smaller_ for memory-constrained environments. In some cases, even 16 can be too big, especially when creating a large number of devices. In any event, it seems unfortunate to me that these values are hard-coded. One shouldn't have to rebuild the kernel to change them, IMHO. So make a patch that changes 16 to 1 if it helps on your system (I'd like to ask - what is the configuration where this kind of change helps?). There is no need for that to be tunable, anyone can live with mempool size being 1. Mikulas -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [dm-devel] [PATCH] dm: Make MIN_IOS, et al, tunable via sysctl.
On Tue, 2013-08-20 at 17:47 -0400, Mikulas Patocka wrote: On Tue, 20 Aug 2013, Frank Mayhar wrote: On Tue, 2013-08-20 at 17:22 -0400, Mikulas Patocka wrote: On Fri, 16 Aug 2013, Frank Mayhar wrote: The device mapper and some of its modules allocate memory pools at various points when setting up a device. In some cases, these pools are fairly large, for example the multipath module allocates a 256-entry pool and the dm itself allocates three of that size. In a memory-constrained environment where we're creating a lot of these devices, the memory use can quickly become significant. Unfortunately, there's currently no way to change the size of the pools other than by changing a constant and rebuilding the kernel. I think it would be better to set the values to some low value (1 should be enough, there is 16 in some places that is already low enough). There is no need to make it user-configurable, because the user will see no additional benefit from bigger mempools. Maybe multipath is the exception - but other dm targets don't really need big mempools so there is no need to make the size configurable. The point is not to make the mempools bigger, the point is to be able to make them _smaller_ for memory-constrained environments. In some cases, even 16 can be too big, especially when creating a large number of devices. In any event, it seems unfortunate to me that these values are hard-coded. One shouldn't have to rebuild the kernel to change them, IMHO. So make a patch that changes 16 to 1 if it helps on your system (I'd like to ask - what is the configuration where this kind of change helps?). There is no need for that to be tunable, anyone can live with mempool size being 1. I reiterate: It seems unfortunate that these values are hard-coded. It seems to me that a sysadmin should be able to tune them according to his or her needs. Particularly when the same kernel is being run on many machines that may have different constraints; building a special kernel for each set of constraints doesn't scale. And as I said, it's a memory-constrained environment. -- Frank Mayhar 310-460-4042 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [dm-devel] [PATCH] dm: Make MIN_IOS, et al, tunable via sysctl.
On Fri, Aug 16, 2013 at 03:55:21PM -0700, Frank Mayhar wrote: > This patch fixes that by changing the hardcoded MIN_IOS (and certain > other) #defines in dm-crypt, dm-io, dm-mpath, dm-snap and dm itself to > sysctl-modifiable values. This lets us change the size of these pools > on the fly, we can reduce the size of the pools and reduce memory > pressure. Did you consider using module_param_named()? (E.g. dm_verity_prefetch_cluster in dm-verity.) Alasdair -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [dm-devel] [PATCH] dm: Make MIN_IOS, et al, tunable via sysctl.
On Fri, Aug 16, 2013 at 03:55:21PM -0700, Frank Mayhar wrote: This patch fixes that by changing the hardcoded MIN_IOS (and certain other) #defines in dm-crypt, dm-io, dm-mpath, dm-snap and dm itself to sysctl-modifiable values. This lets us change the size of these pools on the fly, we can reduce the size of the pools and reduce memory pressure. Did you consider using module_param_named()? (E.g. dm_verity_prefetch_cluster in dm-verity.) Alasdair -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
