[PATCH] md: fix update super 1.0 on rdev size change

[PATCH] md: fix update super 1.0 on rdev size change

[markus]

From: Markus Hochholdinger <markus@hochholdinger.net>

The superblock of version 1.0 doesn't get moved to the new position on a
device size change. This leads to a rdev without a superblock on a known
position, the raid can't be re-assembled.

Fixes: commit d9c0fa509eaf ("md: fix max sectors calculation for super 1.0")
---
 drivers/md/md.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 6c0c3d0d905a..ad968cfc883d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2193,6 +2193,7 @@ super_1_rdev_size_change(struct md_rdev *rdev, sector_t num_sectors)
 
 		if (!num_sectors || num_sectors > max_sectors)
 			num_sectors = max_sectors;
+		rdev->sb_start = sb_start;
 	}
 	sb = page_address(rdev->sb_page);
 	sb->data_size = cpu_to_le64(num_sectors);
-- 
2.30.2

Re: [PATCH] md: fix update super 1.0 on rdev size change

[Xiao Ni]

Hi Markus

The sb_start doesn't change in function super_1_rdev_size_change. For super1.0
the super start is always at a fixed position. Is there a possibility
the disk size
changes? sb_start is calculated based on i_size_read(rdev->bdev->bd_inode).

By the way, can you reproduce this problem? If so, could you share
your test steps?

Regards
Xiao

On Fri, Nov 12, 2021 at 10:29 PM <markus@hochholdinger.net> wrote:
>
> From: Markus Hochholdinger <markus@hochholdinger.net>
>
> The superblock of version 1.0 doesn't get moved to the new position on a
> device size change. This leads to a rdev without a superblock on a known
> position, the raid can't be re-assembled.
>
> Fixes: commit d9c0fa509eaf ("md: fix max sectors calculation for super 1.0")
> ---
>  drivers/md/md.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 6c0c3d0d905a..ad968cfc883d 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -2193,6 +2193,7 @@ super_1_rdev_size_change(struct md_rdev *rdev, sector_t num_sectors)
>
>                 if (!num_sectors || num_sectors > max_sectors)
>                         num_sectors = max_sectors;
> +               rdev->sb_start = sb_start;
>         }
>         sb = page_address(rdev->sb_page);
>         sb->data_size = cpu_to_le64(num_sectors);
> --
> 2.30.2
>

Re: [PATCH] md: fix update super 1.0 on rdev size change

[Markus Hochholdinger]

Hi Xiao,

for 1.0 the super block is at the end of the device, out of "man mdadm":
    -e, --metadata=
              [..]
                     The different sub-versions
                     store the superblock at different locations  on  the  de-
                     vice, either at the end (for 1.0), at the start (for 1.1)
                     or 4K from the start (for 1.2).   "1"  is  equivalent  to
                     "1.2"  (the commonly preferred 1.x format).  "default" is
                     equivalent to "1.2".
(Because of other reasons, we have intentionally choosen the superblock at the 
end of the device.)

We change the device size of raid1 arrays, which are inside a VM, on a regular 
basis. And afterwards we grow the raid1 while the raid1 is online. Therefore, 
the superblock has to be moved.
This is very neat, because we can grow the raid1 and the filesystem size in a 
very short time frame and don't have to rebuild the raid1 twice (remove one 
device, resize and add with full rebuild because the old superblock is 
somewhere inbetween, then the same for the other device) before we can grow 
the raid1 and the filesystem.
If this explanation is not enough why we need this feature, I can explain in 
more detail why someone would do the software raid1 within a VM if you like.

As I understand, if the superblock isn't moved and we have grown the array and 
the filesystem on it, the superblock will now be updated inbetween the 
filesystem and may corrupt the filesystem and data.

Funny thing, after both devices are resized, the raid1 is still online and the 
grow does work. But afterwards, one can't do anything with the raid1, you get 
errors about the superblock, e.g. mdadm -D .. works, but mdadm -E .. for both 
devices doesn't. You can remove one device from the raid1, but you can't add 
it anymore, mdadm --add .. says: "mdadm: cannot load array metadata from /dev/
md0". And you can't re-assemble the raid1 after it is stopped.

I can reproduce this: With kernel version <= 5.8.18 the above works as 
expected. Since kernel version 5.9.x it doesn't anymore.
I tested this patch with kernel 5.15.1 and 5.10.46 and the above works again.


Here is a minimal setup to test this (but in real life we use it in a VM with 
virtual disks which can be resized online):
# truncate -s 1G /var/tmp/rdev1
# truncate -s 1G /var/tmp/rdev2
# losetup -f /var/tmp/rdev1
# losetup -f /var/tmp/rdev2
# losetup -j /var/tmp/rdev1
/dev/loop0: [2304]:786663 (/var/tmp/rdev1)
# losetup -j /var/tmp/rdev2
/dev/loop1: [2304]:788462 (/var/tmp/rdev2)
# mdadm --create --assume-clean /dev/md9 --metadata=1.0 --level=1 --raid-
disks=2 /dev/loop0 /dev/loop1
mdadm: array /dev/md9 started.
# mdadm -E /dev/loop0
/dev/loop0:
          Magic : a92b4efc
        Version : 1.0
    Feature Map : 0x0
[..]
# # grow the first loop device by 100MB
# dd if=/dev/zero bs=1M count=100 >> /var/tmp/rdev1
100+0 records in
100+0 records out
104857600 bytes (105 MB, 100 MiB) copied, 0.0960313 s, 1.1 GB/s
# losetup -c /dev/loop0

### with kernel <= 5.8.18 ###
# mdadm -E /dev/loop0
mdadm: No md superblock detected on /dev/loop0.
# echo 0 > /sys/block/md9/md/rd0/size
# mdadm -E /dev/loop0
/dev/loop0:
          Magic : a92b4efc
        Version : 1.0
    Feature Map : 0x0
[..]
# 

### with kernel >= 5.9.x ###
# mdadm -E /dev/loop0
mdadm: No md superblock detected on /dev/loop0.
# echo 0 > /sys/block/md9/md/rd0/size
# mdadm -E /dev/loop0
mdadm: No md superblock detected on /dev/loop0.
# 


Regards,
Markus

Am Montag, 15. November 2021, 10:47:12 CET schrieb Xiao Ni:
> The sb_start doesn't change in function super_1_rdev_size_change. For
> super1.0 the super start is always at a fixed position. Is there a
> possibility the disk size
> changes? sb_start is calculated based on i_size_read(rdev->bdev->bd_inode).
> 
> By the way, can you reproduce this problem? If so, could you share
> your test steps?
> 
> Regards
> Xiao
> 
> On Fri, Nov 12, 2021 at 10:29 PM <markus@hochholdinger.net> wrote:
> > From: Markus Hochholdinger <markus@hochholdinger.net>
> > 
> > The superblock of version 1.0 doesn't get moved to the new position on a
> > device size change. This leads to a rdev without a superblock on a known
> > position, the raid can't be re-assembled.
> > 
> > Fixes: commit d9c0fa509eaf ("md: fix max sectors calculation for super
> > 1.0") ---
> > 
> >  drivers/md/md.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index 6c0c3d0d905a..ad968cfc883d 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -2193,6 +2193,7 @@ super_1_rdev_size_change(struct md_rdev *rdev,
> > sector_t num_sectors)> 
> >                 if (!num_sectors || num_sectors > max_sectors)
> >                 
> >                         num_sectors = max_sectors;
> > 
> > +               rdev->sb_start = sb_start;
> > 
> >         }
> >         sb = page_address(rdev->sb_page);
> >         sb->data_size = cpu_to_le64(num_sectors);
> > 
> > --
> > 2.30.2

Re: [PATCH] md: fix update super 1.0 on rdev size change

[Wol]

On 15/11/2021 18:39, Markus Hochholdinger wrote:
> (Because of other reasons, we have intentionally choosen the superblock at the
> end of the device.)
> 
> We change the device size of raid1 arrays, which are inside a VM, on a regular
> basis. And afterwards we grow the raid1 while the raid1 is online. Therefore,
> the superblock has to be moved.

A perfect example of this (although I can't see myself growing the 
partitions in this case) is a mirrored /boot.

Superblock 1.0 means that anything can read the partition without 
needing to know about raid. One less thing to go wrong. And I can raid-1 
an EFI partition - I just need to make sure I only modify it from within 
linux so it gets mirrored across both drives. I would want that as 
protection against my primary drive failing - then my EFI partition is 
mirrored letting me boot from the secondary.

Cheers,
Wol

Re: [PATCH] md: fix update super 1.0 on rdev size change

[Xiao Ni]

On Tue, Nov 16, 2021 at 4:07 AM Wol <antlists@youngman.org.uk> wrote:
>
> On 15/11/2021 18:39, Markus Hochholdinger wrote:
> > (Because of other reasons, we have intentionally choosen the superblock at the
> > end of the device.)
> >
> > We change the device size of raid1 arrays, which are inside a VM, on a regular
> > basis. And afterwards we grow the raid1 while the raid1 is online. Therefore,
> > the superblock has to be moved.
>
> A perfect example of this (although I can't see myself growing the
> partitions in this case) is a mirrored /boot.
>
> Superblock 1.0 means that anything can read the partition without
> needing to know about raid. One less thing to go wrong. And I can raid-1
> an EFI partition - I just need to make sure I only modify it from within
> linux so it gets mirrored across both drives. I would want that as
> protection against my primary drive failing - then my EFI partition is
> mirrored letting me boot from the secondary.

Hi Wol

Thanks for the case. If I understand right, this is a case of using
super1.0 right?
Because before boot, it can't know this is a raid device. So it can't read data
if the raid is created with superblock 1.1 and 1.2. Only super1.0 can be used,
Because the data is stored at start of member disk when using super1.0. The
system can read partition table even they don't know it's a raid device leg.

Regards
Xiao

Re: [PATCH] md: fix update super 1.0 on rdev size change

[Xiao Ni]

On Tue, Nov 16, 2021 at 2:39 AM Markus Hochholdinger
<Markus@hochholdinger.net> wrote:
>
> Hi Xiao,
>
> for 1.0 the super block is at the end of the device, out of "man mdadm":
>     -e, --metadata=
>               [..]
>                      The different sub-versions
>                      store the superblock at different locations  on  the  de-
>                      vice, either at the end (for 1.0), at the start (for 1.1)
>                      or 4K from the start (for 1.2).   "1"  is  equivalent  to
>                      "1.2"  (the commonly preferred 1.x format).  "default" is
>                      equivalent to "1.2".
> (Because of other reasons, we have intentionally choosen the superblock at the
> end of the device.)
>
> We change the device size of raid1 arrays, which are inside a VM, on a regular
> basis. And afterwards we grow the raid1 while the raid1 is online. Therefore,
> the superblock has to be moved.

I c. Thanks for giving the test case.

> This is very neat, because we can grow the raid1 and the filesystem size in a
> very short time frame and don't have to rebuild the raid1 twice (remove one
> device, resize and add with full rebuild because the old superblock is
> somewhere inbetween, then the same for the other device) before we can grow
> the raid1 and the filesystem.
> If this explanation is not enough why we need this feature, I can explain in
> more detail why someone would do the software raid1 within a VM if you like.

It's enough. But could you talk more about the reason why create a
raid1 in a vm?
I want to know more scenarios that use raid devices.

>
> As I understand, if the superblock isn't moved and we have grown the array and
> the filesystem on it, the superblock will now be updated inbetween the
> filesystem and may corrupt the filesystem and data.
>
> Funny thing, after both devices are resized, the raid1 is still online and the
> grow does work. But afterwards, one can't do anything with the raid1, you get
> errors about the superblock, e.g. mdadm -D .. works, but mdadm -E .. for both
> devices doesn't. You can remove one device from the raid1, but you can't add
> it anymore, mdadm --add .. says: "mdadm: cannot load array metadata from /dev/
> md0". And you can't re-assemble the raid1 after it is stopped.

mdadm -D reads information from files under /sys/block/md. mdadm -E reads data
from disk. So one works and the other doesn't. And in kernel space, it
doesn't update
the superblock offset, and it still reads superblock from the old
position. But in userspace
it calculates the superblock position based on the disk size. It's in
a mess now.

