* [PATCH] dm cache metadata: Fix loading discard bitset
@ 2019-04-17 14:19 Nikos Tsironis
2019-04-17 14:35 ` Mike Snitzer
0 siblings, 1 reply; 3+ messages in thread
From: Nikos Tsironis @ 2019-04-17 14:19 UTC (permalink / raw)
To: snitzer, agk, dm-devel; +Cc: ejt
Add missing dm_bitset_cursor_next() to properly advance the bitset
cursor.
Otherwise, the discarded state of all blocks is set according to the
discarded state of the first block.
Fixes: ae4a46a1f6 ("dm cache metadata: use bitset cursor api to load discard bitset")
Signed-off-by: Nikos Tsironis <ntsironis@arrikto.com>
---
drivers/md/dm-cache-metadata.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
index 6fc93834da44..151aa95775be 100644
--- a/drivers/md/dm-cache-metadata.c
+++ b/drivers/md/dm-cache-metadata.c
@@ -1167,11 +1167,18 @@ static int __load_discards(struct dm_cache_metadata *cmd,
if (r)
return r;
- for (b = 0; b < from_dblock(cmd->discard_nr_blocks); b++) {
+ for (b = 0; ; b++) {
r = fn(context, cmd->discard_block_size, to_dblock(b),
dm_bitset_cursor_get_value(&c));
if (r)
break;
+
+ if (b >= (from_dblock(cmd->discard_nr_blocks) - 1))
+ break;
+
+ r = dm_bitset_cursor_next(&c);
+ if (r)
+ break;
}
dm_bitset_cursor_end(&c);
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: dm cache metadata: Fix loading discard bitset
2019-04-17 14:19 [PATCH] dm cache metadata: Fix loading discard bitset Nikos Tsironis
@ 2019-04-17 14:35 ` Mike Snitzer
2019-04-17 14:54 ` Nikos Tsironis
0 siblings, 1 reply; 3+ messages in thread
From: Mike Snitzer @ 2019-04-17 14:35 UTC (permalink / raw)
To: Nikos Tsironis; +Cc: dm-devel, ejt, agk
On Wed, Apr 17 2019 at 10:19am -0400,
Nikos Tsironis <ntsironis@arrikto.com> wrote:
> Add missing dm_bitset_cursor_next() to properly advance the bitset
> cursor.
>
> Otherwise, the discarded state of all blocks is set according to the
> discarded state of the first block.
>
> Fixes: ae4a46a1f6 ("dm cache metadata: use bitset cursor api to load discard bitset")
> Signed-off-by: Nikos Tsironis <ntsironis@arrikto.com>
Nice catch, I'll obviously mark this for stable@ too.
One comment below.
> ---
> drivers/md/dm-cache-metadata.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
> index 6fc93834da44..151aa95775be 100644
> --- a/drivers/md/dm-cache-metadata.c
> +++ b/drivers/md/dm-cache-metadata.c
> @@ -1167,11 +1167,18 @@ static int __load_discards(struct dm_cache_metadata *cmd,
> if (r)
> return r;
>
> - for (b = 0; b < from_dblock(cmd->discard_nr_blocks); b++) {
> + for (b = 0; ; b++) {
> r = fn(context, cmd->discard_block_size, to_dblock(b),
> dm_bitset_cursor_get_value(&c));
> if (r)
> break;
> +
> + if (b >= (from_dblock(cmd->discard_nr_blocks) - 1))
> + break;
Not understanding why you moved the conditional inside the loop, I'd
rather keep it in the for(). Or did you have some reason for this?
> +
> + r = dm_bitset_cursor_next(&c);
> + if (r)
> + break;
> }
>
> dm_bitset_cursor_end(&c);
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: dm cache metadata: Fix loading discard bitset
2019-04-17 14:35 ` Mike Snitzer
@ 2019-04-17 14:54 ` Nikos Tsironis
0 siblings, 0 replies; 3+ messages in thread
From: Nikos Tsironis @ 2019-04-17 14:54 UTC (permalink / raw)
To: Mike Snitzer; +Cc: dm-devel, ejt, agk
On 4/17/19 5:35 PM, Mike Snitzer wrote:
> On Wed, Apr 17 2019 at 10:19am -0400,
> Nikos Tsironis <ntsironis@arrikto.com> wrote:
>
>> Add missing dm_bitset_cursor_next() to properly advance the bitset
>> cursor.
>>
>> Otherwise, the discarded state of all blocks is set according to the
>> discarded state of the first block.
>>
>> Fixes: ae4a46a1f6 ("dm cache metadata: use bitset cursor api to load discard bitset")
>> Signed-off-by: Nikos Tsironis <ntsironis@arrikto.com>
>
> Nice catch, I'll obviously mark this for stable@ too.
>
> One comment below.
>
>> ---
>> drivers/md/dm-cache-metadata.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
>> index 6fc93834da44..151aa95775be 100644
>> --- a/drivers/md/dm-cache-metadata.c
>> +++ b/drivers/md/dm-cache-metadata.c
>> @@ -1167,11 +1167,18 @@ static int __load_discards(struct dm_cache_metadata *cmd,
>> if (r)
>> return r;
>>
>> - for (b = 0; b < from_dblock(cmd->discard_nr_blocks); b++) {
>> + for (b = 0; ; b++) {
>> r = fn(context, cmd->discard_block_size, to_dblock(b),
>> dm_bitset_cursor_get_value(&c));
>> if (r)
>> break;
>> +
>> + if (b >= (from_dblock(cmd->discard_nr_blocks) - 1))
>> + break;
>
> Not understanding why you moved the conditional inside the loop, I'd
> rather keep it in the for(). Or did you have some reason for this?
>
After processing the last block, i.e., b == (from_dblock(cmd->discard_nr_blocks) - 1),
dm_bitset_cursor_next() will return -ENODATA. It seemed to me simpler to move the
conditional inside the loop, rather than check explicitly for -ENODATA and handle
it differently, e.g., set r = 0, if r == -ENODATA.
Nikos.
>> +
>> + r = dm_bitset_cursor_next(&c);
>> + if (r)
>> + break;
>> }
>>
>> dm_bitset_cursor_end(&c);
>> --
>> 2.11.0
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-04-17 14:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-17 14:19 [PATCH] dm cache metadata: Fix loading discard bitset Nikos Tsironis
2019-04-17 14:35 ` Mike Snitzer
2019-04-17 14:54 ` Nikos Tsironis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.