| Message ID | 20170524094437.2174-3-zajec5@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Brian Norris |
| Headers | show |
On Wed, May 24, 2017 at 11:44:35AM +0200, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > Some flash device partitions can be containers with extra subpartitions > (volumes). When allocating subpartition it should be validated against > its parent but its master pointer has to point flash device. It's needed > to make all callbacks like part_read work as expected. It also has to > have offset calculated correctly. > > This patch modifies allocate_partition to detect if provided parent is > an existing partition and sets "master" and "offset" correctly if so. > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > --- > V5: Introduction of this patch to handle offset in allocate_partition > and avoid casting const to non-const in mtd_parse_part. > --- > drivers/mtd/mtdpart.c | 31 +++++++++++++++++++++++-------- > 1 file changed, 23 insertions(+), 8 deletions(-) > > diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c > index 92acd89e07cb..8a0629449804 100644 > --- a/drivers/mtd/mtdpart.c > +++ b/drivers/mtd/mtdpart.c > @@ -37,7 +37,13 @@ > static LIST_HEAD(mtd_partitions); > static DEFINE_MUTEX(mtd_partitions_mutex); > > -/* Our partition node structure */ > +/** > + * struct mtd_part - our partition node structure > + * > + * @mtd: struct holding partition details > + * @master: pointer to the flash device MTD struct > + * @offset: partition offset relative to the *flash device* > + */ > struct mtd_part { > struct mtd_info mtd; > struct mtd_info *master; > @@ -393,9 +399,18 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent, > const struct mtd_partition *part, int partno, > uint64_t cur_offset) > { > + struct mtd_info *master = parent; > struct mtd_part *slave; > + uint64_t parent_offset = 0; > char *name; > > + if (mtd_is_partition(parent)) { > + struct mtd_part *parent_part = mtd_to_part(parent); > + > + master = parent_part->master; Are you trying to keep a flat structure, or a tree? It seems like you're mostly doing a flat structure (with one bug, see below), but I was kinda thinking it'd be natural to actually represent the tree structure. In case that's confusing, I'll try to expalin below. Take a look at these two snippets: slave->mtd.dev.parent = IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) ? &parent->dev : parent->dev.parent; ... slave->master = master; So (assuming CONFIG_MTD_PARTITIONED_MASTER), you've set up a sysfs structure in which the parent device is always the device that created you, but the ->master always points at the top-level "master" MTD. [In the !CONFIG_MTD_PARTITIONED_MASTER case, then the sub-partitions will have ->dev.parent set to the device that created the *master*, not the device (i.e., MTD) that created the subpartition. This is inconsistent.] So I guess you need to decide if you're aiming to keep a mostly flat parental structure. i.e., should the ->master graph look like: master MTD -> partition 1 -> partition parsed from partition 1 or should it be a tree: master MTD -> partition 1 -> partition parsed from partition 1 Then, to be consistent you either need the ->mtd.dev.parent to be flat, like this: slave->mtd.dev.parent = IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) || mtd_is_partition(parent) ? &master->dev : master->dev.parent; or tree-like: slave->mtd.dev.parent = IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) || mtd_is_partition(parent) ? &parent->dev : parent->dev.parent; And the corresponding ->master for flat: slave->master = master; or tree-like: slave->master = parent; With tree-like, you need fewer modifications to this function. (Mostly you just would want the naming changes and/or comments, to clarify what "master" means.) With flat, I suppose maybe you only need to bugfix the slave->mtd.dev.parent assignment. Let me know what you think. Brian > + parent_offset = parent_part->offset; > + } > + > /* allocate the partition structure */ > slave = kzalloc(sizeof(*slave), GFP_KERNEL); > name = kstrdup(part->name, GFP_KERNEL); > @@ -493,12 +508,12 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent, > slave->mtd._put_device = part_put_device; > > slave->mtd._erase = part_erase; > - slave->master = parent; > - slave->offset = part->offset; > + slave->master = master; > + slave->offset = parent_offset + part->offset; > > - if (slave->offset == MTDPART_OFS_APPEND) > + if (part->offset == MTDPART_OFS_APPEND) > slave->offset = cur_offset; > - if (slave->offset == MTDPART_OFS_NXTBLK) { > + if (part->offset == MTDPART_OFS_NXTBLK) { > slave->offset = cur_offset; > if (mtd_mod_by_eb(cur_offset, parent) != 0) { > /* Round up to next erasesize */ > @@ -508,7 +523,7 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent, > (unsigned long long)cur_offset, (unsigned long long)slave->offset); > } > } > - if (slave->offset == MTDPART_OFS_RETAIN) { > + if (part->offset == MTDPART_OFS_RETAIN) { > slave->offset = cur_offset; > if (parent->size - slave->offset >= slave->mtd.size) { > slave->mtd.size = parent->size - slave->offset > @@ -536,8 +551,8 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent, > part->name); > goto out_register; > } > - if (slave->offset + slave->mtd.size > parent->size) { > - slave->mtd.size = parent->size - slave->offset; > + if (slave->offset + slave->mtd.size > parent_offset + parent->size) { > + slave->mtd.size = parent_offset + parent->size - slave->offset; > printk(KERN_WARNING"mtd: partition \"%s\" extends beyond the end of device \"%s\" -- size truncated to %#llx\n", > part->name, parent->name, (unsigned long long)slave->mtd.size); > } > -- > 2.11.0 >
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c index 92acd89e07cb..8a0629449804 100644 --- a/drivers/mtd/mtdpart.c +++ b/drivers/mtd/mtdpart.c @@ -37,7 +37,13 @@ static LIST_HEAD(mtd_partitions); static DEFINE_MUTEX(mtd_partitions_mutex); -/* Our partition node structure */ +/** + * struct mtd_part - our partition node structure + * + * @mtd: struct holding partition details + * @master: pointer to the flash device MTD struct + * @offset: partition offset relative to the *flash device* + */ struct mtd_part { struct mtd_info mtd; struct mtd_info *master; @@ -393,9 +399,18 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent, const struct mtd_partition *part, int partno, uint64_t cur_offset) { + struct mtd_info *master = parent; struct mtd_part *slave; + uint64_t parent_offset = 0; char *name; + if (mtd_is_partition(parent)) { + struct mtd_part *parent_part = mtd_to_part(parent); + + master = parent_part->master; + parent_offset = parent_part->offset; + } + /* allocate the partition structure */ slave = kzalloc(sizeof(*slave), GFP_KERNEL); name = kstrdup(part->name, GFP_KERNEL); @@ -493,12 +508,12 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent, slave->mtd._put_device = part_put_device; slave->mtd._erase = part_erase; - slave->master = parent; - slave->offset = part->offset; + slave->master = master; + slave->offset = parent_offset + part->offset; - if (slave->offset == MTDPART_OFS_APPEND) + if (part->offset == MTDPART_OFS_APPEND) slave->offset = cur_offset; - if (slave->offset == MTDPART_OFS_NXTBLK) { + if (part->offset == MTDPART_OFS_NXTBLK) { slave->offset = cur_offset; if (mtd_mod_by_eb(cur_offset, parent) != 0) { /* Round up to next erasesize */ @@ -508,7 +523,7 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent, (unsigned long long)cur_offset, (unsigned long long)slave->offset); } } - if (slave->offset == MTDPART_OFS_RETAIN) { + if (part->offset == MTDPART_OFS_RETAIN) { slave->offset = cur_offset; if (parent->size - slave->offset >= slave->mtd.size) { slave->mtd.size = parent->size - slave->offset @@ -536,8 +551,8 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent, part->name); goto out_register; } - if (slave->offset + slave->mtd.size > parent->size) { - slave->mtd.size = parent->size - slave->offset; + if (slave->offset + slave->mtd.size > parent_offset + parent->size) { + slave->mtd.size = parent_offset + parent->size - slave->offset; printk(KERN_WARNING"mtd: partition \"%s\" extends beyond the end of device \"%s\" -- size truncated to %#llx\n", part->name, parent->name, (unsigned long long)slave->mtd.size); }