>
> I can reproduce this: With kernel version <= 5.8.18 the above works as
> expected. Since kernel version 5.9.x it doesn't anymore.
> I tested this patch with kernel 5.15.1 and 5.10.46 and the above works again.
>
>
> Here is a minimal setup to test this (but in real life we use it in a VM with
> virtual disks which can be resized online):
> # truncate -s 1G /var/tmp/rdev1
> # truncate -s 1G /var/tmp/rdev2
> # losetup -f /var/tmp/rdev1
> # losetup -f /var/tmp/rdev2
> # losetup -j /var/tmp/rdev1
> /dev/loop0: [2304]:786663 (/var/tmp/rdev1)
> # losetup -j /var/tmp/rdev2
> /dev/loop1: [2304]:788462 (/var/tmp/rdev2)
> # mdadm --create --assume-clean /dev/md9 --metadata=1.0 --level=1 --raid-
> disks=2 /dev/loop0 /dev/loop1
> mdadm: array /dev/md9 started.
> # mdadm -E /dev/loop0
> /dev/loop0:
>           Magic : a92b4efc
>         Version : 1.0
>     Feature Map : 0x0
> [..]
> # # grow the first loop device by 100MB
> # dd if=/dev/zero bs=1M count=100 >> /var/tmp/rdev1
> 100+0 records in
> 100+0 records out
> 104857600 bytes (105 MB, 100 MiB) copied, 0.0960313 s, 1.1 GB/s
> # losetup -c /dev/loop0
>
> ### with kernel <= 5.8.18 ###
> # mdadm -E /dev/loop0
> mdadm: No md superblock detected on /dev/loop0.
> # echo 0 > /sys/block/md9/md/rd0/size
> # mdadm -E /dev/loop0
> /dev/loop0:
>           Magic : a92b4efc
>         Version : 1.0
>     Feature Map : 0x0
> [..]
> #
>
> ### with kernel >= 5.9.x ###
> # mdadm -E /dev/loop0
> mdadm: No md superblock detected on /dev/loop0.
> # echo 0 > /sys/block/md9/md/rd0/size
> # mdadm -E /dev/loop0
> mdadm: No md superblock detected on /dev/loop0.
> #

Thanks again for those detail steps.
Xiao

Re: [PATCH] md: fix update super 1.0 on rdev size change

[Markus Hochholdinger]

Hi Xiao,

Am Dienstag, 16. November 2021, 08:53:44 CET schrieb Xiao Ni:
> On Tue, Nov 16, 2021 at 2:39 AM Markus Hochholdinger
[..]
> > This is very neat, because we can grow the raid1 and the filesystem size
> > in a very short time frame and don't have to rebuild the raid1 twice
> > (remove one device, resize and add with full rebuild because the old
> > superblock is somewhere inbetween, then the same for the other device)
> > before we can grow the raid1 and the filesystem.
> > If this explanation is not enough why we need this feature, I can explain
> > in more detail why someone would do the software raid1 within a VM if you
> > like.
> It's enough. But could you talk more about the reason why create a
> raid1 in a vm?
> I want to know more scenarios that use raid devices.

sure. If you don't have the money for a big redundant storage, with multipath 
and fibre and so on you can use just two or more cheap servers (redundant array 
of inexpensive servers) which provide logical volumes of their local (maybe 
non redundant) disks as a iSCSI LUN to the other servers.
Now you build a VM by using two such volumes from different servers and do the 
raid1 inside the VM. If one server fails, there is still the other server who 
has the data. If the volumes are accessible on each hypervisor host by the 
same path you can live migrate your VMs freely between these hosts.
You have a shared nothing architecture, the only thing you have to take care 
of is, that a VM is only running once. You don't need heartbeat/pacemaker/.. 
or any other cluster software to take care about split brain situations within 
the storage. The RAID1 takes care about this inside the VM.

The raid1 bitmap helps a lot if you want to take one server into maintenance 
and re-add it afterwards, because only the changes on the raid1 have to be 
resynced.

By using metadata 1.0, the super block is at the end on the logical volumes 
and you can easily do a snpashot of the logical volumes and mount the snapshot 
for backups without calculating or guessing an offset.

