| 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
| ||
|
Date: Wed, 18 Mar 2015 14:40:54 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Mike Kravetz <mike.kravetz@...cle.com>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
Davidlohr Bueso <dave@...olabs.net>,
Aneesh Kumar <aneesh.kumar@...ux.vnet.ibm.com>,
Joonsoo Kim <iamjoonsoo.kim@....com>
Subject: Re: [PATCH V2 3/4] hugetlbfs: accept subpool min_size mount option
and setup accordingly
On Mon, 16 Mar 2015 16:53:28 -0700 Mike Kravetz <mike.kravetz@...cle.com> wrote:
> Make 'min_size=' be an option when mounting a hugetlbfs. This option
> takes the same value as the 'size' option. min_size can be specified
> with specifying size. If both are specified, min_size must be less
> that or equal to size else the mount will fail. If min_size is
> specified, then at mount time an attempt is made to reserve min_size
> pages. If the reservation fails, the mount fails. At umount time,
> the reserved pages are released.
>
> ...
>
> @@ -761,14 +763,32 @@ static const struct super_operations hugetlbfs_ops = {
> .show_options = generic_show_options,
> };
>
> +enum { NO_SIZE, SIZE_STD, SIZE_PERCENT };
> +
> +static bool
> +hugetlbfs_options_setsize(struct hstate *h, long long *size, int setsize)
> +{
> + if (setsize == NO_SIZE)
> + return false;
> +
> + if (setsize == SIZE_PERCENT) {
> + *size <<= huge_page_shift(h);
> + *size *= h->max_huge_pages;
> + do_div(*size, 100);
I suppose do_div() takes a long long. u64 would be more conventional.
I don't *think* all this code needed to use signed types.
> + }
> +
> + *size >>= huge_page_shift(h);
> + return true;
> +}
> +
> static int
> hugetlbfs_parse_options(char *options, struct hugetlbfs_config *pconfig)
> {
> char *p, *rest;
> substring_t args[MAX_OPT_ARGS];
> int option;
> - unsigned long long size = 0;
> - enum { NO_SIZE, SIZE_STD, SIZE_PERCENT } setsize = NO_SIZE;
> + unsigned long long max_size = 0, min_size = 0;
> + int max_setsize = NO_SIZE, min_setsize = NO_SIZE;
>
> if (!options)
> return 0;
> @@ -806,10 +826,10 @@ hugetlbfs_parse_options(char *options, struct hugetlbfs_config *pconfig)
> /* memparse() will accept a K/M/G without a digit */
> if (!isdigit(*args[0].from))
> goto bad_val;
> - size = memparse(args[0].from, &rest);
> - setsize = SIZE_STD;
> + max_size = memparse(args[0].from, &rest);
> + max_setsize = SIZE_STD;
> if (*rest == '%')
> - setsize = SIZE_PERCENT;
> + max_setsize = SIZE_PERCENT;
> break;
> }
>
> @@ -832,6 +852,17 @@ hugetlbfs_parse_options(char *options, struct hugetlbfs_config *pconfig)
> break;
> }
>
> + case Opt_min_size: {
> + /* memparse() will accept a K/M/G without a digit */
> + if (!isdigit(*args[0].from))
> + goto bad_val;
> + min_size = memparse(args[0].from, &rest);
> + min_setsize = SIZE_STD;
> + if (*rest == '%')
> + min_setsize = SIZE_PERCENT;
> + break;
> + }
> +
> default:
> pr_err("Bad mount option: \"%s\"\n", p);
> return -EINVAL;
> @@ -839,15 +870,17 @@ hugetlbfs_parse_options(char *options, struct hugetlbfs_config *pconfig)
> }
> }
>
> - /* Do size after hstate is set up */
> - if (setsize > NO_SIZE) {
> - struct hstate *h = pconfig->hstate;
> - if (setsize == SIZE_PERCENT) {
> - size <<= huge_page_shift(h);
> - size *= h->max_huge_pages;
> - do_div(size, 100);
> - }
> - pconfig->nr_blocks = (size >> huge_page_shift(h));
> + /* Calculate number of huge pages based on hstate */
> + if (hugetlbfs_options_setsize(pconfig->hstate, &max_size, max_setsize))
> + pconfig->nr_blocks = max_size;
So hugetlbfs_options_setsize takes an arg whichis in units of bytes,
modifies it in-place to b in units of pages and then copies it into
something which is in units of nr_blocks.
> + if (hugetlbfs_options_setsize(pconfig->hstate, &min_size, min_setsize))
> + pconfig->min_size = min_size;
> +
> + /* If max_size specified, then min_size must be smaller */
> + if (max_setsize > NO_SIZE && min_setsize > NO_SIZE &&
> + pconfig->min_size > pconfig->nr_blocks) {
> + pr_err("minimum size can not be greater than maximum size\n");
> + return -EINVAL;
> }
>
> return 0;
> @@ -872,6 +905,7 @@ hugetlbfs_fill_super(struct super_block *sb, void *data, int silent)
> config.gid = current_fsgid();
> config.mode = 0755;
> config.hstate = &default_hstate;
> + config.min_size = 0; /* No default minimum size */
> ret = hugetlbfs_parse_options(data, &config);
> if (ret)
> return ret;
> @@ -885,8 +919,15 @@ hugetlbfs_fill_super(struct super_block *sb, void *data, int silent)
> sbinfo->max_inodes = config.nr_inodes;
> sbinfo->free_inodes = config.nr_inodes;
> sbinfo->spool = NULL;
> - if (config.nr_blocks != -1) {
> - sbinfo->spool = hugepage_new_subpool(config.nr_blocks);
> + /*
> + * Allocate and initialize subpool if maximum or minimum size is
> + * specified. Any needed reservations (for minimim size) are taken
> + * taken when the subpool is created.
> + */
> + if (config.nr_blocks != -1 || config.min_size != 0) {
> + sbinfo->spool = hugepage_new_subpool(config.hstate,
> + config.nr_blocks,
> + config.min_size);
And hugepage_new_subpool() takes something in units of nr_blocks and
copies it into something whcih has units of nr-hugepages.
And it takes an arg called "size" which is no longer number-of-bytes
but is actually number-of-hpages.
It's all rather confusing and unclear. A good philosophy would be
never to use a variable called "size", because the reader doesn't know
what units that size is measured in. Instead, make sure that the name
reflects the variable's units. max_bytes, min_hpages, nr_blocks, etc.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists