Message ID | 1480550174-8675-1-git-send-email-bmarzins@redhat.com (mailing list archive) |
---|---|
State | Accepted, archived |
Delegated to: | Mike Snitzer |
Headers | show |
ack On Wed, Nov 30, 2016 at 05:56:14PM -0600, Benjamin Marzinski wrote: > In dm_sm_metadata_create we temporarily change the dm_space_map > operations from ops, whose destroy function deallocates the > sm_metadata, to bootstrap_ops, whose destroy function doesn't. If we > fail in dm_ll_new_metadata or sm_ll_extend, we exit back to > dm_tm_create_internal, which calls dm_sm_destroy with the intention > of freeing the sm_metadata, but it doesn't. > > This patch sets the dm_space_map operations back to ops if > dm_sm_metadata_create fails when it is set to bootstrap_ops. > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > --- > drivers/md/persistent-data/dm-space-map-metadata.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/drivers/md/persistent-data/dm-space-map-metadata.c b/drivers/md/persistent-data/dm-space-map-metadata.c > index 7e44005..20557e2 100644 > --- a/drivers/md/persistent-data/dm-space-map-metadata.c > +++ b/drivers/md/persistent-data/dm-space-map-metadata.c > @@ -775,17 +775,15 @@ int dm_sm_metadata_create(struct dm_space_map *sm, > memcpy(&smm->sm, &bootstrap_ops, sizeof(smm->sm)); > > r = sm_ll_new_metadata(&smm->ll, tm); > + if (!r) { > + if (nr_blocks > DM_SM_METADATA_MAX_BLOCKS) > + nr_blocks = DM_SM_METADATA_MAX_BLOCKS; > + r = sm_ll_extend(&smm->ll, nr_blocks); > + } > + memcpy(&smm->sm, &ops, sizeof(smm->sm)); > if (r) > return r; > > - if (nr_blocks > DM_SM_METADATA_MAX_BLOCKS) > - nr_blocks = DM_SM_METADATA_MAX_BLOCKS; > - r = sm_ll_extend(&smm->ll, nr_blocks); > - if (r) > - return r; > - > - memcpy(&smm->sm, &ops, sizeof(smm->sm)); > - > /* > * Now we need to update the newly created data structures with the > * allocated blocks that they were built from. > -- > 2.1.0 > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Staged for 4.10 inclusion with a revised header, see: https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.10&id=b34af3050734ba53a7afa4f3ab9fe9d6bbfbf802 On Thu, Dec 01 2016 at 5:58am -0500, Edward Thornber <thornber@redhat.com> wrote: > ack > > On Wed, Nov 30, 2016 at 05:56:14PM -0600, Benjamin Marzinski wrote: > > In dm_sm_metadata_create we temporarily change the dm_space_map > > operations from ops, whose destroy function deallocates the > > sm_metadata, to bootstrap_ops, whose destroy function doesn't. If we > > fail in dm_ll_new_metadata or sm_ll_extend, we exit back to > > dm_tm_create_internal, which calls dm_sm_destroy with the intention > > of freeing the sm_metadata, but it doesn't. > > > > This patch sets the dm_space_map operations back to ops if > > dm_sm_metadata_create fails when it is set to bootstrap_ops. > > > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > > --- > > drivers/md/persistent-data/dm-space-map-metadata.c | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/md/persistent-data/dm-space-map-metadata.c b/drivers/md/persistent-data/dm-space-map-metadata.c > > index 7e44005..20557e2 100644 > > --- a/drivers/md/persistent-data/dm-space-map-metadata.c > > +++ b/drivers/md/persistent-data/dm-space-map-metadata.c > > @@ -775,17 +775,15 @@ int dm_sm_metadata_create(struct dm_space_map *sm, > > memcpy(&smm->sm, &bootstrap_ops, sizeof(smm->sm)); > > > > r = sm_ll_new_metadata(&smm->ll, tm); > > + if (!r) { > > + if (nr_blocks > DM_SM_METADATA_MAX_BLOCKS) > > + nr_blocks = DM_SM_METADATA_MAX_BLOCKS; > > + r = sm_ll_extend(&smm->ll, nr_blocks); > > + } > > + memcpy(&smm->sm, &ops, sizeof(smm->sm)); > > if (r) > > return r; > > > > - if (nr_blocks > DM_SM_METADATA_MAX_BLOCKS) > > - nr_blocks = DM_SM_METADATA_MAX_BLOCKS; > > - r = sm_ll_extend(&smm->ll, nr_blocks); > > - if (r) > > - return r; > > - > > - memcpy(&smm->sm, &ops, sizeof(smm->sm)); > > - > > /* > > * Now we need to update the newly created data structures with the > > * allocated blocks that they were built from. > > -- > > 2.1.0 > > > > -- > dm-devel mailing list > dm-devel@redhat.com > https://www.redhat.com/mailman/listinfo/dm-devel -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Thu, Dec 01, 2016 at 11:41:05AM -0500, Mike Snitzer wrote: > Staged for 4.10 inclusion with a revised header, see: > https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.10&id=b34af3050734ba53a7afa4f3ab9fe9d6bbfbf802 It's not urgent. In general if something goes wrong with a persistent data transaction we just abort and roll back. So there are a lot of error paths where we don't bother to repair metadata that's in the doomed transaction. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/drivers/md/persistent-data/dm-space-map-metadata.c b/drivers/md/persistent-data/dm-space-map-metadata.c index 7e44005..20557e2 100644 --- a/drivers/md/persistent-data/dm-space-map-metadata.c +++ b/drivers/md/persistent-data/dm-space-map-metadata.c @@ -775,17 +775,15 @@ int dm_sm_metadata_create(struct dm_space_map *sm, memcpy(&smm->sm, &bootstrap_ops, sizeof(smm->sm)); r = sm_ll_new_metadata(&smm->ll, tm); + if (!r) { + if (nr_blocks > DM_SM_METADATA_MAX_BLOCKS) + nr_blocks = DM_SM_METADATA_MAX_BLOCKS; + r = sm_ll_extend(&smm->ll, nr_blocks); + } + memcpy(&smm->sm, &ops, sizeof(smm->sm)); if (r) return r; - if (nr_blocks > DM_SM_METADATA_MAX_BLOCKS) - nr_blocks = DM_SM_METADATA_MAX_BLOCKS; - r = sm_ll_extend(&smm->ll, nr_blocks); - if (r) - return r; - - memcpy(&smm->sm, &ops, sizeof(smm->sm)); - /* * Now we need to update the newly created data structures with the * allocated blocks that they were built from.
In dm_sm_metadata_create we temporarily change the dm_space_map operations from ops, whose destroy function deallocates the sm_metadata, to bootstrap_ops, whose destroy function doesn't. If we fail in dm_ll_new_metadata or sm_ll_extend, we exit back to dm_tm_create_internal, which calls dm_sm_destroy with the intention of freeing the sm_metadata, but it doesn't. This patch sets the dm_space_map operations back to ops if dm_sm_metadata_create fails when it is set to bootstrap_ops. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> --- drivers/md/persistent-data/dm-space-map-metadata.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)