We are using this setup for our customers since 2006 in production. The bitmap 
helped a lot with speeding up the resync after a server goes temporary off.
But growing was a pain, because you had to resync twice with the grown volumes 
before you could resize the raid1 and the filesystem.
Since the patch https://marc.info/?l=linux-raid&m=121455006810540&w=2 from 
2008 it was possible to grow a raid1 (and the underlaying disks) without full 
resync of the raid1 and this is working for me since at least kernel 2.6.32.


> > As I understand, if the superblock isn't moved and we have grown the array
> > and the filesystem on it, the superblock will now be updated inbetween
> > the filesystem and may corrupt the filesystem and data.
> > Funny thing, after both devices are resized, the raid1 is still online and
> > the grow does work. But afterwards, one can't do anything with the raid1,
> > you get errors about the superblock, e.g. mdadm -D .. works, but mdadm -E
> > .. for both devices doesn't. You can remove one device from the raid1,
> > but you can't add it anymore, mdadm --add .. says: "mdadm: cannot load
> > array metadata from /dev/ md0". And you can't re-assemble the raid1 after
> > it is stopped.
> mdadm -D reads information from files under /sys/block/md. mdadm -E reads
> data from disk. So one works and the other doesn't. And in kernel space, it
> doesn't update
> the superblock offset, and it still reads superblock from the old
> position. But in userspace
> it calculates the superblock position based on the disk size. It's in
> a mess now.

I see. Maybe some trigger to  /sys/block/mdX/.. could help to sync from kernel 
to disk and the other way round? But this isn't thought out.

[..]
> Thanks again for those detail steps.

Thank you for your time.

Regards,
Markus

Re: [PATCH] md: fix update super 1.0 on rdev size change

[Wols Lists]

On 16/11/2021 09:28, Markus Hochholdinger wrote:
> By using metadata 1.0, the super block is at the end on the logical volumes
> and you can easily do a snpashot of the logical volumes and mount the snapshot
> for backups without calculating or guessing an offset.

Dunno whether now is the time, but if you're moving superblocks, can we 
think about (a) moving superblocks to convert between 1.0, 1.1 and 1.2, 
and (b) possibly leaving backup copies lying around.

As I say, now might not to be the time to do it, but if you're thinking 
about it, an easy way to do it might just hit you in the face, and you 
won't leave any landmines if someone comes along later to do it.

(As maintainer of the raid wiki, backup superblocks assisting raid 
recovery might be a VERY useful tool !!!)

Cheers,
Wol

Re: [PATCH] md: fix update super 1.0 on rdev size change

[Xiao Ni]

On Fri, Nov 12, 2021 at 10:29 PM <markus@hochholdinger.net> wrote:
>
> From: Markus Hochholdinger <markus@hochholdinger.net>
>
> The superblock of version 1.0 doesn't get moved to the new position on a
> device size change. This leads to a rdev without a superblock on a known
> position, the raid can't be re-assembled.
>
> Fixes: commit d9c0fa509eaf ("md: fix max sectors calculation for super 1.0")
> ---
>  drivers/md/md.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 6c0c3d0d905a..ad968cfc883d 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -2193,6 +2193,7 @@ super_1_rdev_size_change(struct md_rdev *rdev, sector_t num_sectors)
>
>                 if (!num_sectors || num_sectors > max_sectors)
>                         num_sectors = max_sectors;
> +               rdev->sb_start = sb_start;
>         }
>         sb = page_address(rdev->sb_page);
>         sb->data_size = cpu_to_le64(num_sectors);
> --
> 2.30.2
>

Reviewd-by: Xiao Ni <xni@redhat.com>

Re: [PATCH] md: fix update super 1.0 on rdev size change

[Paul Menzel]

Dear Markus,


Thank you for your patch.


Am 12.11.21 um 15:28 schrieb markus@hochholdinger.net:
> From: Markus Hochholdinger <markus@hochholdinger.net>
> 
> The superblock of version 1.0 doesn't get moved to the new position on a
> device size change. This leads to a rdev without a superblock on a known
> position, the raid can't be re-assembled.

> Fixes: commit d9c0fa509eaf ("md: fix max sectors calculation for super 1.0")

