* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Mikael Abrahamsson @ 2015-05-26 8:06 UTC (permalink / raw)
To: Another Sillyname; +Cc: linux-raid
In-Reply-To: <CAOS+5GEBJ8dxF4s91dGmF7jx36LyNnu40UQZjLAmK2xVhqLAHw@mail.gmail.com>
On Tue, 26 May 2015, Another Sillyname wrote:
> Not bothered about raiding the swap thanks, my way will suffice as it
> gives me maximum flexibility and resilience......I'm not really
> performance driven on this project.
I don't see how running swap natively on the drives gives "maximum
resilience". Higher resilience is gained by running raid1 for swap.
--
Mikael Abrahamsson email: swmike@swm.pp.se
^ permalink raw reply
* Re: Installing Linux directly onto RAID6 Array...........
From: NeilBrown @ 2015-05-26 8:29 UTC (permalink / raw)
To: Another Sillyname; +Cc: Mikael Abrahamsson, linux-raid
In-Reply-To: <CAOS+5GEZN5d41tGxN+PP6S3ynSWnrXaNK+27rxJyXa-zw1KFaw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3954 bytes --]
On Sun, 24 May 2015 10:08:59 +0100 Another Sillyname
<anothersname@googlemail.com> wrote:
> I suspect you're correct in that I'll end up with the boot partitions
> being in RAID1 and the data in RAID6, however I am seriously
> considering having the boot in RAID6 as well...if I can integrate the
> mdadm.conf into the initramfs properly I can't see a reason not to do
> this?
mdadm.conf is largely a non-issue. You don't need an mdadm.conf to assemble
your array. All the raid configuration lives in the raid metadata.
All you need is for your initrd to know what device contains your root
filesystem (preferably by UUID) so that when mdadm finds that array, the
initrd code can mount it for you.
I believe that GRUB2 can load an initrd and kernel from a filesystem on an
mdraid device, but I don't know where the boot sector would load GRUB2 from.
md's v1.2 metadata leaves 4K at the start of each device. If GRUB2 fits in
there, then it could certainly load, assemble the RAID6, then pull the files
off your root filesystem. But I doubt it.
If GRUB tries to put the boot loader anywhere else, there is a good chance
that md could over-write it, as it believes that it owns all the space after
4K.
According to the documentation, GRUB2 either places the second stage in the
first 32K before the first partition, or in the filesystem at specific block
locations.
The first cannot work if md uses the whole device (works fine if md uses
partitions).
The second cannot work with RAID6 as the blocks are in locations on one
device. This only really work for RAID1.
So feel free to try, and do report any results, but I doubt you'll get it to
work reliably.
NeilBrown
>
> Had a look at the metadata=0.9 option but reading the info on mdadm
> metadata I think I'd prefer to have the metadata at the start of the
> drive, also it looks like metadata=1.2 has extra functionality that I
> may want to use later.
>
> On 24 May 2015 at 09:36, Mikael Abrahamsson <swmike@swm.pp.se> wrote:
> > On Sun, 24 May 2015, Another Sillyname wrote:
> >
> >> So I now have 5 partitions.
> >>
> >> a - bios_boot
> >> b - efi
> >> c - boot
> >> d - root
> >> e - swap
> >>
> >> I'll be adding one more when I'm happy this is working.
> >>
> >> f - home
> >>
> >> 3. Using the methods above I have now created a bootable fedora
> >> system, on a single drive in preparation to now RAID the required
> >> partitions. However my concern comes regarding the mdadm metadata,
> >> simplistically metadata=1.2 apparently writes it's superblock to 4k
> >> after the start of the device, this is exactly where my efi partition
> >> (b above) starts, so my concern is will this superblock overwrite or
> >> mess with my current partition table?
> >>
> >> 4. If the next stage works then I think what I'll actually end up
> >> doing is......
> >>
> >> scrub what I have now.
> >>
> >> create the arrays before running the Fedora Live installer (this
> >> assumes the installer will see /md[x] devices and allow them to be
> >> used to install to). Then incorporate the mdadm.conf data into the
> >> initramfs and regenerate initramfs.
> >>
> >> Ideas/Thoughts/Criticisms?
> >
> >
> > You don't want to run MD on the entire drive in this case, you most likely
> > want to create multiple RAID1 and RAID6 mirrors. RAID1 your boot, root and
> > swap, then run RAID6 on your home partition. Use use superblock type that
> > creates the superblock at the end for the RAID1 partitions.
> >
> > Also, you don't want to refer to "sda" when booting, you want to use
> > UUID=<uuid> in fstab, crypttab etc.
> >
> > --
> > Mikael Abrahamsson email: swmike@swm.pp.se
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH V3 08/11] mdadm: add the ability to change cluster name
From: Guoqing Jiang @ 2015-05-26 8:38 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, rgoldwyn
In-Reply-To: <20150525145310.6b6ff7f8@notabene.brown>
NeilBrown wrote:
> On Wed, 20 May 2015 11:20:40 +0800 Guoqing Jiang <gqjiang@suse.com> wrote:
>
>
>> To support change the cluster name, the commit do the followings:
>>
>> 1. extend original write_bitmap function for new scenario.
>> 2. add the scenarion to handle the modification of cluster's name
>> in write_bitmap1.
>> 3. make update_super1 can change the name in mdp_superblock_1.
>>
>
> You haven't documented --update=home-cluster in mdadm.8.in, or at
>
> fprintf(outf, "Valid --update options are:\n"
>
Sorry, I will add it.
> Also, I just realised that you are storing the cluster name in the array
> name. I don't think that is a clever idea.
> The cluster name can be 64 chars. The array name can only be 32.
>
>
Yes, you are correct. The array name is combined by homecluster and
homehost.
Generally, it doesn't have problem since people don't set the longer
name generally,
of course it is better to double check the length.
if strlen( homecluster + homehost) < 32
set the new name
else
tell the user new cluster is too long, return
> I think leave homehost and homecluster completely out of the array name when
> the array is clustered.
Could you please elaborate more about it? Does it mean add extra cluster
member in
superblock? Something like:
diff --git a/super1.c b/super1.c
index b949f5e..4ea1115 100644
--- a/super1.c
+++ b/super1.c
char set_name[32]; /* set and interpreted by user-space */
+ char cluster_name[64];
If so, the kernel code also need related modification.
Or just let the bitmap stores the clustername, and the set_name doesn't
need to
be change, then display the clustername from bitmap when run 'mdadm -E/D'.
Thanks,
Guoqing
^ permalink raw reply
* Re: raid5 reshape is stuck
From: Xiao Ni @ 2015-05-26 10:00 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <20150525135001.43d1083a@notabene.brown>
----- Original Message -----
> From: "NeilBrown" <neilb@suse.de>
> To: "Xiao Ni" <xni@redhat.com>
> Cc: linux-raid@vger.kernel.org
> Sent: Monday, May 25, 2015 11:50:01 AM
> Subject: Re: raid5 reshape is stuck
>
> On Thu, 21 May 2015 08:31:58 -0400 (EDT) Xiao Ni <xni@redhat.com> wrote:
>
> >
> >
> > ----- Original Message -----
> > > From: "Xiao Ni" <xni@redhat.com>
> > > To: "NeilBrown" <neilb@suse.de>
> > > Cc: linux-raid@vger.kernel.org
> > > Sent: Thursday, May 21, 2015 11:37:57 AM
> > > Subject: Re: raid5 reshape is stuck
> > >
> > >
> > >
> > > ----- Original Message -----
> > > > From: "NeilBrown" <neilb@suse.de>
> > > > To: "Xiao Ni" <xni@redhat.com>
> > > > Cc: linux-raid@vger.kernel.org
> > > > Sent: Thursday, May 21, 2015 7:48:37 AM
> > > > Subject: Re: raid5 reshape is stuck
> > > >
> > > > On Fri, 15 May 2015 03:00:24 -0400 (EDT) Xiao Ni <xni@redhat.com>
> > > > wrote:
> > > >
> > > > > Hi Neil
> > > > >
> > > > > I encounter the problem when I reshape a 4-disks raid5 to raid5.
> > > > > It
> > > > > just
> > > > > can
> > > > > appear with loop devices.
> > > > >
> > > > > The steps are:
> > > > >
> > > > > [root@dhcp-12-158 mdadm-3.3.2]# mdadm -CR /dev/md0 -l5 -n5
> > > > > /dev/loop[0-4]
> > > > > --assume-clean
> > > > > mdadm: /dev/loop0 appears to be part of a raid array:
> > > > > level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > > mdadm: /dev/loop1 appears to be part of a raid array:
> > > > > level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > > mdadm: /dev/loop2 appears to be part of a raid array:
> > > > > level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > > mdadm: /dev/loop3 appears to be part of a raid array:
> > > > > level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > > mdadm: /dev/loop4 appears to be part of a raid array:
> > > > > level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > > mdadm: Defaulting to version 1.2 metadata
> > > > > mdadm: array /dev/md0 started.
> > > > > [root@dhcp-12-158 mdadm-3.3.2]# mdadm /dev/md0 -a /dev/loop5
> > > > > mdadm: added /dev/loop5
> > > > > [root@dhcp-12-158 mdadm-3.3.2]# mdadm --grow /dev/md0 --raid-devices
> > > > > 6
> > > > > mdadm: Need to backup 10240K of critical section..
> > > > > [root@dhcp-12-158 mdadm-3.3.2]# cat /proc/mdstat
> > > > > Personalities : [raid6] [raid5] [raid4]
> > > > > md0 : active raid5 loop5[5] loop4[4] loop3[3] loop2[2] loop1[1]
> > > > > loop0[0]
> > > > > 8187904 blocks super 1.2 level 5, 512k chunk, algorithm 2 [6/6]
> > > > > [UUUUUU]
> > > > > [>....................] reshape = 0.0% (0/2046976)
> > > > > finish=6396.8min
> > > > > speed=0K/sec
> > > > >
> > > > > unused devices: <none>
> > > > >
> > > > > It because the sync_max is set to 0 when run the command --grow
> > > > >
> > > > > [root@dhcp-12-158 mdadm-3.3.2]# cd /sys/block/md0/md/
> > > > > [root@dhcp-12-158 md]# cat sync_max
> > > > > 0
> > > > >
> > > > > I tried reproduce with normal sata devices. The progress of
> > > > > reshape is
> > > > > no problem. Then
> > > > > I checked the Grow.c. If I use sata devices, in function
> > > > > reshape_array,
> > > > > the
> > > > > return value
> > > > > of set_new_data_offset is 0. But if I used loop devices, it return 1.
> > > > > Then
> > > > > it call the function
> > > > > start_reshape.
> > > >
> > > > set_new_data_offset returns '0' if there is room on the devices to
> > > > reduce
> > > > the
> > > > data offset so that the reshape starts writing to unused space on the
> > > > array.
> > > > This removes the need for a backup file, or the use of a spare device
> > > > to
> > > > store a temporary backup.
> > > > It returns '1' if there was no room for relocating the data_offset.
> > > >
> > > > So on your sata devices (which are presumably larger than your loop
> > > > devices)
> > > > there was room. On your loop devices there was not.
> > > >
> > > >
> > > > >
> > > > > In the function start_reshape it set the sync_max to
> > > > > reshape_progress.
> > > > > But in sysfs_read it
> > > > > doesn't read reshape_progress. So it's 0 and the sync_max is set to
> > > > > 0.
> > > > > Why
> > > > > it need to set the
> > > > > sync_max at this? I'm not sure about this.
> > > >
> > > > sync_max is set to 0 so that the reshape does not start until the
> > > > backup
> > > > has
> > > > been taken.
> > > > Once the backup is taken, child_monitor() should set sync_max to "max".
> > > >
> > > > Can you check if that is happening?
> > > >
> > > > Thanks,
> > > > NeilBrown
> > > >
> > > >
> > >
> > > Thanks very much for the explaining. The problem maybe is fixed. I
> > > tried
> > > reproduce this with newest
> > > kernel and newest mdadm. Now the problem don't exist. I'll do more tests
> > > and
> > > give the answer above later.
> > >
> >
> > Hi Neil
> >
> > As you said, it doesn't enter child monitor. The problem still exist.
> >
> > The kernel version :
> > [root@intel-canoepass-02 tmp]# uname -r
> > 4.0.4
> >
> > mdadm I used is the newest git code from
> > git://git.neil.brown.name/mdadm.git
> >
> >
> > In the function continue_via_systemd the parent find pid is bigger than
> > 0 and
> > status is 0. So it return 1. So it have no opportunity to call
> > child_monitor.
>
> If continue_via_systemd succeeded, that implies that
> systemctl start mdadm-grow-continue@mdXXX.service
>
> succeeded. So
> mdadm --grow --continue /dev/mdXXX
>
> was run, so that mdadm should call 'child_monitor' and update sync_max when
> appropriate. Can you check if it does?
[root@intel-waimeabay-hedt-01 create_assemble]# systemctl start mdadm-grow-continue@md0.service
[root@intel-waimeabay-hedt-01 create_assemble]# echo $?
0
[root@intel-waimeabay-hedt-01 create_assemble]# systemctl status mdadm-grow-continue@md0.service
mdadm-grow-continue@md0.service - Manage MD Reshape on /dev/md0
Loaded: loaded (/usr/lib/systemd/system/mdadm-grow-continue@.service; static)
Active: failed (Result: exit-code) since Tue 2015-05-26 05:33:59 EDT; 21s ago
Process: 5374 ExecStart=/usr/sbin/mdadm --grow --continue /dev/%I (code=exited, status=1/FAILURE)
Main PID: 5374 (code=exited, status=1/FAILURE)
May 26 05:33:59 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Started Manage MD Reshape on /dev/md0.
May 26 05:33:59 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: mdadm-grow-continue@md0.service: main process exited, ...URE
May 26 05:33:59 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Unit mdadm-grow-continue@md0.service entered failed state.
Hint: Some lines were ellipsized, use -l to show in full.
[root@intel-waimeabay-hedt-01 create_assemble]# mdadm --grow --continue /dev/md0 --backup-file=tmp0
mdadm: Need to backup 6144K of critical section..
Now the reshape start.
>
>
> >
> >
> > And if it want to set sync_max to 0 until the backup has been taken. Why
> > does not
> > set sync_max to 0 directly, but use the value reshape_progress? There is a
> > little confused.
>
> When reshaping an array to a different array of the same size, such as a
> 4-driver RAID5 to a 5-driver RAID6, then mdadm needs to backup, one piece at
> a time, the entire array (unless it can change data_offset, which is a
> relatively new ability).
>
> If you stop an array when it is in the middle of such a reshape, and then
> reassemble the array, the backup process need to recommence where it left
> off.
> So it tells the kernel that the reshape can progress as far as where it was
> up to before. So 'sync_max' is set based on the value of 'reshape_progress'.
> (This will happen almost instantly).
>
> Then the background mdadm (or the mdadm started by systemd) will backup the
> next few stripes, update sync_max, wait for those stripes to be reshaped,
> then
> discard the old backup, create a new one of the few stripes after that, and
> continue.
>
> Does that make it a little clearer?
This is a big dinner for me. I need digest this for a while. Thanks very much
for this. What's the "backup process"?
Could you explain backup in detail. I read the man about backup file.
When relocating the first few stripes on a RAID5 or RAID6, it is not possible to keep the data on disk completely
consistent and crash-proof. To provide the required safety, mdadm disables writes to the array while this "critical
section" is reshaped, and takes a backup of the data that is in that section.
What's the reason about data consistent when relocate data?
>
> And in response to your other email:
> > Does it should return 1 when pid > 0 and status is not zero?
>
> No. continue_via_systemd should return 1 precisely when the 'systemctl'
> command was successfully run. So 'status' must be zero.
>
>
I got this. So reshape_array should return when continue_via_systemd return 1. Then the
reshape is going on when run the command mdadm --grow --continue. Now the child_monitor
is called and sync_max is set to max.
Best Regards
Xiao
^ permalink raw reply
* Re: raid5 reshape is stuck
From: Xiao Ni @ 2015-05-26 10:48 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <20150525135001.43d1083a@notabene.brown>
----- Original Message -----
> From: "NeilBrown" <neilb@suse.de>
> To: "Xiao Ni" <xni@redhat.com>
> Cc: linux-raid@vger.kernel.org
> Sent: Monday, May 25, 2015 11:50:01 AM
> Subject: Re: raid5 reshape is stuck
>
> On Thu, 21 May 2015 08:31:58 -0400 (EDT) Xiao Ni <xni@redhat.com> wrote:
>
> >
> >
> > ----- Original Message -----
> > > From: "Xiao Ni" <xni@redhat.com>
> > > To: "NeilBrown" <neilb@suse.de>
> > > Cc: linux-raid@vger.kernel.org
> > > Sent: Thursday, May 21, 2015 11:37:57 AM
> > > Subject: Re: raid5 reshape is stuck
> > >
> > >
> > >
> > > ----- Original Message -----
> > > > From: "NeilBrown" <neilb@suse.de>
> > > > To: "Xiao Ni" <xni@redhat.com>
> > > > Cc: linux-raid@vger.kernel.org
> > > > Sent: Thursday, May 21, 2015 7:48:37 AM
> > > > Subject: Re: raid5 reshape is stuck
> > > >
> > > > On Fri, 15 May 2015 03:00:24 -0400 (EDT) Xiao Ni <xni@redhat.com>
> > > > wrote:
> > > >
> > > > > Hi Neil
> > > > >
> > > > > I encounter the problem when I reshape a 4-disks raid5 to raid5.
> > > > > It
> > > > > just
> > > > > can
> > > > > appear with loop devices.
> > > > >
> > > > > The steps are:
> > > > >
> > > > > [root@dhcp-12-158 mdadm-3.3.2]# mdadm -CR /dev/md0 -l5 -n5
> > > > > /dev/loop[0-4]
> > > > > --assume-clean
> > > > > mdadm: /dev/loop0 appears to be part of a raid array:
> > > > > level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > > mdadm: /dev/loop1 appears to be part of a raid array:
> > > > > level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > > mdadm: /dev/loop2 appears to be part of a raid array:
> > > > > level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > > mdadm: /dev/loop3 appears to be part of a raid array:
> > > > > level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > > mdadm: /dev/loop4 appears to be part of a raid array:
> > > > > level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > > mdadm: Defaulting to version 1.2 metadata
> > > > > mdadm: array /dev/md0 started.
> > > > > [root@dhcp-12-158 mdadm-3.3.2]# mdadm /dev/md0 -a /dev/loop5
> > > > > mdadm: added /dev/loop5
> > > > > [root@dhcp-12-158 mdadm-3.3.2]# mdadm --grow /dev/md0 --raid-devices
> > > > > 6
> > > > > mdadm: Need to backup 10240K of critical section..
> > > > > [root@dhcp-12-158 mdadm-3.3.2]# cat /proc/mdstat
> > > > > Personalities : [raid6] [raid5] [raid4]
> > > > > md0 : active raid5 loop5[5] loop4[4] loop3[3] loop2[2] loop1[1]
> > > > > loop0[0]
> > > > > 8187904 blocks super 1.2 level 5, 512k chunk, algorithm 2 [6/6]
> > > > > [UUUUUU]
> > > > > [>....................] reshape = 0.0% (0/2046976)
> > > > > finish=6396.8min
> > > > > speed=0K/sec
> > > > >
> > > > > unused devices: <none>
> > > > >
> > > > > It because the sync_max is set to 0 when run the command --grow
> > > > >
> > > > > [root@dhcp-12-158 mdadm-3.3.2]# cd /sys/block/md0/md/
> > > > > [root@dhcp-12-158 md]# cat sync_max
> > > > > 0
> > > > >
> > > > > I tried reproduce with normal sata devices. The progress of
> > > > > reshape is
> > > > > no problem. Then
> > > > > I checked the Grow.c. If I use sata devices, in function
> > > > > reshape_array,
> > > > > the
> > > > > return value
> > > > > of set_new_data_offset is 0. But if I used loop devices, it return 1.
> > > > > Then
> > > > > it call the function
> > > > > start_reshape.
> > > >
> > > > set_new_data_offset returns '0' if there is room on the devices to
> > > > reduce
> > > > the
> > > > data offset so that the reshape starts writing to unused space on the
> > > > array.
> > > > This removes the need for a backup file, or the use of a spare device
> > > > to
> > > > store a temporary backup.
> > > > It returns '1' if there was no room for relocating the data_offset.
> > > >
> > > > So on your sata devices (which are presumably larger than your loop
> > > > devices)
> > > > there was room. On your loop devices there was not.
> > > >
> > > >
> > > > >
> > > > > In the function start_reshape it set the sync_max to
> > > > > reshape_progress.
> > > > > But in sysfs_read it
> > > > > doesn't read reshape_progress. So it's 0 and the sync_max is set to
> > > > > 0.
> > > > > Why
> > > > > it need to set the
> > > > > sync_max at this? I'm not sure about this.
> > > >
> > > > sync_max is set to 0 so that the reshape does not start until the
> > > > backup
> > > > has
> > > > been taken.
> > > > Once the backup is taken, child_monitor() should set sync_max to "max".
> > > >
> > > > Can you check if that is happening?
> > > >
> > > > Thanks,
> > > > NeilBrown
> > > >
> > > >
> > >
> > > Thanks very much for the explaining. The problem maybe is fixed. I
> > > tried
> > > reproduce this with newest
> > > kernel and newest mdadm. Now the problem don't exist. I'll do more tests
> > > and
> > > give the answer above later.
> > >
> >
> > Hi Neil
> >
> > As you said, it doesn't enter child monitor. The problem still exist.
> >
> > The kernel version :
> > [root@intel-canoepass-02 tmp]# uname -r
> > 4.0.4
> >
> > mdadm I used is the newest git code from
> > git://git.neil.brown.name/mdadm.git
> >
> >
> > In the function continue_via_systemd the parent find pid is bigger than
> > 0 and
> > status is 0. So it return 1. So it have no opportunity to call
> > child_monitor.
>
> If continue_via_systemd succeeded, that implies that
> systemctl start mdadm-grow-continue@mdXXX.service
>
> succeeded. So
> mdadm --grow --continue /dev/mdXXX
>
> was run, so that mdadm should call 'child_monitor' and update sync_max when
> appropriate. Can you check if it does?
The service is not running.
[root@intel-waimeabay-hedt-01 create_assemble]# systemctl start mdadm-grow-continue@md0.service
[root@intel-waimeabay-hedt-01 create_assemble]# echo $?
0
[root@intel-waimeabay-hedt-01 create_assemble]# systemctl status mdadm-grow-continue@md0.service
mdadm-grow-continue@md0.service - Manage MD Reshape on /dev/md0
Loaded: loaded (/usr/lib/systemd/system/mdadm-grow-continue@.service; static)
Active: failed (Result: exit-code) since Tue 2015-05-26 05:33:59 EDT; 21s ago
Process: 5374 ExecStart=/usr/sbin/mdadm --grow --continue /dev/%I (code=exited, status=1/FAILURE)
Main PID: 5374 (code=exited, status=1/FAILURE)
May 26 05:33:59 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Started Manage MD Reshape on /dev/md0.
May 26 05:33:59 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: mdadm-grow-continue@md0.service: main process exited, ...URE
May 26 05:33:59 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Unit mdadm-grow-continue@md0.service entered failed state.
Hint: Some lines were ellipsized, use -l to show in full.
[root@intel-waimeabay-hedt-01 create_assemble]# mdadm --grow --continue /dev/md0 --backup-file=tmp0
mdadm: Need to backup 6144K of critical section..
Now the reshape start.
Try modify the service file :
ExecStart=/usr/sbin/mdadm --grow --continue /dev/%I --backup-file=/root/tmp0
It doesn't work too.
[root@intel-waimeabay-hedt-01 ~]# systemctl daemon-reload
[root@intel-waimeabay-hedt-01 ~]# systemctl start mdadm-grow-continue@md0.service
[root@intel-waimeabay-hedt-01 ~]# systemctl status mdadm-grow-continue@md0.service
mdadm-grow-continue@md0.service - Manage MD Reshape on /dev/md0
Loaded: loaded (/usr/lib/systemd/system/mdadm-grow-continue@.service; static)
Active: failed (Result: exit-code) since Tue 2015-05-26 05:50:22 EDT; 10s ago
Process: 6475 ExecStart=/usr/sbin/mdadm --grow --continue /dev/%I --backup-file=/root/tmp0 (code=exited, status=1/FAILURE)
Main PID: 6475 (code=exited, status=1/FAILURE)
May 26 05:50:22 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Started Manage MD Reshape on /dev/md0.
May 26 05:50:22 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: mdadm-grow-continue@md0.service: main process exited, ...URE
May 26 05:50:22 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Unit mdadm-grow-continue@md0.service entered failed state.
Hint: Some lines were ellipsized, use -l to show in full.
>
>
> >
> >
> > And if it want to set sync_max to 0 until the backup has been taken. Why
> > does not
> > set sync_max to 0 directly, but use the value reshape_progress? There is a
> > little confused.
>
> When reshaping an array to a different array of the same size, such as a
> 4-driver RAID5 to a 5-driver RAID6, then mdadm needs to backup, one piece at
> a time, the entire array (unless it can change data_offset, which is a
> relatively new ability).
>
> If you stop an array when it is in the middle of such a reshape, and then
> reassemble the array, the backup process need to recommence where it left
> off.
> So it tells the kernel that the reshape can progress as far as where it was
> up to before. So 'sync_max' is set based on the value of 'reshape_progress'.
> (This will happen almost instantly).
>
> Then the background mdadm (or the mdadm started by systemd) will backup the
> next few stripes, update sync_max, wait for those stripes to be reshaped,
> then
> discard the old backup, create a new one of the few stripes after that, and
> continue.
>
> Does that make it a little clearer?
This is a big dinner for me. I need digest this for a while. Thanks very much
for this. What's the "backup process"?
Could you explain backup in detail. I read the man about backup file.
When relocating the first few stripes on a RAID5 or RAID6, it is not possible to keep the data on disk completely
consistent and crash-proof. To provide the required safety, mdadm disables writes to the array while this "critical
section" is reshaped, and takes a backup of the data that is in that section.
What's the reason about data consistent when relocate data?
>
> And in response to your other email:
> > Does it should return 1 when pid > 0 and status is not zero?
>
> No. continue_via_systemd should return 1 precisely when the 'systemctl'
> command was successfully run. So 'status' must be zero.
>
>
I got this. So reshape_array should return when continue_via_systemd return 1. Then the
reshape is going on when run the command mdadm --grow --continue. Now the child_monitor
is called and sync_max is set to max.
Best Regards
Xiao
^ permalink raw reply
* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Another Sillyname @ 2015-05-26 11:18 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <alpine.DEB.2.02.1505261005260.9487@uplift.swm.pp.se>
Mikael
Very easy to understand......any way that requires entering the swaps
into /etc/fstab therefore means that if any drive and it's contained
swap fails the reboot can fail (plus the overhead of all those uuid
numbers in fstab). My way means that as the swaps don't get loaded
unless the drive is alive the reboot has more resilience.
On 26 May 2015 at 09:06, Mikael Abrahamsson <swmike@swm.pp.se> wrote:
> On Tue, 26 May 2015, Another Sillyname wrote:
>
>> Not bothered about raiding the swap thanks, my way will suffice as it
>> gives me maximum flexibility and resilience......I'm not really
>> performance driven on this project.
>
>
> I don't see how running swap natively on the drives gives "maximum
> resilience". Higher resilience is gained by running raid1 for swap.
>
>
> --
> Mikael Abrahamsson email: swmike@swm.pp.se
^ permalink raw reply
* Re: Installing Linux directly onto RAID6 Array...........
From: Another Sillyname @ 2015-05-26 13:18 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150526182933.52c6c37e@notabene.brown>
Excepting that by incorporating the mdadm.conf into the initramfs I
don't need to assemble the manually array as the information is
already there. I don't have to risk a superblock overwrite as you go
on to state as I already have the mdguid info......anyway I'm not
certain my way will work....that's the point of doing the testing.
On 26 May 2015 at 09:29, NeilBrown <neilb@suse.de> wrote:
> On Sun, 24 May 2015 10:08:59 +0100 Another Sillyname
> <anothersname@googlemail.com> wrote:
>
>> I suspect you're correct in that I'll end up with the boot partitions
>> being in RAID1 and the data in RAID6, however I am seriously
>> considering having the boot in RAID6 as well...if I can integrate the
>> mdadm.conf into the initramfs properly I can't see a reason not to do
>> this?
>
> mdadm.conf is largely a non-issue. You don't need an mdadm.conf to assemble
> your array. All the raid configuration lives in the raid metadata.
> All you need is for your initrd to know what device contains your root
> filesystem (preferably by UUID) so that when mdadm finds that array, the
> initrd code can mount it for you.
>
> I believe that GRUB2 can load an initrd and kernel from a filesystem on an
> mdraid device, but I don't know where the boot sector would load GRUB2 from.
>
> md's v1.2 metadata leaves 4K at the start of each device. If GRUB2 fits in
> there, then it could certainly load, assemble the RAID6, then pull the files
> off your root filesystem. But I doubt it.
>
> If GRUB tries to put the boot loader anywhere else, there is a good chance
> that md could over-write it, as it believes that it owns all the space after
> 4K.
>
> According to the documentation, GRUB2 either places the second stage in the
> first 32K before the first partition, or in the filesystem at specific block
> locations.
> The first cannot work if md uses the whole device (works fine if md uses
> partitions).
> The second cannot work with RAID6 as the blocks are in locations on one
> device. This only really work for RAID1.
>
> So feel free to try, and do report any results, but I doubt you'll get it to
> work reliably.
>
> NeilBrown
>
>>
>> Had a look at the metadata=0.9 option but reading the info on mdadm
>> metadata I think I'd prefer to have the metadata at the start of the
>> drive, also it looks like metadata=1.2 has extra functionality that I
>> may want to use later.
>>
>> On 24 May 2015 at 09:36, Mikael Abrahamsson <swmike@swm.pp.se> wrote:
>> > On Sun, 24 May 2015, Another Sillyname wrote:
>> >
>> >> So I now have 5 partitions.
>> >>
>> >> a - bios_boot
>> >> b - efi
>> >> c - boot
>> >> d - root
>> >> e - swap
>> >>
>> >> I'll be adding one more when I'm happy this is working.
>> >>
>> >> f - home
>> >>
>> >> 3. Using the methods above I have now created a bootable fedora
>> >> system, on a single drive in preparation to now RAID the required
>> >> partitions. However my concern comes regarding the mdadm metadata,
>> >> simplistically metadata=1.2 apparently writes it's superblock to 4k
>> >> after the start of the device, this is exactly where my efi partition
>> >> (b above) starts, so my concern is will this superblock overwrite or
>> >> mess with my current partition table?
>> >>
>> >> 4. If the next stage works then I think what I'll actually end up
>> >> doing is......
>> >>
>> >> scrub what I have now.
>> >>
>> >> create the arrays before running the Fedora Live installer (this
>> >> assumes the installer will see /md[x] devices and allow them to be
>> >> used to install to). Then incorporate the mdadm.conf data into the
>> >> initramfs and regenerate initramfs.
>> >>
>> >> Ideas/Thoughts/Criticisms?
>> >
>> >
>> > You don't want to run MD on the entire drive in this case, you most likely
>> > want to create multiple RAID1 and RAID6 mirrors. RAID1 your boot, root and
>> > swap, then run RAID6 on your home partition. Use use superblock type that
>> > creates the superblock at the end for the RAID1 partitions.
>> >
>> > Also, you don't want to refer to "sda" when booting, you want to use
>> > UUID=<uuid> in fstab, crypttab etc.
>> >
>> > --
>> > Mikael Abrahamsson email: swmike@swm.pp.se
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Mikael Abrahamsson @ 2015-05-26 14:08 UTC (permalink / raw)
To: Another Sillyname; +Cc: linux-raid
In-Reply-To: <CAOS+5GHWktYuV2Q7NhsqybgFY5-QKRaGih0YcerD08_rKZp4Ng@mail.gmail.com>
On Tue, 26 May 2015, Another Sillyname wrote:
> Very easy to understand......any way that requires entering the swaps
> into /etc/fstab therefore means that if any drive and it's contained
> swap fails the reboot can fail (plus the overhead of all those uuid
> numbers in fstab). My way means that as the swaps don't get loaded
> unless the drive is alive the reboot has more resilience.
If you have RAID1 for swap, you only need a single component drive to work
for the RAID1 to be able to start. It also means any drive can fail and
your swap information still works.
And I don't understand why you're referring to "overhead of all those uuid
numbers in fstab". You're worried about 10-20 bytes extra in fstab???
--
Mikael Abrahamsson email: swmike@swm.pp.se
^ permalink raw reply
* Re: [PATCH v4 06/11] md/raid5: get rid of bio_fits_rdev()
From: Ming Lin @ 2015-05-26 14:33 UTC (permalink / raw)
To: Christoph Hellwig
Cc: NeilBrown, Ming Lin, lkml, Kent Overstreet, Jens Axboe,
Dongsu Park, linux-raid
In-Reply-To: <20150525141708.GA26249@lst.de>
On Mon, May 25, 2015 at 7:17 AM, Christoph Hellwig <hch@lst.de> wrote:
> On Mon, May 25, 2015 at 05:54:14PM +1000, NeilBrown wrote:
>> Did I write that? I guess I did :-(
>> I meant *after*. Don't get rid of bio_fits_rdev until split_bio is in
>> chunk_aligned_read().
>
> I suspect the whole series could use some reordering.
Nice reordering.
I'll do this.
Thanks.
>
> patch 1:
>
> add ->bio_split and blk_queue_split
>
> patch 2..n:
>
> one for each non-trivial driver that implements ->merge_bvec_fn to
> remove it and instead split bios in ->make_request. The md patch
> to do the right thing in chunk_aligned_read goes into the general
> md patch here. The bcache patch also goes into this series.
>
> patch n+1:
>
> - add blk_queue_split calls for remaining trivial drivers
>
> patch n+2:
>
> - remove ->merge_bvec_fn and checking of max_sectors a for all
> drivers, simplify bio_add_page
>
> patch n+2:
>
> - remove splitting in blkdev_issue_discard
>
> patch n+3
>
> - remove bio_fits_rdev
>
> patch n+4
>
> - remove bio_get_nr_vecs
>
> patch n+4
>
> - use bio_add_page
>
> patch n+5
>
> - update documentation
^ permalink raw reply
* Re: [PATCH] md-cluster: avoid deadlock on MESSAGE lock resource
From: Goldwyn Rodrigues @ 2015-05-26 14:44 UTC (permalink / raw)
To: Abhijit Bhopatkar, linux-raid, Lidong Zhong; +Cc: Reese Faucette (rfaucett)
In-Reply-To: <556330FA.1030200@cisco.com>
On 05/25/2015 09:26 AM, Abhijit Bhopatkar wrote:
> On 17/05/15 2:28 am, Goldwyn Rodrigues wrote:
>>
>>
>> On 05/08/2015 08:14 AM, Abhijit Bhopatkar wrote:
>>> On 08/05/15 6:40 pm, Abhijit Bhopatkar wrote:
>>>>
>>>> Every receiver has CR lock on MESSAGE while processing the message. When
>>>> every receiver releases ACK lock and for some reason fails to grab EX on
>>>> MESSAGE resource in time, a waiting sender could queue an EX on MESSAGE
>>>> instead. Now when receiver queues its up convert request on MESSAGE it
>>>> will end up in a deadlock situation.
>>>>
>>>> Setting NOQUEUE flag on MESSAGE lock resource while grabbing the EX on
>>>> MESSAGE on sender will avoid this deadlock. If sender can not grab
>>>> MESSAGE lock immediately it should retry until the lock is granted.
>>>>
>>>> Signed-off-by: Abhijit Bhopatkar <abhopatk@cisco.com>
>>>> ---
>>>> This has been minimally tested on a three node cluster.
>>>>
>>>
>>> I have tested standard mdadm operations (create, assemble etc).
>>> What more testing would you want me to do on this before its considered
>>> ready?
>>
>> I am not sure how using LKF_NOQUEUE will help in this situation here. LKF_NOQUEUE primarily means do not queue if you can't grant it right away. Besides, I don't like the idea of goto loop.
>>
>> The sender can still creep in between the ack and the message locks. A situation would be where the "disrupting" sender is the lock owner of all the locks and hence will not have to pay communication costs and will manage to attain the locks faster.
>>
>> Perhaps DLM_LKF_HEADQUEUE or DLM_LKF_NOORDER is what you are looking for, but that again is not the complete solution.
>>
>> Another idea I could think of is for the sender to downconvert TOKEN to a shared lock such as CR halfway in the communication (say after message CR), and all receivers take the TOKEN in CR mode and release it once the communication is finally over.
>>
>> Regards,
>>
>>
> I agree about the goto pollution and yes converting receivers to use DLM_LKF_HEADQUEUE will solve the problem gracefully. Will send the new patch shortly.
>
> However I do not understand why this is incomplete solution. The "disruptive sender" as you have called it, is already "TOKEN" owner and otherwise it will compete for TOKEN lock as usual with other senders with equal priority. Not gaining any priority over others. The changes simply make sender stall for all _receivers_ to complete their serialization and wait till all receivers convert MESSAGE lock from CR to EX to NL, nothing else changes.
>
Yes, you are right. I ignored an operation on the message lock resource.
I will perform some tests before I signoff.
Thanks,
--
Goldwyn
^ permalink raw reply
* Re: [PATCH 2/8] md/raid5: Ensure a batch member is not handled prematurely.
From: Shaohua Li @ 2015-05-26 18:16 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <20150523102640.20be3fca@notabene.brown>
On Sat, May 23, 2015 at 10:26:40AM +1000, NeilBrown wrote:
> On Fri, 22 May 2015 16:44:02 -0700 Shaohua Li <shli@kernel.org> wrote:
>
> > On Fri, May 22, 2015 at 03:30:58PM +1000, NeilBrown wrote:
> > > If a stripe is a member of a batch, but not the head, it must
> > > not be handled separately from the rest of the batch.
> > >
> > > 'clear_batch_ready()' handles this requirement to some
> > > extent but not completely. If a member is passed to handle_stripe()
> > > a second time it returns '0' indicating the stripe can be handled,
> > > which is wrong.
> > > So add an extra test.
> > >
> > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > ---
> > > drivers/md/raid5.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > > index c3ccefbd4fe7..9a803b735848 100644
> > > --- a/drivers/md/raid5.c
> > > +++ b/drivers/md/raid5.c
> > > @@ -4192,9 +4192,13 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
> > >
> > > static int clear_batch_ready(struct stripe_head *sh)
> > > {
> > > + /* Return '1' if this is a member of batch, or
> > > + * '0' if it is a lone stripe or a head which can now be
> > > + * handled.
> > > + */
> > > struct stripe_head *tmp;
> > > if (!test_and_clear_bit(STRIPE_BATCH_READY, &sh->state))
> > > - return 0;
> > > + return (sh->batch_head && sh->batch_head != sh);
> > > spin_lock(&sh->stripe_lock);
> > > if (!sh->batch_head) {
> > > spin_unlock(&sh->stripe_lock);
> >
> > which case can this happen in?
>
> It definitely happens as I had reliable problems until I added this fix.
> 'retry_aligned_read()' can call handle_stripe() on any stripe at any time,
> but I doubt that would apply. I might try putting a warn-on there and see if
> it provides any hints.
>
> >
> > Patches look good. But I'm not in Fusionio any more, so can't check the
> > performance in big raid array with fast flash cards. I'm doing some tests here.
> > I hit a warning in break_stripe_batch_list, STRIPE_BIT_DELAY is set in the
> > stripe state. I'm checking the reason, but if you have thoughts I can try
> > immediately, please let me know.
>
> I got STRIPE_BIT_DELAY a few times. That was the main reason for
>
> md/raid5: ensure whole batch is delayed for all required bitmap updates.
>
> and they went away after I got that patch right.
>
> Maybe there is a race in there..
>
> If you can reproduce it, maybe WARN whenever STRIPE_BIT_DELAY gets set on a
> stripe with ->batch_head.
Ok, there is a race in add_stripe_bio(). We unlocked the stripe_lock to set the
BIT_DELAY. After the unlock, the stripe might be added to a batch,
stripe_add_to_batch_list didn't clear the bit. Holding the lock in
add_stripe_bio and checking ->batch_head again when we set the bit should fix
the issue.
And STRIPE_ON_UNPLUG_LIST and STRIPE_ON_RELEASE_LIST are set is legit in
break_stripe_batch_list(), they should be removed from the WARN_ON_ONCE().
Thanks,
Shaohua
^ permalink raw reply
* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Wols Lists @ 2015-05-26 20:11 UTC (permalink / raw)
To: Mikael Abrahamsson, Another Sillyname; +Cc: linux-raid
In-Reply-To: <alpine.DEB.2.02.1505261605450.9487@uplift.swm.pp.se>
On 26/05/15 15:08, Mikael Abrahamsson wrote:
> On Tue, 26 May 2015, Another Sillyname wrote:
>
>> Very easy to understand......any way that requires entering the swaps
>> into /etc/fstab therefore means that if any drive and it's contained
>> swap fails the reboot can fail (plus the overhead of all those uuid
>> numbers in fstab). My way means that as the swaps don't get loaded
>> unless the drive is alive the reboot has more resilience.
>
> If you have RAID1 for swap, you only need a single component drive to
> work for the RAID1 to be able to start. It also means any drive can fail
> and your swap information still works.
And you're wasting a lot of disk space. What's important to you (that a
live swap disk shouldn't fail) is not important to me and doesn't seem
to be important to Another Sillyname.
My disk drives have 32Gb swap partitions. Overkill? Dunno. But I'd
rather linux raid 0's them for 64Gb swap than mdadm raid 1's them for
32Gb swap. I have a couple of 20Gb tmpfs partitions :-)
I don't know why Another Sillyname wants to chain his swap partitions,
but it's his choice. Maybe like me, his system gets rebooted a couple of
times a day, and failure to start is a far more real risk than failure
in use. He's running Fedora - that seems likely then ...
Cheers,
Wol
^ permalink raw reply
* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Another Sillyname @ 2015-05-26 21:02 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <5564D373.30705@youngman.org.uk>
Guys
Without wishing to sound like the list police (no-one expects the list
police!!) this has drifted off topic into a fairly unimportant
area.....I've made my decision and will live with the consequences for
now. As I'm currently 10 hours into a circa 40 hour 20TB data copy
you can all rest assured nothing short of a catastrophic failure is
going to change the current setup......when I get another test box in
a couple of weeks I'll revisit.
Until then thanks for all the help but can we let this lie please.
Tony
On 26 May 2015 at 21:11, Wols Lists <antlists@youngman.org.uk> wrote:
> On 26/05/15 15:08, Mikael Abrahamsson wrote:
>> On Tue, 26 May 2015, Another Sillyname wrote:
>>
>>> Very easy to understand......any way that requires entering the swaps
>>> into /etc/fstab therefore means that if any drive and it's contained
>>> swap fails the reboot can fail (plus the overhead of all those uuid
>>> numbers in fstab). My way means that as the swaps don't get loaded
>>> unless the drive is alive the reboot has more resilience.
>>
>> If you have RAID1 for swap, you only need a single component drive to
>> work for the RAID1 to be able to start. It also means any drive can fail
>> and your swap information still works.
>
> And you're wasting a lot of disk space. What's important to you (that a
> live swap disk shouldn't fail) is not important to me and doesn't seem
> to be important to Another Sillyname.
>
> My disk drives have 32Gb swap partitions. Overkill? Dunno. But I'd
> rather linux raid 0's them for 64Gb swap than mdadm raid 1's them for
> 32Gb swap. I have a couple of 20Gb tmpfs partitions :-)
>
> I don't know why Another Sillyname wants to chain his swap partitions,
> but it's his choice. Maybe like me, his system gets rebooted a couple of
> times a day, and failure to start is a far more real risk than failure
> in use. He's running Fedora - that seems likely then ...
>
> Cheers,
> Wol
^ permalink raw reply
* Re: [PATCH v4 06/11] md/raid5: get rid of bio_fits_rdev()
From: Ming Lin @ 2015-05-26 22:32 UTC (permalink / raw)
To: Ming Lin
Cc: Christoph Hellwig, NeilBrown, lkml, Kent Overstreet, Jens Axboe,
Dongsu Park, linux-raid
In-Reply-To: <CAF1ivSajEe1ec_dCeUec=ER3keKVfhhNvv5pWiti_M++NRrBDQ@mail.gmail.com>
On Tue, May 26, 2015 at 7:33 AM, Ming Lin <mlin@kernel.org> wrote:
> On Mon, May 25, 2015 at 7:17 AM, Christoph Hellwig <hch@lst.de> wrote:
>> On Mon, May 25, 2015 at 05:54:14PM +1000, NeilBrown wrote:
>>> Did I write that? I guess I did :-(
>>> I meant *after*. Don't get rid of bio_fits_rdev until split_bio is in
>>> chunk_aligned_read().
>>
>> I suspect the whole series could use some reordering.
>
> Nice reordering.
> I'll do this.
Here is the reordering.
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=block-generic-req
I'll post it if you are OK.
[PATCH 01/15] block: add blk_queue_split()
[PATCH 02/15] md: remove ->merge_bvec_fn
[PATCH 03/15] dm: remov merge functions
[PATCH 04/15] drbd: remove ->merge_bvec_fn
[PATCH 05/15] pktcdvd: remove ->merge_bvec_fn
[PATCH 06/15] rbd: remove ->merge_bvec_fn
[PATCH 07/15] bcache: remove driver private bio splitting code
[PATCH 08/15] btrfs: remove bio splitting and merge_bvec_fn() calls
[PATCH 09/15] block: call blk_queue_split() in make_request functions
[PATCH 10/15] block: kill ->merge_bvec_fn and simplify bio_add_page
[PATCH 11/15] block: remove split code in blkdev_issue_discard
[PATCH 12/15] md/raid5: get rid of bio_fits_rdev()
[PATCH 13/15] block: remove bio_get_nr_vecs()
[PATCH 14/15] fs: use helper bio_add_page() instead of open coding on
[PATCH 15/15] Documentation: update notes in biovecs about
>
> Thanks.
>
>>
>> patch 1:
>>
>> add ->bio_split and blk_queue_split
>>
>> patch 2..n:
>>
>> one for each non-trivial driver that implements ->merge_bvec_fn to
>> remove it and instead split bios in ->make_request. The md patch
>> to do the right thing in chunk_aligned_read goes into the general
>> md patch here. The bcache patch also goes into this series.
>>
>> patch n+1:
>>
>> - add blk_queue_split calls for remaining trivial drivers
>>
>> patch n+2:
>>
>> - remove ->merge_bvec_fn and checking of max_sectors a for all
>> drivers, simplify bio_add_page
>>
>> patch n+2:
>>
>> - remove splitting in blkdev_issue_discard
>>
>> patch n+3
>>
>> - remove bio_fits_rdev
>>
>> patch n+4
>>
>> - remove bio_get_nr_vecs
>>
>> patch n+4
>>
>> - use bio_add_page
>>
>> patch n+5
>>
>> - update documentation
^ permalink raw reply
* Re: [PATCH 2/8] md/raid5: Ensure a batch member is not handled prematurely.
From: NeilBrown @ 2015-05-26 22:35 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20150526181647.GA38853@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 5336 bytes --]
On Tue, 26 May 2015 11:16:47 -0700 Shaohua Li <shli@kernel.org> wrote:
> On Sat, May 23, 2015 at 10:26:40AM +1000, NeilBrown wrote:
> > On Fri, 22 May 2015 16:44:02 -0700 Shaohua Li <shli@kernel.org> wrote:
> >
> > > On Fri, May 22, 2015 at 03:30:58PM +1000, NeilBrown wrote:
> > > > If a stripe is a member of a batch, but not the head, it must
> > > > not be handled separately from the rest of the batch.
> > > >
> > > > 'clear_batch_ready()' handles this requirement to some
> > > > extent but not completely. If a member is passed to handle_stripe()
> > > > a second time it returns '0' indicating the stripe can be handled,
> > > > which is wrong.
> > > > So add an extra test.
> > > >
> > > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > > ---
> > > > drivers/md/raid5.c | 6 +++++-
> > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > > > index c3ccefbd4fe7..9a803b735848 100644
> > > > --- a/drivers/md/raid5.c
> > > > +++ b/drivers/md/raid5.c
> > > > @@ -4192,9 +4192,13 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
> > > >
> > > > static int clear_batch_ready(struct stripe_head *sh)
> > > > {
> > > > + /* Return '1' if this is a member of batch, or
> > > > + * '0' if it is a lone stripe or a head which can now be
> > > > + * handled.
> > > > + */
> > > > struct stripe_head *tmp;
> > > > if (!test_and_clear_bit(STRIPE_BATCH_READY, &sh->state))
> > > > - return 0;
> > > > + return (sh->batch_head && sh->batch_head != sh);
> > > > spin_lock(&sh->stripe_lock);
> > > > if (!sh->batch_head) {
> > > > spin_unlock(&sh->stripe_lock);
> > >
> > > which case can this happen in?
> >
> > It definitely happens as I had reliable problems until I added this fix.
> > 'retry_aligned_read()' can call handle_stripe() on any stripe at any time,
> > but I doubt that would apply. I might try putting a warn-on there and see if
> > it provides any hints.
> >
> > >
> > > Patches look good. But I'm not in Fusionio any more, so can't check the
> > > performance in big raid array with fast flash cards. I'm doing some tests here.
> > > I hit a warning in break_stripe_batch_list, STRIPE_BIT_DELAY is set in the
> > > stripe state. I'm checking the reason, but if you have thoughts I can try
> > > immediately, please let me know.
> >
> > I got STRIPE_BIT_DELAY a few times. That was the main reason for
> >
> > md/raid5: ensure whole batch is delayed for all required bitmap updates.
> >
> > and they went away after I got that patch right.
> >
> > Maybe there is a race in there..
> >
> > If you can reproduce it, maybe WARN whenever STRIPE_BIT_DELAY gets set on a
> > stripe with ->batch_head.
>
> Ok, there is a race in add_stripe_bio(). We unlocked the stripe_lock to set the
> BIT_DELAY. After the unlock, the stripe might be added to a batch,
> stripe_add_to_batch_list didn't clear the bit. Holding the lock in
> add_stripe_bio and checking ->batch_head again when we set the bit should fix
> the issue.
We can't hold a spin_lock over bitmap_startwrite(), and we really need to
make sure the write doesn't start until bitmap_startwrite has completed.
So we need to keep the stripe_head out of any batch during that time.
So I've added an extra state bit.
Could you please review and possibly test the patch below?
>
> And STRIPE_ON_UNPLUG_LIST and STRIPE_ON_RELEASE_LIST are set is legit in
> break_stripe_batch_list(), they should be removed from the WARN_ON_ONCE().
Yes, you are right. Thanks.
>
> Thanks,
> Shaohua
Thanks,
NeilBrown
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 041341c66ae5..89d6faafffda 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -760,6 +760,7 @@ static void unlock_two_stripes(struct stripe_head *sh1, struct stripe_head *sh2)
static bool stripe_can_batch(struct stripe_head *sh)
{
return test_bit(STRIPE_BATCH_READY, &sh->state) &&
+ !test_bit(STRIPE_BITMAP_PENDING, &sh->state) &&
is_full_stripe_write(sh);
}
@@ -3007,14 +3008,18 @@ static int add_stripe_bio(struct stripe_head *sh, struct bio *bi, int dd_idx,
pr_debug("added bi b#%llu to stripe s#%llu, disk %d.\n",
(unsigned long long)(*bip)->bi_iter.bi_sector,
(unsigned long long)sh->sector, dd_idx);
- spin_unlock_irq(&sh->stripe_lock);
if (conf->mddev->bitmap && firstwrite) {
+ set_bit(STRIPE_BITMAP_PENDING, &sh->state);
+ spin_unlock_irq(&sh->stripe_lock);
bitmap_startwrite(conf->mddev->bitmap, sh->sector,
STRIPE_SECTORS, 0);
+ spin_lock_irq(&sh->stripe_lock);
+ clear_bit(STRIPE_BITMAP_PENDING, &sh->state);
sh->bm_seq = conf->seq_flush+1;
set_bit(STRIPE_BIT_DELAY, &sh->state);
}
+ spin_lock_irq(&sh->stripe_lock);
if (stripe_can_batch(sh))
stripe_add_to_batch_list(conf, sh);
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index d7b2bc8b756f..02c3bf8fbfe7 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -337,6 +337,9 @@ enum {
STRIPE_ON_RELEASE_LIST,
STRIPE_BATCH_READY,
STRIPE_BATCH_ERR,
+ STRIPE_BITMAP_PENDING, /* Being added to bitmap, don't add
+ * to batch yet.
+ */
};
#define STRIPE_EXPAND_SYNC_FLAGS \
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply related
* Re: [PATCH v4 06/11] md/raid5: get rid of bio_fits_rdev()
From: NeilBrown @ 2015-05-26 23:03 UTC (permalink / raw)
To: Ming Lin
Cc: Christoph Hellwig, lkml, Kent Overstreet, Jens Axboe, Dongsu Park,
linux-raid
In-Reply-To: <CAF1ivSYxfE+bZrPD7vpHPsU+XAD+S2Lkz=h84vA9yCy86h40ow@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3302 bytes --]
On Tue, 26 May 2015 15:32:38 -0700 Ming Lin <mlin@kernel.org> wrote:
> On Tue, May 26, 2015 at 7:33 AM, Ming Lin <mlin@kernel.org> wrote:
> > On Mon, May 25, 2015 at 7:17 AM, Christoph Hellwig <hch@lst.de> wrote:
> >> On Mon, May 25, 2015 at 05:54:14PM +1000, NeilBrown wrote:
> >>> Did I write that? I guess I did :-(
> >>> I meant *after*. Don't get rid of bio_fits_rdev until split_bio is in
> >>> chunk_aligned_read().
> >>
> >> I suspect the whole series could use some reordering.
> >
> > Nice reordering.
> > I'll do this.
>
> Here is the reordering.
> https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=block-generic-req
>
> I'll post it if you are OK.
>
> [PATCH 01/15] block: add blk_queue_split()
> [PATCH 02/15] md: remove ->merge_bvec_fn
> [PATCH 03/15] dm: remov merge functions
> [PATCH 04/15] drbd: remove ->merge_bvec_fn
> [PATCH 05/15] pktcdvd: remove ->merge_bvec_fn
> [PATCH 06/15] rbd: remove ->merge_bvec_fn
> [PATCH 07/15] bcache: remove driver private bio splitting code
> [PATCH 08/15] btrfs: remove bio splitting and merge_bvec_fn() calls
> [PATCH 09/15] block: call blk_queue_split() in make_request functions
> [PATCH 10/15] block: kill ->merge_bvec_fn and simplify bio_add_page
> [PATCH 11/15] block: remove split code in blkdev_issue_discard
> [PATCH 12/15] md/raid5: get rid of bio_fits_rdev()
> [PATCH 13/15] block: remove bio_get_nr_vecs()
> [PATCH 14/15] fs: use helper bio_add_page() instead of open coding on
> [PATCH 15/15] Documentation: update notes in biovecs about
The changes to dm.c and dm.h should be in the "dm:" patch, not "md:".
But I don't think the sequence is right.
You cannot remove ->merge_bvec_fn for *any* stacked device until *all* devices
make use of blk_queue_split() (or otherwise handle arbitrarily large bios).
I think it would be easiest to:
- add blk_queue_split() and call it from common code before ->make_request_fn
is called. The ensure all devices can accept arbitrarily large bios.
- driver-by-driver remove merge_bvec_fn and make sure the the driver can cope
with arbitrary bios themselve, calling blk_queue_split in the make_request
function only if needed
- finally remove the call to blk_queue_split from the common code.
Does that make sense to others?
Thanks,
NeilBrown
>
> >
> > Thanks.
> >
> >>
> >> patch 1:
> >>
> >> add ->bio_split and blk_queue_split
> >>
> >> patch 2..n:
> >>
> >> one for each non-trivial driver that implements ->merge_bvec_fn to
> >> remove it and instead split bios in ->make_request. The md patch
> >> to do the right thing in chunk_aligned_read goes into the general
> >> md patch here. The bcache patch also goes into this series.
> >>
> >> patch n+1:
> >>
> >> - add blk_queue_split calls for remaining trivial drivers
> >>
> >> patch n+2:
> >>
> >> - remove ->merge_bvec_fn and checking of max_sectors a for all
> >> drivers, simplify bio_add_page
> >>
> >> patch n+2:
> >>
> >> - remove splitting in blkdev_issue_discard
> >>
> >> patch n+3
> >>
> >> - remove bio_fits_rdev
> >>
> >> patch n+4
> >>
> >> - remove bio_get_nr_vecs
> >>
> >> patch n+4
> >>
> >> - use bio_add_page
> >>
> >> patch n+5
> >>
> >> - update documentation
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 2/8] md/raid5: Ensure a batch member is not handled prematurely.
From: Shaohua Li @ 2015-05-26 23:21 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <20150527083532.32486e08@notabene.brown>
On Wed, May 27, 2015 at 08:35:32AM +1000, NeilBrown wrote:
> On Tue, 26 May 2015 11:16:47 -0700 Shaohua Li <shli@kernel.org> wrote:
>
> > On Sat, May 23, 2015 at 10:26:40AM +1000, NeilBrown wrote:
> > > On Fri, 22 May 2015 16:44:02 -0700 Shaohua Li <shli@kernel.org> wrote:
> > >
> > > > On Fri, May 22, 2015 at 03:30:58PM +1000, NeilBrown wrote:
> > > > > If a stripe is a member of a batch, but not the head, it must
> > > > > not be handled separately from the rest of the batch.
> > > > >
> > > > > 'clear_batch_ready()' handles this requirement to some
> > > > > extent but not completely. If a member is passed to handle_stripe()
> > > > > a second time it returns '0' indicating the stripe can be handled,
> > > > > which is wrong.
> > > > > So add an extra test.
> > > > >
> > > > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > > > ---
> > > > > drivers/md/raid5.c | 6 +++++-
> > > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > > > > index c3ccefbd4fe7..9a803b735848 100644
> > > > > --- a/drivers/md/raid5.c
> > > > > +++ b/drivers/md/raid5.c
> > > > > @@ -4192,9 +4192,13 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
> > > > >
> > > > > static int clear_batch_ready(struct stripe_head *sh)
> > > > > {
> > > > > + /* Return '1' if this is a member of batch, or
> > > > > + * '0' if it is a lone stripe or a head which can now be
> > > > > + * handled.
> > > > > + */
> > > > > struct stripe_head *tmp;
> > > > > if (!test_and_clear_bit(STRIPE_BATCH_READY, &sh->state))
> > > > > - return 0;
> > > > > + return (sh->batch_head && sh->batch_head != sh);
> > > > > spin_lock(&sh->stripe_lock);
> > > > > if (!sh->batch_head) {
> > > > > spin_unlock(&sh->stripe_lock);
> > > >
> > > > which case can this happen in?
> > >
> > > It definitely happens as I had reliable problems until I added this fix.
> > > 'retry_aligned_read()' can call handle_stripe() on any stripe at any time,
> > > but I doubt that would apply. I might try putting a warn-on there and see if
> > > it provides any hints.
> > >
> > > >
> > > > Patches look good. But I'm not in Fusionio any more, so can't check the
> > > > performance in big raid array with fast flash cards. I'm doing some tests here.
> > > > I hit a warning in break_stripe_batch_list, STRIPE_BIT_DELAY is set in the
> > > > stripe state. I'm checking the reason, but if you have thoughts I can try
> > > > immediately, please let me know.
> > >
> > > I got STRIPE_BIT_DELAY a few times. That was the main reason for
> > >
> > > md/raid5: ensure whole batch is delayed for all required bitmap updates.
> > >
> > > and they went away after I got that patch right.
> > >
> > > Maybe there is a race in there..
> > >
> > > If you can reproduce it, maybe WARN whenever STRIPE_BIT_DELAY gets set on a
> > > stripe with ->batch_head.
> >
> > Ok, there is a race in add_stripe_bio(). We unlocked the stripe_lock to set the
> > BIT_DELAY. After the unlock, the stripe might be added to a batch,
> > stripe_add_to_batch_list didn't clear the bit. Holding the lock in
> > add_stripe_bio and checking ->batch_head again when we set the bit should fix
> > the issue.
>
> We can't hold a spin_lock over bitmap_startwrite(), and we really need to
> make sure the write doesn't start until bitmap_startwrite has completed.
> So we need to keep the stripe_head out of any batch during that time.
> So I've added an extra state bit.
>
> Could you please review and possibly test the patch below?
>
> >
> > And STRIPE_ON_UNPLUG_LIST and STRIPE_ON_RELEASE_LIST are set is legit in
> > break_stripe_batch_list(), they should be removed from the WARN_ON_ONCE().
>
> Yes, you are right. Thanks.
>
> >
> > Thanks,
> > Shaohua
>
> Thanks,
> NeilBrown
>
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 041341c66ae5..89d6faafffda 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -760,6 +760,7 @@ static void unlock_two_stripes(struct stripe_head *sh1, struct stripe_head *sh2)
> static bool stripe_can_batch(struct stripe_head *sh)
> {
> return test_bit(STRIPE_BATCH_READY, &sh->state) &&
> + !test_bit(STRIPE_BITMAP_PENDING, &sh->state) &&
> is_full_stripe_write(sh);
> }
>
> @@ -3007,14 +3008,18 @@ static int add_stripe_bio(struct stripe_head *sh, struct bio *bi, int dd_idx,
> pr_debug("added bi b#%llu to stripe s#%llu, disk %d.\n",
> (unsigned long long)(*bip)->bi_iter.bi_sector,
> (unsigned long long)sh->sector, dd_idx);
> - spin_unlock_irq(&sh->stripe_lock);
>
> if (conf->mddev->bitmap && firstwrite) {
> + set_bit(STRIPE_BITMAP_PENDING, &sh->state);
> + spin_unlock_irq(&sh->stripe_lock);
> bitmap_startwrite(conf->mddev->bitmap, sh->sector,
> STRIPE_SECTORS, 0);
> + spin_lock_irq(&sh->stripe_lock);
> + clear_bit(STRIPE_BITMAP_PENDING, &sh->state);
> sh->bm_seq = conf->seq_flush+1;
> set_bit(STRIPE_BIT_DELAY, &sh->state);
> }
> + spin_lock_irq(&sh->stripe_lock);
should be unlock here. I'll report back if anything is wrong.
Thanks,
Shaohua
^ permalink raw reply
* Re: [PATCH 2/8] md/raid5: Ensure a batch member is not handled prematurely.
From: NeilBrown @ 2015-05-26 23:34 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20150527083532.32486e08@notabene.brown>
[-- Attachment #1: Type: text/plain, Size: 3272 bytes --]
On Wed, 27 May 2015 08:35:32 +1000 NeilBrown <neilb@suse.de> wrote:
> Could you please review and possibly test the patch below?
>
well... that patch had a fairly obvious double-lock bug.
Try this one.
(oh, just saw your email that you spotted the lock bug :-)
NeilBrown
From: NeilBrown <neilb@suse.de>
Date: Wed, 27 May 2015 08:43:45 +1000
Subject: [PATCH] md/raid5: close race between STRIPE_BIT_DELAY and batching.
The first time a write is added to a stripe, we need to set the
bitmap bits (if a bitmap is active).
While doing that the stripe is not locked and other writes could
be added and then the stripe could be added to a batch.
Once it has entered the batch it is too large to set STRIPE_BIT_DELAY
as the batch head has taken over when the stripe will be written.
We cannot hold the spinlock while adding the bitmap bit,
so introduce a new stripe_head flag 'STRIPE_BITMAP_PENDING' which
indicates that adding to the bitmap is pending. This prevents
the stripe from being added to a batch.
Only the first thread to add a write to a stripe can set this bit,
so it is safe for it to clear it again when it is done.
Reported-by: Shaohua Li <shli@kernel.org>
Signed-off-by: NeilBrown <neilb@suse.de>
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 73b5376dad3b..dae587ecdf71 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -760,6 +760,7 @@ static void unlock_two_stripes(struct stripe_head *sh1, struct stripe_head *sh2)
static bool stripe_can_batch(struct stripe_head *sh)
{
return test_bit(STRIPE_BATCH_READY, &sh->state) &&
+ !test_bit(STRIPE_BITMAP_PENDING, &sh->state) &&
is_full_stripe_write(sh);
}
@@ -3007,14 +3008,27 @@ static int add_stripe_bio(struct stripe_head *sh, struct bio *bi, int dd_idx,
pr_debug("added bi b#%llu to stripe s#%llu, disk %d.\n",
(unsigned long long)(*bip)->bi_iter.bi_sector,
(unsigned long long)sh->sector, dd_idx);
- spin_unlock_irq(&sh->stripe_lock);
if (conf->mddev->bitmap && firstwrite) {
+ /* Cannot hold spinlock over bitmap_startwrite,
+ * but must ensure this isn't added to a batch until
+ * we have added to the bitmap and set bm_seq.
+ * So set STRIPE_BITMAP_PENDING to prevent
+ * batching.
+ * Only the first thread to add a write to a stripe
+ * can set this bit, so we "own" it.
+ */
+ WARN_ON(test_bit(STRIPE_BITMAP_PENDING, &sh->state));
+ set_bit(STRIPE_BITMAP_PENDING, &sh->state);
+ spin_unlock_irq(&sh->stripe_lock);
bitmap_startwrite(conf->mddev->bitmap, sh->sector,
STRIPE_SECTORS, 0);
+ spin_lock_irq(&sh->stripe_lock);
+ clear_bit(STRIPE_BITMAP_PENDING, &sh->state);
sh->bm_seq = conf->seq_flush+1;
set_bit(STRIPE_BIT_DELAY, &sh->state);
}
+ spin_unlock_irq(&sh->stripe_lock);
if (stripe_can_batch(sh))
stripe_add_to_batch_list(conf, sh);
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index d7b2bc8b756f..02c3bf8fbfe7 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -337,6 +337,9 @@ enum {
STRIPE_ON_RELEASE_LIST,
STRIPE_BATCH_READY,
STRIPE_BATCH_ERR,
+ STRIPE_BITMAP_PENDING, /* Being added to bitmap, don't add
+ * to batch yet.
+ */
};
#define STRIPE_EXPAND_SYNC_FLAGS \
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply related
* Re: [PATCH v4 06/11] md/raid5: get rid of bio_fits_rdev()
From: Ming Lin @ 2015-05-26 23:42 UTC (permalink / raw)
To: NeilBrown
Cc: Ming Lin, Christoph Hellwig, lkml, Kent Overstreet, Jens Axboe,
Dongsu Park, linux-raid
In-Reply-To: <20150527090309.2cddf33b@notabene.brown>
On Tue, May 26, 2015 at 4:03 PM, NeilBrown <neilb@suse.de> wrote:
> On Tue, 26 May 2015 15:32:38 -0700 Ming Lin <mlin@kernel.org> wrote:
>
>> On Tue, May 26, 2015 at 7:33 AM, Ming Lin <mlin@kernel.org> wrote:
>> > On Mon, May 25, 2015 at 7:17 AM, Christoph Hellwig <hch@lst.de> wrote:
>> >> On Mon, May 25, 2015 at 05:54:14PM +1000, NeilBrown wrote:
>> >>> Did I write that? I guess I did :-(
>> >>> I meant *after*. Don't get rid of bio_fits_rdev until split_bio is in
>> >>> chunk_aligned_read().
>> >>
>> >> I suspect the whole series could use some reordering.
>> >
>> > Nice reordering.
>> > I'll do this.
>>
>> Here is the reordering.
>> https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=block-generic-req
>>
>> I'll post it if you are OK.
>>
>> [PATCH 01/15] block: add blk_queue_split()
>> [PATCH 02/15] md: remove ->merge_bvec_fn
>> [PATCH 03/15] dm: remov merge functions
>> [PATCH 04/15] drbd: remove ->merge_bvec_fn
>> [PATCH 05/15] pktcdvd: remove ->merge_bvec_fn
>> [PATCH 06/15] rbd: remove ->merge_bvec_fn
>> [PATCH 07/15] bcache: remove driver private bio splitting code
>> [PATCH 08/15] btrfs: remove bio splitting and merge_bvec_fn() calls
>> [PATCH 09/15] block: call blk_queue_split() in make_request functions
>> [PATCH 10/15] block: kill ->merge_bvec_fn and simplify bio_add_page
>> [PATCH 11/15] block: remove split code in blkdev_issue_discard
>> [PATCH 12/15] md/raid5: get rid of bio_fits_rdev()
>> [PATCH 13/15] block: remove bio_get_nr_vecs()
>> [PATCH 14/15] fs: use helper bio_add_page() instead of open coding on
>> [PATCH 15/15] Documentation: update notes in biovecs about
>
> The changes to dm.c and dm.h should be in the "dm:" patch, not "md:".
Will move it.
>
> But I don't think the sequence is right.
>
> You cannot remove ->merge_bvec_fn for *any* stacked device until *all* devices
> make use of blk_queue_split() (or otherwise handle arbitrarily large bios).
>
> I think it would be easiest to:
> - add blk_queue_split() and call it from common code before ->make_request_fn
> is called. The ensure all devices can accept arbitrarily large bios.
For "common code", do you mean "generic_make_request()"
diff --git a/block/blk-core.c b/block/blk-core.c
index fbbb337..bb6455b 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1942,6 +1942,7 @@ void generic_make_request(struct bio *bio)
do {
struct request_queue *q = bdev_get_queue(bio->bi_bdev);
+ blk_queue_split(q, &bio, q->bio_split);
q->make_request_fn(q, bio);
bio = bio_list_pop(current->bio_list);
> - driver-by-driver remove merge_bvec_fn and make sure the the driver can cope
> with arbitrary bios themselve, calling blk_queue_split in the make_request
> function only if needed
> - finally remove the call to blk_queue_split from the common code.
>
> Does that make sense to others?
>
> Thanks,
> NeilBrown
>
>>
>> >
>> > Thanks.
>> >
>> >>
>> >> patch 1:
>> >>
>> >> add ->bio_split and blk_queue_split
>> >>
>> >> patch 2..n:
>> >>
>> >> one for each non-trivial driver that implements ->merge_bvec_fn to
>> >> remove it and instead split bios in ->make_request. The md patch
>> >> to do the right thing in chunk_aligned_read goes into the general
>> >> md patch here. The bcache patch also goes into this series.
>> >>
>> >> patch n+1:
>> >>
>> >> - add blk_queue_split calls for remaining trivial drivers
>> >>
>> >> patch n+2:
>> >>
>> >> - remove ->merge_bvec_fn and checking of max_sectors a for all
>> >> drivers, simplify bio_add_page
>> >>
>> >> patch n+2:
>> >>
>> >> - remove splitting in blkdev_issue_discard
>> >>
>> >> patch n+3
>> >>
>> >> - remove bio_fits_rdev
>> >>
>> >> patch n+4
>> >>
>> >> - remove bio_get_nr_vecs
>> >>
>> >> patch n+4
>> >>
>> >> - use bio_add_page
>> >>
>> >> patch n+5
>> >>
>> >> - update documentation
>
^ permalink raw reply related
* Re: raid5 reshape is stuck
From: NeilBrown @ 2015-05-27 0:02 UTC (permalink / raw)
To: Xiao Ni; +Cc: linux-raid
In-Reply-To: <427651758.4121803.1432637303447.JavaMail.zimbra@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 6987 bytes --]
On Tue, 26 May 2015 06:48:23 -0400 (EDT) Xiao Ni <xni@redhat.com> wrote:
> > >
> > > In the function continue_via_systemd the parent find pid is bigger than
> > > 0 and
> > > status is 0. So it return 1. So it have no opportunity to call
> > > child_monitor.
> >
> > If continue_via_systemd succeeded, that implies that
> > systemctl start mdadm-grow-continue@mdXXX.service
> >
> > succeeded. So
> > mdadm --grow --continue /dev/mdXXX
> >
> > was run, so that mdadm should call 'child_monitor' and update sync_max when
> > appropriate. Can you check if it does?
>
> The service is not running.
>
> [root@intel-waimeabay-hedt-01 create_assemble]# systemctl start mdadm-grow-continue@md0.service
> [root@intel-waimeabay-hedt-01 create_assemble]# echo $?
> 0
> [root@intel-waimeabay-hedt-01 create_assemble]# systemctl status mdadm-grow-continue@md0.service
> mdadm-grow-continue@md0.service - Manage MD Reshape on /dev/md0
> Loaded: loaded (/usr/lib/systemd/system/mdadm-grow-continue@.service; static)
> Active: failed (Result: exit-code) since Tue 2015-05-26 05:33:59 EDT; 21s ago
> Process: 5374 ExecStart=/usr/sbin/mdadm --grow --continue /dev/%I (code=exited, status=1/FAILURE)
> Main PID: 5374 (code=exited, status=1/FAILURE)
>
> May 26 05:33:59 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Started Manage MD Reshape on /dev/md0.
> May 26 05:33:59 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: mdadm-grow-continue@md0.service: main process exited, ...URE
> May 26 05:33:59 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Unit mdadm-grow-continue@md0.service entered failed state.
> Hint: Some lines were ellipsized, use -l to show in full.
Hmm.. I wonder why systemctl isn't reporting the error message from mdadm.
>
> [root@intel-waimeabay-hedt-01 create_assemble]# mdadm --grow --continue /dev/md0 --backup-file=tmp0
> mdadm: Need to backup 6144K of critical section..
>
> Now the reshape start.
>
> Try modify the service file :
> ExecStart=/usr/sbin/mdadm --grow --continue /dev/%I --backup-file=/root/tmp0
>
> It doesn't work too.
I tried that change and it make it work.
>
> [root@intel-waimeabay-hedt-01 ~]# systemctl daemon-reload
> [root@intel-waimeabay-hedt-01 ~]# systemctl start mdadm-grow-continue@md0.service
> [root@intel-waimeabay-hedt-01 ~]# systemctl status mdadm-grow-continue@md0.service
> mdadm-grow-continue@md0.service - Manage MD Reshape on /dev/md0
> Loaded: loaded (/usr/lib/systemd/system/mdadm-grow-continue@.service; static)
> Active: failed (Result: exit-code) since Tue 2015-05-26 05:50:22 EDT; 10s ago
> Process: 6475 ExecStart=/usr/sbin/mdadm --grow --continue /dev/%I --backup-file=/root/tmp0 (code=exited, status=1/FAILURE)
> Main PID: 6475 (code=exited, status=1/FAILURE)
>
> May 26 05:50:22 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Started Manage MD Reshape on /dev/md0.
> May 26 05:50:22 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: mdadm-grow-continue@md0.service: main process exited, ...URE
> May 26 05:50:22 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Unit mdadm-grow-continue@md0.service entered failed state.
> Hint: Some lines were ellipsized, use -l to show in full.
>
>
>
> >
> >
> > >
> > >
> > > And if it want to set sync_max to 0 until the backup has been taken. Why
> > > does not
> > > set sync_max to 0 directly, but use the value reshape_progress? There is a
> > > little confused.
> >
> > When reshaping an array to a different array of the same size, such as a
> > 4-driver RAID5 to a 5-driver RAID6, then mdadm needs to backup, one piece at
> > a time, the entire array (unless it can change data_offset, which is a
> > relatively new ability).
> >
> > If you stop an array when it is in the middle of such a reshape, and then
> > reassemble the array, the backup process need to recommence where it left
> > off.
> > So it tells the kernel that the reshape can progress as far as where it was
> > up to before. So 'sync_max' is set based on the value of 'reshape_progress'.
> > (This will happen almost instantly).
> >
> > Then the background mdadm (or the mdadm started by systemd) will backup the
> > next few stripes, update sync_max, wait for those stripes to be reshaped,
> > then
> > discard the old backup, create a new one of the few stripes after that, and
> > continue.
> >
> > Does that make it a little clearer?
>
> This is a big dinner for me. I need digest this for a while. Thanks very much
> for this. What's the "backup process"?
>
> Could you explain backup in detail. I read the man about backup file.
>
> When relocating the first few stripes on a RAID5 or RAID6, it is not possible to keep the data on disk completely
> consistent and crash-proof. To provide the required safety, mdadm disables writes to the array while this "critical
> section" is reshaped, and takes a backup of the data that is in that section.
>
> What's the reason about data consistent when relocate data?
If you are reshaping a RAID5 from 3 drives to 4 drives, then the first stripe
will start out as:
D0 D1 P -
and you want to change it to
D0 D1 D2 P
If the system crashes while that is happening, you won't know if either or
both of D2 and P were written, but it is fairly safe just to assume they
weren't and recalculate the parity.
However the second stripe will initially be:
P D2 D3
and you want to change it to
P D3 D4 D5
If you crash in the middle of doing that you cannot know which block is D3
- if either. D4 might have been written, and D3 not yet written. So D3 is
lost.
So mdadm takes a copy of a whole stripe, allows the kernel to reshape that
one stripe, updates the metadata to record that the stripe has been fully
reshaped, and then discards the backup.
So if you crash in the middle of reshaping the second stripe above, mdadm
will restore it from the backup.
The backup can be stored in a separate file, or in a device which is being
added to the array.
The reason why "mdadm --grow --continue" doesn't work unless you add the
"--backup=...." is because it doesn't find the "device being added" - it
looks for a spare, but there aren't any spares any more. That should be
easy enough to fix.
Thanks,
NeilBrown
>
> >
> > And in response to your other email:
> > > Does it should return 1 when pid > 0 and status is not zero?
> >
> > No. continue_via_systemd should return 1 precisely when the 'systemctl'
> > command was successfully run. So 'status' must be zero.
> >
> >
>
> I got this. So reshape_array should return when continue_via_systemd return 1. Then the
> reshape is going on when run the command mdadm --grow --continue. Now the child_monitor
> is called and sync_max is set to max.
>
> Best Regards
> Xiao
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 2/8] md/raid5: Ensure a batch member is not handled prematurely.
From: Shaohua Li @ 2015-05-27 0:10 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <20150527093451.0c83be44@notabene.brown>
On Wed, May 27, 2015 at 09:34:51AM +1000, NeilBrown wrote:
> On Wed, 27 May 2015 08:35:32 +1000 NeilBrown <neilb@suse.de> wrote:
>
> > Could you please review and possibly test the patch below?
> >
>
> well... that patch had a fairly obvious double-lock bug.
> Try this one.
> (oh, just saw your email that you spotted the lock bug :-)
>
>
> NeilBrown
>
> From: NeilBrown <neilb@suse.de>
> Date: Wed, 27 May 2015 08:43:45 +1000
> Subject: [PATCH] md/raid5: close race between STRIPE_BIT_DELAY and batching.
>
> The first time a write is added to a stripe, we need to set the
> bitmap bits (if a bitmap is active).
> While doing that the stripe is not locked and other writes could
> be added and then the stripe could be added to a batch.
> Once it has entered the batch it is too large to set STRIPE_BIT_DELAY
> as the batch head has taken over when the stripe will be written.
>
> We cannot hold the spinlock while adding the bitmap bit,
> so introduce a new stripe_head flag 'STRIPE_BITMAP_PENDING' which
> indicates that adding to the bitmap is pending. This prevents
> the stripe from being added to a batch.
>
> Only the first thread to add a write to a stripe can set this bit,
> so it is safe for it to clear it again when it is done.
>
> Reported-by: Shaohua Li <shli@kernel.org>
> Signed-off-by: NeilBrown <neilb@suse.de>
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 73b5376dad3b..dae587ecdf71 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -760,6 +760,7 @@ static void unlock_two_stripes(struct stripe_head *sh1, struct stripe_head *sh2)
> static bool stripe_can_batch(struct stripe_head *sh)
> {
> return test_bit(STRIPE_BATCH_READY, &sh->state) &&
> + !test_bit(STRIPE_BITMAP_PENDING, &sh->state) &&
> is_full_stripe_write(sh);
> }
>
> @@ -3007,14 +3008,27 @@ static int add_stripe_bio(struct stripe_head *sh, struct bio *bi, int dd_idx,
> pr_debug("added bi b#%llu to stripe s#%llu, disk %d.\n",
> (unsigned long long)(*bip)->bi_iter.bi_sector,
> (unsigned long long)sh->sector, dd_idx);
> - spin_unlock_irq(&sh->stripe_lock);
>
> if (conf->mddev->bitmap && firstwrite) {
> + /* Cannot hold spinlock over bitmap_startwrite,
> + * but must ensure this isn't added to a batch until
> + * we have added to the bitmap and set bm_seq.
> + * So set STRIPE_BITMAP_PENDING to prevent
> + * batching.
> + * Only the first thread to add a write to a stripe
> + * can set this bit, so we "own" it.
> + */
> + WARN_ON(test_bit(STRIPE_BITMAP_PENDING, &sh->state));
I keep hitting this. the firstwrite is set for every device.
Thanks,
Shaohua
^ permalink raw reply
* Re: [PATCH 2/8] md/raid5: Ensure a batch member is not handled prematurely.
From: NeilBrown @ 2015-05-27 0:36 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20150527001005.GA106894@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 6431 bytes --]
On Tue, 26 May 2015 17:10:05 -0700 Shaohua Li <shli@kernel.org> wrote:
> On Wed, May 27, 2015 at 09:34:51AM +1000, NeilBrown wrote:
> > On Wed, 27 May 2015 08:35:32 +1000 NeilBrown <neilb@suse.de> wrote:
> >
> > > Could you please review and possibly test the patch below?
> > >
> >
> > well... that patch had a fairly obvious double-lock bug.
> > Try this one.
> > (oh, just saw your email that you spotted the lock bug :-)
> >
> >
> > NeilBrown
> >
> > From: NeilBrown <neilb@suse.de>
> > Date: Wed, 27 May 2015 08:43:45 +1000
> > Subject: [PATCH] md/raid5: close race between STRIPE_BIT_DELAY and batching.
> >
> > The first time a write is added to a stripe, we need to set the
> > bitmap bits (if a bitmap is active).
> > While doing that the stripe is not locked and other writes could
> > be added and then the stripe could be added to a batch.
> > Once it has entered the batch it is too large to set STRIPE_BIT_DELAY
> > as the batch head has taken over when the stripe will be written.
> >
> > We cannot hold the spinlock while adding the bitmap bit,
> > so introduce a new stripe_head flag 'STRIPE_BITMAP_PENDING' which
> > indicates that adding to the bitmap is pending. This prevents
> > the stripe from being added to a batch.
> >
> > Only the first thread to add a write to a stripe can set this bit,
> > so it is safe for it to clear it again when it is done.
> >
> > Reported-by: Shaohua Li <shli@kernel.org>
> > Signed-off-by: NeilBrown <neilb@suse.de>
> >
> > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > index 73b5376dad3b..dae587ecdf71 100644
> > --- a/drivers/md/raid5.c
> > +++ b/drivers/md/raid5.c
> > @@ -760,6 +760,7 @@ static void unlock_two_stripes(struct stripe_head *sh1, struct stripe_head *sh2)
> > static bool stripe_can_batch(struct stripe_head *sh)
> > {
> > return test_bit(STRIPE_BATCH_READY, &sh->state) &&
> > + !test_bit(STRIPE_BITMAP_PENDING, &sh->state) &&
> > is_full_stripe_write(sh);
> > }
> >
> > @@ -3007,14 +3008,27 @@ static int add_stripe_bio(struct stripe_head *sh, struct bio *bi, int dd_idx,
> > pr_debug("added bi b#%llu to stripe s#%llu, disk %d.\n",
> > (unsigned long long)(*bip)->bi_iter.bi_sector,
> > (unsigned long long)sh->sector, dd_idx);
> > - spin_unlock_irq(&sh->stripe_lock);
> >
> > if (conf->mddev->bitmap && firstwrite) {
> > + /* Cannot hold spinlock over bitmap_startwrite,
> > + * but must ensure this isn't added to a batch until
> > + * we have added to the bitmap and set bm_seq.
> > + * So set STRIPE_BITMAP_PENDING to prevent
> > + * batching.
> > + * Only the first thread to add a write to a stripe
> > + * can set this bit, so we "own" it.
> > + */
> > + WARN_ON(test_bit(STRIPE_BITMAP_PENDING, &sh->state));
> I keep hitting this. the firstwrite is set for every device.
>
I wonder why I'm not.... maybe because I'm using /dev/loop devices and they
behave a bit differently.
Of course 'firstwrite' is not per-stripe, it is per-device-in-a-stripe.
So it will be set for every device.
Which is rather pointless really, we only need to set the bitmap once for the
whole stripe. Maybe it was easier the other way.
Could you try this?
Thanks!
NeilBrown
From: NeilBrown <neilb@suse.de>
Date: Wed, 27 May 2015 08:43:45 +1000
Subject: [PATCH] md/raid5: close race between STRIPE_BIT_DELAY and batching.
When we add a write to a stripe we need to make sure the bitmap
bit is set. While doing that the stripe is not locked so it could
be added to a batch after which further changes to STRIPE_BIT_DELAY
and ->bm_seq are ineffective.
So we need to hold off adding to a stripe until bitmap_startwrite has
completed at least once, and we need to avoid further changes to
STRIPE_BIT_DELAY once the stripe has been added to a batch.
If a bitmap_startwrite() completes after the stripe was added to a
batch, it will not have set the bit, only incremented a counter, so no
extra delay of the stripe is needed.
Reported-by: Shaohua Li <shli@kernel.org>
Signed-off-by: NeilBrown <neilb@suse.de>
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 73b5376dad3b..5d5ce97c2210 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -760,6 +760,7 @@ static void unlock_two_stripes(struct stripe_head *sh1, struct stripe_head *sh2)
static bool stripe_can_batch(struct stripe_head *sh)
{
return test_bit(STRIPE_BATCH_READY, &sh->state) &&
+ !test_bit(STRIPE_BITMAP_PENDING, &sh->state) &&
is_full_stripe_write(sh);
}
@@ -3007,14 +3008,32 @@ static int add_stripe_bio(struct stripe_head *sh, struct bio *bi, int dd_idx,
pr_debug("added bi b#%llu to stripe s#%llu, disk %d.\n",
(unsigned long long)(*bip)->bi_iter.bi_sector,
(unsigned long long)sh->sector, dd_idx);
- spin_unlock_irq(&sh->stripe_lock);
if (conf->mddev->bitmap && firstwrite) {
+ /* Cannot hold spinlock over bitmap_startwrite,
+ * but must ensure this isn't added to a batch until
+ * we have added to the bitmap and set bm_seq.
+ * So set STRIPE_BITMAP_PENDING to prevent
+ * batching.
+ * If multiple add_stripe_bio() calls race here they
+ * much all set STRIPE_BITMAP_PENDING. So only the first one
+ * to complete "bitmap_startwrite" gets to set
+ * STRIPE_BIT_DELAY. This is important as once a stripe
+ * is added to a batch, STRIPE_BIT_DELAY cannot be changed
+ * any more.
+ */
+ set_bit(STRIPE_BITMAP_PENDING, &sh->state);
+ spin_unlock_irq(&sh->stripe_lock);
bitmap_startwrite(conf->mddev->bitmap, sh->sector,
STRIPE_SECTORS, 0);
- sh->bm_seq = conf->seq_flush+1;
- set_bit(STRIPE_BIT_DELAY, &sh->state);
+ spin_lock_irq(&sh->stripe_lock);
+ clear_bit(STRIPE_BITMAP_PENDING, &sh->state);
+ if (!sh->batch_head) {
+ sh->bm_seq = conf->seq_flush+1;
+ set_bit(STRIPE_BIT_DELAY, &sh->state);
+ }
}
+ spin_unlock_irq(&sh->stripe_lock);
if (stripe_can_batch(sh))
stripe_add_to_batch_list(conf, sh);
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index d7b2bc8b756f..02c3bf8fbfe7 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -337,6 +337,9 @@ enum {
STRIPE_ON_RELEASE_LIST,
STRIPE_BATCH_READY,
STRIPE_BATCH_ERR,
+ STRIPE_BITMAP_PENDING, /* Being added to bitmap, don't add
+ * to batch yet.
+ */
};
#define STRIPE_EXPAND_SYNC_FLAGS \
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply related
* Re: [PATCH v4 06/11] md/raid5: get rid of bio_fits_rdev()
From: NeilBrown @ 2015-05-27 0:38 UTC (permalink / raw)
To: Ming Lin
Cc: Christoph Hellwig, lkml, Kent Overstreet, Jens Axboe, Dongsu Park,
linux-raid
In-Reply-To: <CAF1ivSZee_aV4XOtgc9YOR4YG4Lsd0tJkE-F+i9jg-5Ep7_Afg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2988 bytes --]
On Tue, 26 May 2015 16:42:35 -0700 Ming Lin <mlin@kernel.org> wrote:
> On Tue, May 26, 2015 at 4:03 PM, NeilBrown <neilb@suse.de> wrote:
> > On Tue, 26 May 2015 15:32:38 -0700 Ming Lin <mlin@kernel.org> wrote:
> >
> >> On Tue, May 26, 2015 at 7:33 AM, Ming Lin <mlin@kernel.org> wrote:
> >> > On Mon, May 25, 2015 at 7:17 AM, Christoph Hellwig <hch@lst.de> wrote:
> >> >> On Mon, May 25, 2015 at 05:54:14PM +1000, NeilBrown wrote:
> >> >>> Did I write that? I guess I did :-(
> >> >>> I meant *after*. Don't get rid of bio_fits_rdev until split_bio is in
> >> >>> chunk_aligned_read().
> >> >>
> >> >> I suspect the whole series could use some reordering.
> >> >
> >> > Nice reordering.
> >> > I'll do this.
> >>
> >> Here is the reordering.
> >> https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=block-generic-req
> >>
> >> I'll post it if you are OK.
> >>
> >> [PATCH 01/15] block: add blk_queue_split()
> >> [PATCH 02/15] md: remove ->merge_bvec_fn
> >> [PATCH 03/15] dm: remov merge functions
> >> [PATCH 04/15] drbd: remove ->merge_bvec_fn
> >> [PATCH 05/15] pktcdvd: remove ->merge_bvec_fn
> >> [PATCH 06/15] rbd: remove ->merge_bvec_fn
> >> [PATCH 07/15] bcache: remove driver private bio splitting code
> >> [PATCH 08/15] btrfs: remove bio splitting and merge_bvec_fn() calls
> >> [PATCH 09/15] block: call blk_queue_split() in make_request functions
> >> [PATCH 10/15] block: kill ->merge_bvec_fn and simplify bio_add_page
> >> [PATCH 11/15] block: remove split code in blkdev_issue_discard
> >> [PATCH 12/15] md/raid5: get rid of bio_fits_rdev()
> >> [PATCH 13/15] block: remove bio_get_nr_vecs()
> >> [PATCH 14/15] fs: use helper bio_add_page() instead of open coding on
> >> [PATCH 15/15] Documentation: update notes in biovecs about
> >
> > The changes to dm.c and dm.h should be in the "dm:" patch, not "md:".
>
> Will move it.
>
> >
> > But I don't think the sequence is right.
> >
> > You cannot remove ->merge_bvec_fn for *any* stacked device until *all* devices
> > make use of blk_queue_split() (or otherwise handle arbitrarily large bios).
> >
> > I think it would be easiest to:
> > - add blk_queue_split() and call it from common code before ->make_request_fn
> > is called. The ensure all devices can accept arbitrarily large bios.
>
> For "common code", do you mean "generic_make_request()"
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index fbbb337..bb6455b 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -1942,6 +1942,7 @@ void generic_make_request(struct bio *bio)
> do {
> struct request_queue *q = bdev_get_queue(bio->bi_bdev);
>
> + blk_queue_split(q, &bio, q->bio_split);
> q->make_request_fn(q, bio);
>
> bio = bio_list_pop(current->bio_list);
Yes, that is what I mean (assuming that is the only place that calls
->make_request_fn).
Thanks,
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: raid5 reshape is stuck
From: NeilBrown @ 2015-05-27 1:10 UTC (permalink / raw)
To: Xiao Ni; +Cc: linux-raid
In-Reply-To: <20150527100253.221ab553@notabene.brown>
[-- Attachment #1: Type: text/plain, Size: 7996 bytes --]
On Wed, 27 May 2015 10:02:53 +1000 NeilBrown <neilb@suse.de> wrote:
> On Tue, 26 May 2015 06:48:23 -0400 (EDT) Xiao Ni <xni@redhat.com> wrote:
>
>
> > > >
> > > > In the function continue_via_systemd the parent find pid is bigger than
> > > > 0 and
> > > > status is 0. So it return 1. So it have no opportunity to call
> > > > child_monitor.
> > >
> > > If continue_via_systemd succeeded, that implies that
> > > systemctl start mdadm-grow-continue@mdXXX.service
> > >
> > > succeeded. So
> > > mdadm --grow --continue /dev/mdXXX
> > >
> > > was run, so that mdadm should call 'child_monitor' and update sync_max when
> > > appropriate. Can you check if it does?
> >
> > The service is not running.
> >
> > [root@intel-waimeabay-hedt-01 create_assemble]# systemctl start mdadm-grow-continue@md0.service
> > [root@intel-waimeabay-hedt-01 create_assemble]# echo $?
> > 0
> > [root@intel-waimeabay-hedt-01 create_assemble]# systemctl status mdadm-grow-continue@md0.service
> > mdadm-grow-continue@md0.service - Manage MD Reshape on /dev/md0
> > Loaded: loaded (/usr/lib/systemd/system/mdadm-grow-continue@.service; static)
> > Active: failed (Result: exit-code) since Tue 2015-05-26 05:33:59 EDT; 21s ago
> > Process: 5374 ExecStart=/usr/sbin/mdadm --grow --continue /dev/%I (code=exited, status=1/FAILURE)
> > Main PID: 5374 (code=exited, status=1/FAILURE)
> >
> > May 26 05:33:59 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Started Manage MD Reshape on /dev/md0.
> > May 26 05:33:59 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: mdadm-grow-continue@md0.service: main process exited, ...URE
> > May 26 05:33:59 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Unit mdadm-grow-continue@md0.service entered failed state.
> > Hint: Some lines were ellipsized, use -l to show in full.
>
> Hmm.. I wonder why systemctl isn't reporting the error message from mdadm.
>
>
> >
> > [root@intel-waimeabay-hedt-01 create_assemble]# mdadm --grow --continue /dev/md0 --backup-file=tmp0
> > mdadm: Need to backup 6144K of critical section..
> >
> > Now the reshape start.
> >
> > Try modify the service file :
> > ExecStart=/usr/sbin/mdadm --grow --continue /dev/%I --backup-file=/root/tmp0
> >
> > It doesn't work too.
>
> I tried that change and it make it work.
>
> >
> > [root@intel-waimeabay-hedt-01 ~]# systemctl daemon-reload
> > [root@intel-waimeabay-hedt-01 ~]# systemctl start mdadm-grow-continue@md0.service
> > [root@intel-waimeabay-hedt-01 ~]# systemctl status mdadm-grow-continue@md0.service
> > mdadm-grow-continue@md0.service - Manage MD Reshape on /dev/md0
> > Loaded: loaded (/usr/lib/systemd/system/mdadm-grow-continue@.service; static)
> > Active: failed (Result: exit-code) since Tue 2015-05-26 05:50:22 EDT; 10s ago
> > Process: 6475 ExecStart=/usr/sbin/mdadm --grow --continue /dev/%I --backup-file=/root/tmp0 (code=exited, status=1/FAILURE)
> > Main PID: 6475 (code=exited, status=1/FAILURE)
> >
> > May 26 05:50:22 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Started Manage MD Reshape on /dev/md0.
> > May 26 05:50:22 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: mdadm-grow-continue@md0.service: main process exited, ...URE
> > May 26 05:50:22 intel-waimeabay-hedt-01.lab.eng.rdu.redhat.com systemd[1]: Unit mdadm-grow-continue@md0.service entered failed state.
> > Hint: Some lines were ellipsized, use -l to show in full.
> >
> >
> >
> > >
> > >
> > > >
> > > >
> > > > And if it want to set sync_max to 0 until the backup has been taken. Why
> > > > does not
> > > > set sync_max to 0 directly, but use the value reshape_progress? There is a
> > > > little confused.
> > >
> > > When reshaping an array to a different array of the same size, such as a
> > > 4-driver RAID5 to a 5-driver RAID6, then mdadm needs to backup, one piece at
> > > a time, the entire array (unless it can change data_offset, which is a
> > > relatively new ability).
> > >
> > > If you stop an array when it is in the middle of such a reshape, and then
> > > reassemble the array, the backup process need to recommence where it left
> > > off.
> > > So it tells the kernel that the reshape can progress as far as where it was
> > > up to before. So 'sync_max' is set based on the value of 'reshape_progress'.
> > > (This will happen almost instantly).
> > >
> > > Then the background mdadm (or the mdadm started by systemd) will backup the
> > > next few stripes, update sync_max, wait for those stripes to be reshaped,
> > > then
> > > discard the old backup, create a new one of the few stripes after that, and
> > > continue.
> > >
> > > Does that make it a little clearer?
> >
> > This is a big dinner for me. I need digest this for a while. Thanks very much
> > for this. What's the "backup process"?
> >
> > Could you explain backup in detail. I read the man about backup file.
> >
> > When relocating the first few stripes on a RAID5 or RAID6, it is not possible to keep the data on disk completely
> > consistent and crash-proof. To provide the required safety, mdadm disables writes to the array while this "critical
> > section" is reshaped, and takes a backup of the data that is in that section.
> >
> > What's the reason about data consistent when relocate data?
>
> If you are reshaping a RAID5 from 3 drives to 4 drives, then the first stripe
> will start out as:
>
> D0 D1 P -
>
> and you want to change it to
>
> D0 D1 D2 P
>
> If the system crashes while that is happening, you won't know if either or
> both of D2 and P were written, but it is fairly safe just to assume they
> weren't and recalculate the parity.
> However the second stripe will initially be:
>
> P D2 D3
>
> and you want to change it to
>
> P D3 D4 D5
>
> If you crash in the middle of doing that you cannot know which block is D3
> - if either. D4 might have been written, and D3 not yet written. So D3 is
> lost.
>
> So mdadm takes a copy of a whole stripe, allows the kernel to reshape that
> one stripe, updates the metadata to record that the stripe has been fully
> reshaped, and then discards the backup.
> So if you crash in the middle of reshaping the second stripe above, mdadm
> will restore it from the backup.
>
> The backup can be stored in a separate file, or in a device which is being
> added to the array.
>
>
> The reason why "mdadm --grow --continue" doesn't work unless you add the
> "--backup=...." is because it doesn't find the "device being added" - it
> looks for a spare, but there aren't any spares any more. That should be
> easy enough to fix.
That wasn't too painful - I think this fixes the problem.
Could you confirm?
Thanks,
NeilBrown
diff --git a/Grow.c b/Grow.c
index a20ff3e70142..85de1d27f03a 100644
--- a/Grow.c
+++ b/Grow.c
@@ -850,7 +850,8 @@ int reshape_prepare_fdlist(char *devname,
for (sd = sra->devs; sd; sd = sd->next) {
if (sd->disk.state & (1<<MD_DISK_FAULTY))
continue;
- if (sd->disk.state & (1<<MD_DISK_SYNC)) {
+ if (sd->disk.state & (1<<MD_DISK_SYNC) &&
+ sd->disk.raid_disk < raid_disks) {
char *dn = map_dev(sd->disk.major,
sd->disk.minor, 1);
fdlist[sd->disk.raid_disk]
@@ -3184,7 +3185,7 @@ started:
d = reshape_prepare_fdlist(devname, sra, odisks,
nrdisks, blocks, backup_file,
fdlist, offsets);
- if (d < 0) {
+ if (d < odisks) {
goto release;
}
if ((st->ss->manage_reshape == NULL) ||
@@ -3196,7 +3197,7 @@ started:
devname);
pr_err(" Please provide one with \"--backup=...\"\n");
goto release;
- } else if (sra->array.spare_disks == 0) {
+ } else if (d == odisks) {
pr_err("%s: Cannot grow - need a spare or backup-file to backup critical section\n", devname);
goto release;
}
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply related
* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Mikael Abrahamsson @ 2015-05-27 4:59 UTC (permalink / raw)
To: Wols Lists; +Cc: Another Sillyname, linux-raid
In-Reply-To: <5564D373.30705@youngman.org.uk>
On Tue, 26 May 2015, Wols Lists wrote:
> And you're wasting a lot of disk space. What's important to you (that a
> live swap disk shouldn't fail) is not important to me and doesn't seem
> to be important to Another Sillyname.
I am not so sure. He seems to want "maximum resiliance", or at least
that's what he's saying.
> I don't know why Another Sillyname wants to chain his swap partitions,
> but it's his choice. Maybe like me, his system gets rebooted a couple of
> times a day, and failure to start is a far more real risk than failure
> in use. He's running Fedora - that seems likely then ...
Yes, it's his choice, but he should also make this choice understanding
what he's doing, so it's an informed and corrent choice. When I read his
postings, I am not so sure this is the case.
--
Mikael Abrahamsson email: swmike@swm.pp.se
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox