Hi Dan,
On Sat, Dec 10, 2016 at 10:28:30PM -0800, Dan Williams wrote:
> In preparation for a facility that enables dax regions to be
> sub-divided, introduce a 'dax/available_size' attribute. This attribute
> appears under the parent device that registered the device-dax region,
> and it assumes that the device-dax-core owns the driver-data for that
> device.
>
> 'dax/available_size' adjusts dynamically as dax-device instances are
> registered and unregistered.
>
> As a side effect of using __request_region() to reserve capacity from
> the dax_region we now track pointers to those returned resources rather
> than duplicating the passed in resource array.
>
> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> ---
[...]
> +static const struct attribute_group *dax_region_attribute_groups[] = {
> + &dax_region_attribute_group,
> + NULL,
> };
>
> static struct inode *dax_alloc_inode(struct super_block *sb)
> @@ -200,12 +251,27 @@ void dax_region_put(struct dax_region *dax_region)
> }
> EXPORT_SYMBOL_GPL(dax_region_put);
>
> +
Stray extra newline?
[...]
> struct dax_region *alloc_dax_region(struct device *parent, int region_id,
> struct resource *res, unsigned int align, void *addr,
> unsigned long pfn_flags)
> {
> struct dax_region *dax_region;
>
> + if (dev_get_drvdata(parent)) {
> + dev_WARN(parent, "dax core found drvdata already in use\n");
> + return NULL;
> + }
> +
My first thought was, it might be interesting to see who already claimed
the drvdata. Then I figured, how are multiple sub-regions of a dax-device
supposed to work? What am I missing here?
> if (!IS_ALIGNED(res->start, align)
> || !IS_ALIGNED(resource_size(res), align))
> return NULL;
> @@ -213,16 +279,26 @@ struct dax_region *alloc_dax_region(struct device
> *parent, int region_id,
> dax_region = kzalloc(sizeof(*dax_region), GFP_KERNEL);
> if (!dax_region)
> return NULL;
> -
> - memcpy(&dax_region->res, res, sizeof(*res));
> + dev_set_drvdata(parent, dax_region);
> + dax_region->res.name = dev_name(parent);
> + dax_region->res.start = res->start;
> + dax_region->res.end = res->end;
> dax_region->pfn_flags = pfn_flags;
> + mutex_init(&dax_region->lock);
> kref_init(&dax_region->kref);
> dax_region->id = region_id;
> ida_init(&dax_region->ida);
> dax_region->align = align;
> dax_region->dev = parent;
> dax_region->base = addr;
> + if (sysfs_create_groups(&parent->kobj, dax_region_attribute_groups)) {
> + kfree(dax_region);
> + return NULL;;
> + }
>
> + kref_get(&dax_region->kref);
> + if (devm_add_action_or_reset(parent, dax_region_unregister, dax_region))
> + return NULL;
> return dax_region;
> }
> EXPORT_SYMBOL_GPL(alloc_dax_region);
[...]
> @@ -568,28 +654,42 @@ struct dax_dev *devm_create_dax_dev(struct dax_region
> *dax_region,
> struct cdev *cdev;
> dev_t dev_t;
>
> - dax_dev = kzalloc(sizeof(*dax_dev) + sizeof(*res) * count, GFP_KERNEL);
> + dax_dev = kzalloc(sizeof(*dax_dev), GFP_KERNEL);
> if (!dax_dev)
> return ERR_PTR(-ENOMEM);
>
> + dax_dev->res = kzalloc(sizeof(res) * count, GFP_KERNEL);
dax_dev->res = kcalloc(sizeof(res), count, GFP_KERNEL); ?
> + if (!dax_dev->res)
> + goto err_res;
> +
Byte,
Johannes