I think it’s common to not write *commit* in there, but just the short 
hash. `scripts/checkpatch.pl` does not mention that, but it mentions:

     ERROR: Missing Signed-off-by: line(s)

     total: 1 errors, 0 warnings, 7 lines checked

> ---
>   drivers/md/md.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 6c0c3d0d905a..ad968cfc883d 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -2193,6 +2193,7 @@ super_1_rdev_size_change(struct md_rdev *rdev, sector_t num_sectors)
>   
>   		if (!num_sectors || num_sectors > max_sectors)
>   			num_sectors = max_sectors;
> +		rdev->sb_start = sb_start;
>   	}
>   	sb = page_address(rdev->sb_page);
>   	sb->data_size = cpu_to_le64(num_sectors);
> 


Kind regards,

Paul

Re: [PATCH] md: fix update super 1.0 on rdev size change

[Markus Hochholdinger]

Hi Paul,

Am Dienstag, 16. November 2021, 10:24:17 CET schrieb Paul Menzel:
> Dear Markus,
> Thank you for your patch.

I have to thank you.


> Am 12.11.21 um 15:28 schrieb markus@hochholdinger.net:
> > From: Markus Hochholdinger <markus@hochholdinger.net>
> > The superblock of version 1.0 doesn't get moved to the new position on a
> > device size change. This leads to a rdev without a superblock on a known
> > position, the raid can't be re-assembled.
> > Fixes: commit d9c0fa509eaf ("md: fix max sectors calculation for super
> > 1.0")
> I think it’s common to not write *commit* in there, but just the short
> hash. `scripts/checkpatch.pl` does not mention that, but it mentions:
>      ERROR: Missing Signed-off-by: line(s)
>      total: 1 errors, 0 warnings, 7 lines checked

I'm sorry, this was my first patch request against the linux kernel.
Within https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
tree/Documentation/process/submitting-patches.rst I read to sign off a patch if 
it is from me, but the line was there before. So I thought I don't have to 
sign it.

Should I do the patch request again with Signed-off information?


> >   drivers/md/md.c | 1 +
> >   1 file changed, 1 insertion(+)
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index 6c0c3d0d905a..ad968cfc883d 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -2193,6 +2193,7 @@ super_1_rdev_size_change(struct md_rdev *rdev,
> > sector_t num_sectors)> 
> >   		if (!num_sectors || num_sectors > max_sectors)
> >   			num_sectors = max_sectors;
> > +		rdev->sb_start = sb_start;
> >   	}
> >   	sb = page_address(rdev->sb_page);
> >   	sb->data_size = cpu_to_le64(num_sectors);
> Kind regards,
> Paul

Regards,
Markus

Re: [PATCH] md: fix update super 1.0 on rdev size change

[Paul Menzel]

Dear Markus,


Am 16.11.21 um 10:44 schrieb Markus Hochholdinger:

> Am Dienstag, 16. November 2021, 10:24:17 CET schrieb Paul Menzel:

[…]

>> Am 12.11.21 um 15:28 schrieb markus@hochholdinger.net:
>>> From: Markus Hochholdinger <markus@hochholdinger.net>
>>> The superblock of version 1.0 doesn't get moved to the new position on a
>>> device size change. This leads to a rdev without a superblock on a known
>>> position, the raid can't be re-assembled.
>>> Fixes: commit d9c0fa509eaf ("md: fix max sectors calculation for super
>>> 1.0")
>> I think it’s common to not write *commit* in there, but just the short
>> hash. `scripts/checkpatch.pl` does not mention that, but it mentions:
>>       ERROR: Missing Signed-off-by: line(s)
>>       total: 1 errors, 0 warnings, 7 lines checked
> 
> I'm sorry, this was my first patch request against the linux kernel.

Awesome. We all started with that.

> Within https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
> tree/Documentation/process/submitting-patches.rst I read to sign off a patch if
> it is from me, but the line was there before. So I thought I don't have to
> sign it.

No, even reverts have to be signed off.

> Should I do the patch request again with Signed-off information?

Yes, please do, and maybe amend the commit message, to mention, that the 
line was removed by mistake, and is added back.

You can create a v2 easily with the `git send-email` switch below:

     -v <n>, --reroll-count=<n>

You can also add the Reviewed-by lines you already got.

[…]


Kind regards,

Paul