Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230620210003.3818348-1-jbaron@akamai.com>
In-Reply-To: <20230620210003.3818348-1-jbaron@akamai.com>
From:   Song Liu <song@kernel.org>
Date:   Tue, 20 Jun 2023 14:39:57 -0700
Message-ID: <CAPhsuW4iJjC5=z0rZEOw5xJ5MdEXyA+9fgvS1NjWooDpuZ31_g@mail.gmail.com>
Subject: Re: raid0 + original layout + discard + different disk sizes => can
 corrupt data
To:     Jason Baron <jbaron@akamai.com>
Cc:     linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org,
        neilb@suse.de, mzhivich@akamai.com, dbanerje@akamai.com,
        jdike@akamai.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Jason,

On Tue, Jun 20, 2023 at 2:00=E2=80=AFPM Jason Baron <jbaron@akamai.com> wro=
te:
>
> Hi,
>
> SUMMARY
> =3D=3D=3D=3D=3D=3D=3D
>
> We've found that using raid0 with the 'original' layout and discard enabl=
ed
> with different disk sizes (such that at least two zones are created) can
> result in data corruption. This is due to the fact that the discard
> handling in 'raid0_handle_discard()' assumes the 'alternate' layout.
>
> More specifically, while multiple zones are necessary to create the
> corruption, the corruption may not occur with multiple zones if they
> layout in such a way the layout matches what the 'alternate' layout
> would have produced. Thus, not all raid0 devices with the 'original'
> layout, different size disks and discard enabled will encounter this
> corruption.
>
> BACKGROUND
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>
> The 3.14 kernel inadvertleny changed the raid0 disk layout for different
> size disks. Thus, running a pre-3.14 kernel and post-3.14 kernel on the
> same raid0 array could corrupt data. This lead to the creation of the
> 'original' layout (to match the pre-3.14 layout) and the 'alternate' layo=
ut
> (to match the post 3.14 layout) in the 5.4 kernel time frame and an optio=
n
> to tell the kernel which layout to use (since it couldn't be autodetected=
)
> [1]. The mdadm tool also added support for writing the layout to the raid=
0
> superblock which the kernel honors. The mdadm tool defaults to the origin=
al
> layout unless --layout is given (--layout=3Dalternate or --layout=3Dorigi=
nal).
>
> REPRODUCER (pseudocode)
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>
> The below steps were the simplest we found to consistently reproduced the
> corruption:
>
> 1. create raid0 array with different size disks using original layout
> 2. mkfs
> 3. mount -o discard
> 4. create lots of files
> 5. remove 1/2 the files
> 6. fstrim -a (or just the mount point for the raid0 array)
> 7. umount
> 8. fsck -fn /dev/md1
>
> Here is an example corruption, though we've seen many others:
>
> fsck from util-linux 2.34
> e2fsck 1.45.5 (07-Jan-2020)
> Pass 1: Checking inodes, blocks, and sizes
> Pass 2: Checking directory structure
> Directory inode 24557, block #0, offset 0: directory has no checksum.
> Fix? no
>
> Directory inode 24557, block #0, offset 0: directory corrupted
> Salvage? no
>
> e2fsck: aborted
>
> /dev/md1: ********** WARNING: Filesystem still has errors **********
>
>
> VULNERABLE VERSIONS
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>
> We've confirmed the corruption with the latest 6.4-rc7 kernel. Kernels th=
at
> support the 'original' layout could have this issue. The 'original' layou=
t
> was added to the 5.4+ kernels, although the 'original' layout may have be=
en
> backported to earlier kernels.
>
> Kernels versions from 3.14 to 5.4 that only have support for the 'alterna=
te'
> layout should be ok. I'm not sure about pre-3.14 kernels that only have t=
he
> 'original' layout. One would have to go back and see if they support
> discard and if that support is correct.
>
> WORKAROUNDS
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>
> Until a proper code fix is implemented in the kernel and updated, valid
> workarounds we have found are any 1 of the following:
>
> 1. disable discard and fstrim from running on the raid0 array
> 2. create new arrays only with the 'alternate' layout, the 'mdadm'
>    command understands mdadm --layout=3Dalternate
> 3. ensure there is only 1 raid0 zone (if disks are all the same size)
>
> That said, you may already have 'silent corruption' if data is not covere=
d
> by a checksum.
>
> FIXING
> =3D=3D=3D=3D=3D=3D=3D
>
> It may make sense to have the kernel just disable discard for raid0 with
> the 'original' layout, and update mdadm to default to 'alternate' and tre=
at
> the 'original' layout more as a deprecated format? I'm curious as to why
> mdadm is using 'original' layout as the default for new arrays?
>
> Alternatively, we could patch the 'original' layout and add proper discar=
d
> support.

Thanks for the detailed report and analysis!

We will look into fixing it.

Song

>
> THANKS
> =3D=3D=3D=3D=3D=3D
>
> Thanks to Michael Zhivich, Debabrata Banerjee and Jeff Dike for helping u=
ncover this issue.
>
> Thanks,
>
> -Jason
>
> [1] https://wiki.ubuntu.com/Kernel/Raid0LayoutMigration