| Message ID | 20220228141354.1091687-1-luomeng12@huawei.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
| Series | [PTACH] Revert "dm space maps: don't reset space map allocation cursor when committing" | expand |
Because thin-pool is storage over-commitment, one of the following scenarios exists: constantly create and delete file, then the search doesn't hit the end of the metadata device, but ramdisk hits the end (not support discard). So the cursor doesn't reset. 在 2022/2/28 23:37, Mike Snitzer 写道: > What you're saying doesn't make any sense. Especially when you > consider this last part of the commit message: > "Fix these issues by leaving the cursor alone, only resetting when the > search hits the end of the metadata device." -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
I'm having trouble understanding your issue. Does your ramdisk only allocate backing memory on demand? (ie. is the ramdisk itself a thinly provisioned device?). If so, not supporting discard seems to be the problem. Thinp makes no promises about where it will allocate your data. If you write a file, delete it, discard and then rewrite the file there is no guarantee that the file will be written to the same location. On Tue, Mar 1, 2022 at 2:08 PM luomeng <luomeng12@huawei.com> wrote: > Because thin-pool is storage over-commitment, one of the following > scenarios exists: constantly create and delete file, then the search > doesn't hit the end of the metadata device, but ramdisk hits the end > (not support discard). So the cursor doesn't reset. > > 在 2022/2/28 23:37, Mike Snitzer 写道: > > What you're saying doesn't make any sense. Especially when you > > consider this last part of the commit message: > > "Fix these issues by leaving the cursor alone, only resetting when the > > search hits the end of the metadata device." > > -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
Yes, ramdisk itself is a thinly provisioned device. But We can't restrict what disks users use. So if disk doesn't support discard, it will cause this problem. 在 2022/3/2 0:06, Edward Thornber 写道: > I'm having trouble understanding your issue. Does your ramdisk only > allocate backing memory on demand? (ie. is the ramdisk itself a thinly > provisioned device?). If so, not supporting discard seems to be the > problem. > > Thinp makes no promises about where it will allocate your data. If you > write a file, delete it, discard and then rewrite the file there is no > guarantee that the file will be written to the same location. > > On Tue, Mar 1, 2022 at 2:08 PM luomeng <luomeng12@huawei.com > <mailto:luomeng12@huawei.com>> wrote: > > Because thin-pool is storage over-commitment, one of the following > scenarios exists: constantly create and delete file, then the search > doesn't hit the end of the metadata device, but ramdisk hits the end > (not support discard). So the cursor doesn't reset. > > 在 2022/2/28 23:37, Mike Snitzer 写道: > > What you're saying doesn't make any sense. Especially when you > > consider this last part of the commit message: > > "Fix these issues by leaving the cursor alone, only resetting > when the > > search hits the end of the metadata device." > -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
diff --git a/drivers/md/persistent-data/dm-space-map-disk.c b/drivers/md/persistent-data/dm-space-map-disk.c index d0a8d5e73c28..17afbebd9244 100644 --- a/drivers/md/persistent-data/dm-space-map-disk.c +++ b/drivers/md/persistent-data/dm-space-map-disk.c @@ -134,14 +134,6 @@ static int sm_disk_new_block(struct dm_space_map *sm, dm_block_t *b) * Any block we allocate has to be free in both the old and current ll. */ r = sm_ll_find_common_free_block(&smd->old_ll, &smd->ll, smd->begin, smd->ll.nr_blocks, b); - if (r == -ENOSPC) { - /* - * There's no free block between smd->begin and the end of the metadata device. - * We search before smd->begin in case something has been freed. - */ - r = sm_ll_find_common_free_block(&smd->old_ll, &smd->ll, 0, smd->begin, b); - } - if (r) return r; @@ -164,6 +156,7 @@ static int sm_disk_commit(struct dm_space_map *sm) return r; memcpy(&smd->old_ll, &smd->ll, sizeof(smd->old_ll)); + smd->begin = 0; smd->nr_allocated_this_transaction = 0; return 0; diff --git a/drivers/md/persistent-data/dm-space-map-metadata.c b/drivers/md/persistent-data/dm-space-map-metadata.c index 392ae26134a4..42920e930dbd 100644 --- a/drivers/md/persistent-data/dm-space-map-metadata.c +++ b/drivers/md/persistent-data/dm-space-map-metadata.c @@ -453,14 +453,6 @@ static int sm_metadata_new_block_(struct dm_space_map *sm, dm_block_t *b) * Any block we allocate has to be free in both the old and current ll. */ r = sm_ll_find_common_free_block(&smm->old_ll, &smm->ll, smm->begin, smm->ll.nr_blocks, b); - if (r == -ENOSPC) { - /* - * There's no free block between smm->begin and the end of the metadata device. - * We search before smm->begin in case something has been freed. - */ - r = sm_ll_find_common_free_block(&smm->old_ll, &smm->ll, 0, smm->begin, b); - } - if (r) return r; @@ -512,6 +504,7 @@ static int sm_metadata_commit(struct dm_space_map *sm) return r; memcpy(&smm->old_ll, &smm->ll, sizeof(smm->old_ll)); + smm->begin = 0; smm->allocated_this_transaction = 0; return 0;
This reverts commit 5faafc77f7de69147d1e818026b9a0cbf036a7b2. This commit 5faafc77f7de ("dm space maps: don't reset space map allocation cursor when committing") change the way to find free block. But when use ramdisk(not support discard) for thin-pool,and storage over-commitment. Then constantly create and delete file, can find block in thin-pool, but can't find block in ramdisk. So need revert this patch. Fixes: 5faafc77f7de ("dm space maps: don't reset space map allocation cursor when committing") Signed-off-by: Luo Meng <luomeng12@huawei.com> --- drivers/md/persistent-data/dm-space-map-disk.c | 9 +-------- drivers/md/persistent-data/dm-space-map-metadata.c | 9 +-------- 2 files changed, 2 insertions(+), 16 deletions(-)