Message ID | 1423162081-1734-1-git-send-email-dehrenberg@chromium.org |
---|---|
State | Superseded |
Headers | show |
On 02/05/2015 03:48 PM, Dan Ehrenberg wrote: > MTD allows dynamic partitioning by the BLKPG ioctl. > This patch makes dynamic partitioning more flexible by: > - Allowing addition of a partition to be added on top of another > partition. The two partitions compose naturally: the offsets are added > and lengths are checked to be in bounds. This is useful when > repartitioning an existing partitioned device since the underlying > device doesn't exist to add partitions to. IMHO, this explanation is not that clear. In particular "Allowing addition of a partition to be added on top of another partition" looks odd to me. Maybe you can say something like: "The current code restricts partition dynamic addition to work only on the master MTD device. This doesn't make a lot of sense, and is impossible to meet if the device is already partitioned (since the master MTD device is not visible). This commit removes the restriction." > - Removing overlap checks for dynamic partitions. I don't see any > particular reason why overlapping dynamic partitions should be > prohibited while static partitions are allowed to overlap freely, and > this is useful for users who want one additional partition to span > over the whole device. Makes sense. > - Allowing partitions to be deleted by referencing any partition with > the same master. For example, if you have /dev/mtd0 and /dev/mtd1 both > partitions on the same underlying device, then you can call > BLKPG_DEL_PARTITION with an fd of /dev/mtd0 and pno of /dev/mtd1, and > /dev/mtd1 will be deleted (as opposed to returning EINVAL to signal a > missing partition, which it did previously). > Should we split this in two or three patches? Don't have a strong opinion.
Hi Dan, On Thu, 5 Feb 2015 10:48:01 -0800 Dan Ehrenberg <dehrenberg@chromium.org> wrote: > MTD allows dynamic partitioning by the BLKPG ioctl. > This patch makes dynamic partitioning more flexible by: > - Allowing addition of a partition to be added on top of another > partition. The two partitions compose naturally: the offsets are added > and lengths are checked to be in bounds. This is useful when > repartitioning an existing partitioned device since the underlying > device doesn't exist to add partitions to. > - Removing overlap checks for dynamic partitions. I don't see any > particular reason why overlapping dynamic partitions should be > prohibited while static partitions are allowed to overlap freely, and > this is useful for users who want one additional partition to span > over the whole device. > - Allowing partitions to be deleted by referencing any partition with > the same master. For example, if you have /dev/mtd0 and /dev/mtd1 both > partitions on the same underlying device, then you can call > BLKPG_DEL_PARTITION with an fd of /dev/mtd0 and pno of /dev/mtd1, and > /dev/mtd1 will be deleted (as opposed to returning EINVAL to signal a > missing partition, which it did previously). > > Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org> > --- > drivers/mtd/mtdchar.c | 4 ---- > drivers/mtd/mtdpart.c | 32 ++++++++++++++++---------------- > 2 files changed, 16 insertions(+), 20 deletions(-) > > diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c > index 5356395..30215c7 100644 > --- a/drivers/mtd/mtdchar.c > +++ b/drivers/mtd/mtdchar.c > @@ -545,10 +545,6 @@ static int mtdchar_blkpg_ioctl(struct mtd_info *mtd, > switch (a.op) { > case BLKPG_ADD_PARTITION: > > - /* Only master mtd device must be used to add partitions */ > - if (mtd_is_partition(mtd)) > - return -EINVAL; > - > /* Sanitize user input */ > p.devname[BLKPG_DEVNAMELTH - 1] = '\0'; > > diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c > index a3e3a7d..7874bbd 100644 > --- a/drivers/mtd/mtdpart.c > +++ b/drivers/mtd/mtdpart.c > @@ -551,7 +551,7 @@ int mtd_add_partition(struct mtd_info *master, const char *name, Since you no longer require the first argument to be a master mtd device maybe you should rename it. > long long offset, long long length) > { > struct mtd_partition part; > - struct mtd_part *p, *new; > + struct mtd_part *new; > uint64_t start, end; > int ret = 0; > > @@ -566,6 +566,12 @@ int mtd_add_partition(struct mtd_info *master, const char *name, > if (length <= 0) > return -EINVAL; > > + if (mtd_is_partition(master)) { > + struct mtd_part *master_partition = PART(master); > + offset += master_partition->offset; > + master = master_partition->master; > + } > + > part.name = name; > part.size = length; > part.offset = offset; > @@ -580,27 +586,12 @@ int mtd_add_partition(struct mtd_info *master, const char *name, > end = offset + length; > > mutex_lock(&mtd_partitions_mutex); > - list_for_each_entry(p, &mtd_partitions, list) > - if (p->master == master) { > - if ((start >= p->offset) && > - (start < (p->offset + p->mtd.size))) > - goto err_inv; > - > - if ((end >= p->offset) && > - (end < (p->offset + p->mtd.size))) > - goto err_inv; > - } > - > list_add(&new->list, &mtd_partitions); > mutex_unlock(&mtd_partitions_mutex); > > add_mtd_device(&new->mtd); > > return ret; > -err_inv: > - mutex_unlock(&mtd_partitions_mutex); > - free_partition(new); > - return -EINVAL; > } > EXPORT_SYMBOL_GPL(mtd_add_partition); > > @@ -609,6 +600,15 @@ int mtd_del_partition(struct mtd_info *master, int partno) Ditto. Best Regards, Boris
diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c index 5356395..30215c7 100644 --- a/drivers/mtd/mtdchar.c +++ b/drivers/mtd/mtdchar.c @@ -545,10 +545,6 @@ static int mtdchar_blkpg_ioctl(struct mtd_info *mtd, switch (a.op) { case BLKPG_ADD_PARTITION: - /* Only master mtd device must be used to add partitions */ - if (mtd_is_partition(mtd)) - return -EINVAL; - /* Sanitize user input */ p.devname[BLKPG_DEVNAMELTH - 1] = '\0'; diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c index a3e3a7d..7874bbd 100644 --- a/drivers/mtd/mtdpart.c +++ b/drivers/mtd/mtdpart.c @@ -551,7 +551,7 @@ int mtd_add_partition(struct mtd_info *master, const char *name, long long offset, long long length) { struct mtd_partition part; - struct mtd_part *p, *new; + struct mtd_part *new; uint64_t start, end; int ret = 0; @@ -566,6 +566,12 @@ int mtd_add_partition(struct mtd_info *master, const char *name, if (length <= 0) return -EINVAL; + if (mtd_is_partition(master)) { + struct mtd_part *master_partition = PART(master); + offset += master_partition->offset; + master = master_partition->master; + } + part.name = name; part.size = length; part.offset = offset; @@ -580,27 +586,12 @@ int mtd_add_partition(struct mtd_info *master, const char *name, end = offset + length; mutex_lock(&mtd_partitions_mutex); - list_for_each_entry(p, &mtd_partitions, list) - if (p->master == master) { - if ((start >= p->offset) && - (start < (p->offset + p->mtd.size))) - goto err_inv; - - if ((end >= p->offset) && - (end < (p->offset + p->mtd.size))) - goto err_inv; - } - list_add(&new->list, &mtd_partitions); mutex_unlock(&mtd_partitions_mutex); add_mtd_device(&new->mtd); return ret; -err_inv: - mutex_unlock(&mtd_partitions_mutex); - free_partition(new); - return -EINVAL; } EXPORT_SYMBOL_GPL(mtd_add_partition); @@ -609,6 +600,15 @@ int mtd_del_partition(struct mtd_info *master, int partno) struct mtd_part *slave, *next; int ret = -EINVAL; + /* If del_partition was called as a partition, just treat + * this as if it were called on its master. This is a harmless + * generalization and lets partitions be deleted in a system + * where they are sometimes already present on boot */ + if (mtd_is_partition(master)) { + struct mtd_part *master_partition = PART(master); + master = master_partition->master; + } + mutex_lock(&mtd_partitions_mutex); list_for_each_entry_safe(slave, next, &mtd_partitions, list) if ((slave->master == master) &&
MTD allows dynamic partitioning by the BLKPG ioctl. This patch makes dynamic partitioning more flexible by: - Allowing addition of a partition to be added on top of another partition. The two partitions compose naturally: the offsets are added and lengths are checked to be in bounds. This is useful when repartitioning an existing partitioned device since the underlying device doesn't exist to add partitions to. - Removing overlap checks for dynamic partitions. I don't see any particular reason why overlapping dynamic partitions should be prohibited while static partitions are allowed to overlap freely, and this is useful for users who want one additional partition to span over the whole device. - Allowing partitions to be deleted by referencing any partition with the same master. For example, if you have /dev/mtd0 and /dev/mtd1 both partitions on the same underlying device, then you can call BLKPG_DEL_PARTITION with an fd of /dev/mtd0 and pno of /dev/mtd1, and /dev/mtd1 will be deleted (as opposed to returning EINVAL to signal a missing partition, which it did previously). Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org> --- drivers/mtd/mtdchar.c | 4 ---- drivers/mtd/mtdpart.c | 32 ++++++++++++++++---------------- 2 files changed, 16 insertions(+), 20 deletions(-)