On 03/18/2015 02:40 PM, Andrew Morton wrote:
On Mon, 16 Mar 2015 16:53:28 -0700 Mike Kravetz <[email protected]> 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.


Thanks for the comments.

I didn't want to cut/paste/duplicate the code used to parse the existing
size option.  But, it looks like I made it harder to understand.  I'll
take a pass as cleaning this up and making it more clear.

--
Mike Kravetz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to