Re: [PATCH V6 1/5] mtd: partitions: add helper for deleting partition

[Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On Fri, May 26, 2017 at 03:14:11PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> 
> There are two similar functions handling deletion. One handles single
> partition and another whole MTD flash device. They share (duplicate)
> some code so it makes sense to add a small helper for that part.
> 
> Function del_mtd_partitions has been moved a bit to keep all deleting
> stuff together.
> 
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

> +/**
> + * __mtd_del_partition - delete MTD partition
> + *
> + * @priv: internal MTD struct for partition to be deleted
> + *
> + * This function must be called with the partitions mutex locked.
> + */
> +static int __mtd_del_partition(struct mtd_part *priv)
> +{
> +	int err;
> +
> +	sysfs_remove_files(&priv->mtd.dev.kobj, mtd_partition_attrs);

Notably, the above wasn't called in del_mtd_partitions() previously. Is
that intentional?

As I read the code, we were actually missing this before; either in
error handling, or on device removal. But in either case, the entire
device was going to disappear, so the sysfs files would have been
cleaned up anyway?

Also, is there any chance of double-calling this? I think not, but even
if there is, it looks like the low-level routines are reslient to
non-existent files.

I'm mostly thinking out loud here. I think this is a positive change,
even if it wasn't noted explicitly.

> +
> +	err = del_mtd_device(&priv->mtd);
> +	if (err)
> +		return err;
> +
> +	list_del(&priv->list);
> +	free_partition(priv);
> +
> +	return 0;
> +}

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html