On 3/23/20 11:55 PM, Dan Williams wrote:
> static ssize_t dev_dax_resize(struct dax_region *dax_region,
> struct dev_dax *dev_dax, resource_size_t size)
> {
> resource_size_t avail = dax_region_avail_size(dax_region), to_alloc;
> - resource_size_t dev_size = range_len(&dev_dax->range);
> + resource_size_t dev_size = dev_dax_size(dev_dax);
> struct resource *region_res = &dax_region->res;
> struct device *dev = &dev_dax->dev;
> - const char *name = dev_name(dev);
> struct resource *res, *first;
> + resource_size_t alloc = 0;
> + int rc;
>
> if (dev->driver)
> return -EBUSY;
> @@ -684,38 +766,47 @@ static ssize_t dev_dax_resize(struct dax_region *dax_region,
> * allocating a new resource.
> */
> first = region_res->child;
> - if (!first)
> - return __alloc_dev_dax_range(dev_dax, dax_region->res.start,
> - to_alloc);
You probably want to retain the condition above?
Otherwise it removes the ability to create new devices or resizing it , once we
have zero-ed the last one.
> - for (res = first; to_alloc && res; res = res->sibling) {
> +retry:
> + rc = -ENOSPC;
> + for (res = first; res; res = res->sibling) {
> struct resource *next = res->sibling;
> - resource_size_t free;
>
> /* space at the beginning of the region */
> - free = 0;
> - if (res == first && res->start > dax_region->res.start)
> - free = res->start - dax_region->res.start;
> - if (free >= to_alloc && dev_size == 0)
> - return __alloc_dev_dax_range(dev_dax,
> - dax_region->res.start, to_alloc);
> -
> - free = 0;
> + if (res == first && res->start > dax_region->res.start) {
> + alloc = min(res->start - dax_region->res.start,
> + to_alloc);
> + rc = __alloc_dev_dax_range(dev_dax,
> + dax_region->res.start, alloc);
> + break;
> + }
> +
> + alloc = 0;
> /* space between allocations */
> if (next && next->start > res->end + 1)
> - free = next->start - res->end + 1;
> + alloc = min(next->start - (res->end + 1), to_alloc);
>
> /* space at the end of the region */
> - if (free < to_alloc && !next && res->end < region_res->end)
> - free = region_res->end - res->end;
> -
> - if (free >= to_alloc && strcmp(name, res->name) == 0)
> - return __adjust_dev_dax_range(dev_dax, res,
> - resource_size(res) + to_alloc);
> - else if (free >= to_alloc && dev_size == 0)
> - return __alloc_dev_dax_range(dev_dax, res->end + 1,
> - to_alloc);
> + if (!alloc && !next && res->end < region_res->end)
> + alloc = min(region_res->end - res->end, to_alloc);
> +
> + if (!alloc)
> + continue;
> +
> + if (adjust_ok(dev_dax, res)) {
> + rc = __adjust_dev_dax_range(dev_dax, res,
> + resource_size(res) + alloc);
> + break;
> + }
> + rc = __alloc_dev_dax_range(dev_dax, res->end + 1,
> + alloc);
I am wondering if we should switch to:
if (adjust_ok(...))
rc = __adjust_dev_dax_range(...);
else
rc = __alloc_dev_dax_range(...);
And then a debug print at the end depicting whether and how did we grabbed
space? Something like:
dev_dbg(&dev_dax->dev, "%s(%d) %d", action, location, rc);
Assuming we set @location to its values when we allocate space at the end,
beginning or middle; and @action to whether we adjusted up/down or allocated new
range.
Essentially, something similar to namespaces scan_allocate() just to help
troubleshoot?
Regards,
Joao
