Re: [PATCH v3 3/7] block: add storage element management ioctls

[Christoph Hellwig <hch@lst.de>]

> +	if (copy_from_user(&rep, argp,
> +			   sizeof(struct blk_storage_elements_report)))

Can we shorten the name of the struct a bit?  blk_se_report?
Similar for other identifiers?

> +		return -EFAULT;
> +
> +	ret = bdev_report_storage_elements(bdev, NULL, &nr_elements);
> +	if (ret)
> +		return ret;
> +
> +	nr_elements = min(rep.nr_elements, nr_elements);
> +	if (!nr_elements)
> +		return -EINVAL;
> +
> +	elements = kzalloc_objs(struct blk_storage_element, nr_elements);

This doesn't work as it could race.  We'll always need to allocate
the space for all the elements the user asked for?

> +	retc = copy_to_user(argp + sizeof(struct blk_storage_elements_report),
> +			    elements,
> +			    sizeof(struct blk_storage_element) * nr_elements);
> +	if (retc) {
> +		ret = -EFAULT;
> +		goto free_elements;
> +	}
> +
> +	rep.nr_elements = nr_elements;
> +	retc = copy_to_user(argp, &rep,
> +			    sizeof(struct blk_storage_elements_report));

Why not copy back the entire struct in one go?

> +		ret = truncate_bdev_range(bdev, mode, 0,
> +			(get_capacity(bdev->bd_disk) << SECTOR_SHIFT) - 1);

Use bdev_nr_bytes() here?

> +	/*
> +	 * Storage element restoration is a destructive operation that will
> +	 * reset all zones. So fflush the device volatile write cache and
> +	 * invalidate all cached data that we may have.
> +	 */
> +	filemap_invalidate_lock(bdev->bd_mapping);
> +	ret = blkdev_issue_flush(bdev->bd_disk->part0);
> +	if (!ret)
> +		ret = truncate_bdev_range(bdev, mode, 0,
> +			(get_capacity(bdev->bd_disk) << SECTOR_SHIFT) - 1);

Same.  Also what about factoring the duplicate code block into a helper?