From: Mikulas Patocka <[email protected]>
commit 7ee06ddc4038f936b0d4459d37a7d4d844fb03db upstream.
If an origin target has no snapshots, o->split_boundary is set to 0.
This causes BUG_ON(sectors <= 0) in block/bio.c:bio_split().
Fix this by initializing chunk_size, and in turn split_boundary, to
rounddown_pow_of_two(UINT_MAX) -- the largest power of two that fits
into "unsigned" type.
Reported-by: Michael Tokarev <[email protected]>
Tested-by: Michael Tokarev <[email protected]>
Cc: [email protected]
Signed-off-by: Mikulas Patocka <[email protected]>
Signed-off-by: Mike Snitzer <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/md/dm-snap.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -854,12 +854,11 @@ static int dm_add_exception(void *contex
static uint32_t __minimum_chunk_size(struct origin *o)
{
struct dm_snapshot *snap;
- unsigned chunk_size = 0;
+ unsigned chunk_size = rounddown_pow_of_two(UINT_MAX);
if (o)
list_for_each_entry(snap, &o->snapshots, list)
- chunk_size = min_not_zero(chunk_size,
- snap->store->chunk_size);
+ chunk_size = min(chunk_size, snap->store->chunk_size);
return (uint32_t) chunk_size;
}
Hi Greg
I'd like to ask you to drop this patch from all stable branches.
It causes regression with snapshot merging and the regression is much
worse than the bug that it fixes.
Mikulas
On Mon, 24 May 2021, Greg Kroah-Hartman wrote:
> From: Mikulas Patocka <[email protected]>
>
> commit 7ee06ddc4038f936b0d4459d37a7d4d844fb03db upstream.
>
> If an origin target has no snapshots, o->split_boundary is set to 0.
> This causes BUG_ON(sectors <= 0) in block/bio.c:bio_split().
>
> Fix this by initializing chunk_size, and in turn split_boundary, to
> rounddown_pow_of_two(UINT_MAX) -- the largest power of two that fits
> into "unsigned" type.
>
> Reported-by: Michael Tokarev <[email protected]>
> Tested-by: Michael Tokarev <[email protected]>
> Cc: [email protected]
> Signed-off-by: Mikulas Patocka <[email protected]>
> Signed-off-by: Mike Snitzer <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> drivers/md/dm-snap.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> --- a/drivers/md/dm-snap.c
> +++ b/drivers/md/dm-snap.c
> @@ -854,12 +854,11 @@ static int dm_add_exception(void *contex
> static uint32_t __minimum_chunk_size(struct origin *o)
> {
> struct dm_snapshot *snap;
> - unsigned chunk_size = 0;
> + unsigned chunk_size = rounddown_pow_of_two(UINT_MAX);
>
> if (o)
> list_for_each_entry(snap, &o->snapshots, list)
> - chunk_size = min_not_zero(chunk_size,
> - snap->store->chunk_size);
> + chunk_size = min(chunk_size, snap->store->chunk_size);
>
> return (uint32_t) chunk_size;
> }
>
>
On Tue, May 25, 2021 at 07:36:57AM -0400, Mikulas Patocka wrote:
> Hi Greg
>
> I'd like to ask you to drop this patch from all stable branches.
>
> It causes regression with snapshot merging and the regression is much
> worse than the bug that it fixes.
Sure, but is there a fix in Linus's branch for this that I should take
instead, or is it reverted in linux-next already?
thanks,
greg k-h
On Tue, 25 May 2021, Greg Kroah-Hartman wrote:
> On Tue, May 25, 2021 at 07:36:57AM -0400, Mikulas Patocka wrote:
> > Hi Greg
> >
> > I'd like to ask you to drop this patch from all stable branches.
> >
> > It causes regression with snapshot merging and the regression is much
> > worse than the bug that it fixes.
>
> Sure, but is there a fix in Linus's branch for this that I should take
> instead, or is it reverted in linux-next already?
>
> thanks,
>
> greg k-h
There is no fix now - but the bug that this fixed is minor (it fixes a
crash in a configuration that is never set by lvm2) - and the regression
is major (snapshot merging doesn't work).
I'll work with Mike on reverting it a providing a proper fix.
Mikulas