* Re: write performance of HW RAID VS MD RAID
From: Neil Brown @ 2015-06-10 23:00 UTC (permalink / raw)
To: Ming Lin; +Cc: linux-raid
In-Reply-To: <CAF1ivSa644NVVd5R=2g8rvbZ-FK5cgXy_U1XJjYwMf+TJdftCw@mail.gmail.com>
On Wed, 10 Jun 2015 15:27:07 -0700
Ming Lin <mlin@kernel.org> wrote:
> Hi NeilBrown,
>
> As you may already see, I run a lot of tests with 10 HDDs for the patchset
> "simplify block layer based on immutable biovecs"
>
> Here is the summary.
> http://minggr.net/pub/20150608/fio_results/summary.log
>
> MD RAID6 read performance is OK.
> But write performance is much lower than HW RAID6.
>
> Is it a known issue?
It is not unexpected.
There are two likely reasons.
One is that HW RAID cards often have on-board NVRAM which is used as a
write-behind cache. This allows better throughput by hiding latency and more
often gathering full-stripe writes. HW RAID cards may also have accelerators
for the parity calculations, but that is not likely to make a big difference.
What sort of RAID6 controller do you have?
The other is that it is not easy for MD/RAID6 to schedule writes stripes
optimally. It doesn't really know if more writes are coming, so it should
wait, or if it already has everything - so it should get to work straight away.
It is possible that it could reply to writes as soon as they are in the
(volatile) cache and only force things to storage when a REQ_FUA or REQ_FLUSH
arrives. That might help ... or it might corrupt filesystems :-(
As long as the patches don't make things obviously worse, I'm happy.
Thanks,
NeilBrown
^ permalink raw reply
* write performance of HW RAID VS MD RAID
From: Ming Lin @ 2015-06-10 22:27 UTC (permalink / raw)
To: linux-raid; +Cc: Neil Brown
Hi NeilBrown,
As you may already see, I run a lot of tests with 10 HDDs for the patchset
"simplify block layer based on immutable biovecs"
Here is the summary.
http://minggr.net/pub/20150608/fio_results/summary.log
MD RAID6 read performance is OK.
But write performance is much lower than HW RAID6.
Is it a known issue?
Thanks.
^ permalink raw reply
* Re: 4.1-rc6 radi5 OOPS
From: Jes Sorensen @ 2015-06-10 21:02 UTC (permalink / raw)
To: Neil Brown; +Cc: linux-raid, Xiao Ni
In-Reply-To: <20150610101942.0bc26a25@home.neil.brown.name>
Neil Brown <neilb@suse.de> writes:
> On Wed, 03 Jun 2015 17:57:43 -0400
> Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
>
>> NeilBrown <neilb@suse.de> writes:
>> > On Wed, 03 Jun 2015 16:20:21 -0400 Jes Sorensen
>> > <Jes.Sorensen@redhat.com> wrote:
>> >
>> >> Neil,
>> >>
>> >> I was running testing on the current 4.1-rc6 tree (Linus' top of
>> >> trunk 8cd9234c64c584432f6992fe944ca9e46ca8ea76) and I am seeing
>> >> the following OOPS which is reproducible.
>> >>
>> >> It shows up when running the mdadm test suite, 07changelevelintr
>> >> to be specific.
>> >>
>> >> Is this something you have seen?
>> >>
>> >> Cheers,
>> >> Jes
>> >>
>> >> ------------[ cut here ]------------
>> >> kernel BUG at drivers/md/raid5.c:5391!
>> >
>> > No, I haven't seen that. And I've been running the test suite
>> > quite a bit lately.
>> >
>> > Can you get it to print out the relevant numbers? Include
>> > readpos/writepos/safepos too.
>>
>> This enough? Let me know if you need more.
>>
>> I suspect this started happening with the changes that went in between
>> 4.1-rc5 and 4.1-rc6. I will try to bisect it tomorrow.
>>
>> Cheers,
>> Jes
>>
>> mddev->dev_sectors: 0x9800, reshape_sectors: 0x0200 stripe_addr:
>> fffffffffffffdff, sector_nr 0, readpos 511, writepos -513, safepos
>> 512
>
> These numbers suggest that conf->reshape_progress divided by
> "data_disks" or "new_data_disks" is -1 - or really the unsigned
> equivalent, which is MaxSectors.
> But unless data_disks is 1, ->reshape_progress must really be -2 or -3
> or something.
> So maybe if you could confirm the values of ->reshape_progress,
> data_disks, and new_data_disks, that might help.
>
>
> I don't think ->reshape_progress could get a negative value in any way
> except by being assigned MaxSectors. And that only happens when the
> reshape has really completely finished.
>
> So it looks like some sort of race. I have other evidence of a race
> with the resync/reshape thread starting/stopping. If I track that
> down it'll probably fix this issue too.
Hi Neil,
I added the debug output you asked for - this is what I got. Looks like
reshape_progress did get set to -1.
Cheers,
Jes
mddev->dev_sectors: 0x9800, reshape_sectors: 0x0200 reshape_progress 0xffffffffffffffff, stripe_addr: fffffffffffffdff, sector_nr 0, readpos 511, writepos -513, safepos 512, data_disks 1, new_data_disks 1
------------[ cut here ]------------
kernel BUG at drivers/md/raid5.c:5363!
invalid opcode: 0000 [#1] SMP
Modules linked in: raid456 async_raid6_recov async_memcpy async_pq async_xor xor async_tx raid6_pq nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4 iptable_filter ip_tables tun bridge stp llc xfs x86_pkg_temp_thermal coretemp kvm_intel kvm crc32c_intel ghash_clmulni_intel aesni_intel aes_x86_64 libcrc32c glue_helper lrw gf128mul ablk_helper cryptd iTCO_wdt microcode iTCO_vendor_support raid0 ppdev pcspkr parport_pc nfsd parport shpchp i2c_i801 video i2c_core acpi_cpufreq lpc_ich mfd_core auth_rpcgss oid_registry exportfs nfs_acl lockd grace uinput sunrpc ext4 mbcache jbd2 sd_mod ahci libahci e1000e ptp pps_core r8169 mii dm_mirror dm_region_hash dm_log dm_mod ipv6 autofs4
CPU: 3 PID: 15216 Comm: md0_resync Not tainted 4.1.0-rc2+ #6
Hardware name: Intel Corporation S1200BTL/S1200BTL, BIOS S1200BT.86B.02.00.0035.030220120927 03/02/2012
task: ffff8800a582f040 ti: ffff8800bd410000 task.ti: ffff8800bd410000
RIP: 0010:[<ffffffffa04003a3>] [<ffffffffa04003a3>] reshape_request+0x8e3/0x8f0 [raid456]
RSP: 0018:ffff8800bd413b48 EFLAGS: 00010296
RAX: 00000000000000cc RBX: 0000000000000001 RCX: 0000000000000000
RDX: 0000000000000001 RSI: ffff88023ee6d368 RDI: ffff88023ee6d368
RBP: ffff8800bd413c28 R08: 0000000000000400 R09: ffffffff81d6c864
R10: 000000000000058b R11: 000000000000058a R12: ffff8800bd413d0c
R13: 0000000000000000 R14: ffff8800bd413d0c R15: ffff8800bd6d6000
FS: 0000000000000000(0000) GS:ffff88023ee60000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f32b2cc7000 CR3: 0000000001a0b000 CR4: 00000000001406e0
Stack:
00000000000001ff fffffffffffffdff 0000000000000200 ffff880200000001
0000000000000001 0000000000010fb0 0000000000000000 0000000000000200
0000000000000001 0000000000000000 0000000000000003 fffffffffffffdff
Call Trace:
[<ffffffff815b2d33>] ? __schedule+0x383/0x8e0
[<ffffffffa04006de>] sync_request+0x32e/0x3a0 [raid456]
[<ffffffff81092708>] ? __wake_up+0x48/0x60
[<ffffffff8148b564>] md_do_sync+0x8f4/0xe90
[<ffffffff810779cc>] ? update_rq_clock.part.89+0x1c/0x40
[<ffffffff81487888>] md_thread+0x128/0x140
[<ffffffff81487760>] ? find_pers+0x80/0x80
[<ffffffff81487760>] ? find_pers+0x80/0x80
[<ffffffff81071b49>] kthread+0xc9/0xe0
[<ffffffff810edd76>] ? __audit_syscall_exit+0x1e6/0x280
[<ffffffff81071a80>] ? kthread_create_on_node+0x170/0x170
[<ffffffff815b6b92>] ret_from_fork+0x42/0x70
[<ffffffff81071a80>] ? kthread_create_on_node+0x170/0x170
Code: 85 78 ff ff ff 4c 8b 8d 68 ff ff ff 8b 55 84 4c 89 04 24 89 5c 24 20 44 89 5c 24 18 48 89 44 24 08 49 89 c0 31 c0 e8 8d c4 1a e1 <0f> 0b 66 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 41 57 41 56
RIP [<ffffffffa04003a3>] reshape_request+0x8e3/0x8f0 [raid456]
RSP <ffff8800bd413b48>
---[ end trace f745eac38e148690 ]---
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<ffffffff81073c2f>] exit_creds+0x1f/0x70
PGD a58ee067 PUD bd4f7067 PMD 0
Oops: 0000 [#2] SMP
Modules linked in: raid456 async_raid6_recov async_memcpy async_pq async_xor xor async_tx raid6_pq nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4 iptable_filter ip_tables tun bridge stp llc xfs x86_pkg_temp_thermal coretemp kvm_intel kvm crc32c_intel ghash_clmulni_intel aesni_intel aes_x86_64 libcrc32c glue_helper lrw gf128mul ablk_helper cryptd iTCO_wdt microcode iTCO_vendor_support raid0 ppdev pcspkr parport_pc nfsd parport shpchp i2c_i801 video i2c_core acpi_cpufreq lpc_ich mfd_core auth_rpcgss oid_registry exportfs nfs_acl lockd grace uinput sunrpc ext4 mbcache jbd2 sd_mod ahci libahci e1000e ptp pps_core r8169 mii dm_mirror dm_region_hash dm_log dm_mod ipv6 autofs4
CPU: 1 PID: 15096 Comm: mdadm Tainted: G D 4.1.0-rc2+ #6
Hardware name: Intel Corporation S1200BTL/S1200BTL, BIOS S1200BT.86B.02.00.0035.030220120927 03/02/2012
task: ffff880233e17000 ti: ffff8800365a4000 task.ti: ffff8800365a4000
RIP: 0010:[<ffffffff81073c2f>] [<ffffffff81073c2f>] exit_creds+0x1f/0x70
RSP: 0018:ffff8800365a7ca8 EFLAGS: 00010292
RAX: 0000000000000000 RBX: ffff8800a582f040 RCX: ffff8800365a7d00
RDX: 0000000000006a70 RSI: 0000000000000296 RDI: 0000000000000000
RBP: ffff8800365a7cb8 R08: 0000000000000000 R09: 0000000000000000
R10: 000000000000000b R11: 0000000000000246 R12: ffff8800a582f040
R13: 0000000000000000 R14: ffff8800a5d16000 R15: 0000000000000004
FS: 00007f3e1e2dc740(0000) GS:ffff88023ee20000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 00000000bd1b6000 CR4: 00000000001406e0
Stack:
ffff8800365a7cb8 ffff8800a582f040 ffff8800365a7cd8 ffffffff8105265a
0000000000000000 ffff8800a582f040 ffff8800365a7d08 ffffffff81072058
ffff880200000006 ffff880234cb5cc0 ffff8800bd6d6150 0000000000000004
Call Trace:
[<ffffffff8105265a>] __put_task_struct+0x4a/0x130
[<ffffffff81072058>] kthread_stop+0x88/0x100
[<ffffffff814878e5>] md_unregister_thread+0x45/0x80
[<ffffffff8148e32d>] md_reap_sync_thread+0x1d/0x1a0
[<ffffffff8148e650>] action_store+0x1a0/0x290
[<ffffffff8105cd5d>] ? ns_capable+0x2d/0x60
[<ffffffff8148bb7b>] md_attr_store+0x7b/0xd0
[<ffffffff8120f45d>] sysfs_kf_write+0x3d/0x50
[<ffffffff8120ebba>] kernfs_fop_write+0x12a/0x180
[<ffffffff81199878>] __vfs_write+0x28/0xf0
[<ffffffff8119c479>] ? __sb_start_write+0x49/0xf0
[<ffffffff81230873>] ? security_file_permission+0x23/0xa0
[<ffffffff81199f69>] vfs_write+0xa9/0x1b0
[<ffffffff8119ad36>] SyS_write+0x46/0xb0
[<ffffffff810edb34>] ? __audit_syscall_entry+0xb4/0x110
[<ffffffff815b67d7>] system_call_fastpath+0x12/0x6a
Code: 0f 84 37 fe ff ff e9 10 fe ff ff 90 0f 1f 44 00 00 55 48 89 e5 53 48 89 fb 48 83 ec 08 48 8b 87 c8 09 00 00 48 8b bf c0 09 00 00 <8b> 00 48 c7 83 c0 09 00 00 00 00 00 00 f0 ff 0f 74 1f 48 8b bb
RIP [<ffffffff81073c2f>] exit_creds+0x1f/0x70
RSP <ffff8800365a7ca8>
CR2: 0000000000000000
---[ end trace f745eac38e148691 ]---
^ permalink raw reply
* Re: [PATCH] md: fix a build warning
From: Neil Brown @ 2015-06-10 20:38 UTC (permalink / raw)
To: Julia Lawall; +Cc: walter harms, Firo Yang, linux-raid, kernel-janitors
In-Reply-To: <alpine.DEB.2.10.1506101935390.5463@hadrien>
On Wed, 10 Jun 2015 19:36:43 +0200 (CEST)
Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Wed, 10 Jun 2015, walter harms wrote:
>
> >
> >
> > Am 10.06.2015 17:20, schrieb Firo Yang:
> > > Warning like this:
> > >
> > > drivers/md/md.c: In function ‘update_array_info’:
> > > drivers/md/md.c:6394:26: warning: logical not is only applied
> > > to the left hand side of comparison [-Wlogical-not-parentheses]
> > > !mddev->persistent != info->not_persistent||
> > >
> > > I fix it by enclosing !mddev->persistent with parentheses
> > >
> > > By the way, I also fixed a line over 80 characters warning outputed
> > > by ./scripts/checkpatch.pl
> > >
> > > Signed-off-by: Firo Yang <firogm@gmail.com>
> > > ---
> > > drivers/md/md.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > > index dd85be9..b420d82 100644
> > > --- a/drivers/md/md.c
> > > +++ b/drivers/md/md.c
> > > @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
> > > mddev->ctime != info->ctime ||
> > > mddev->level != info->level ||
> > > /* mddev->layout != info->layout || */
> > > - !mddev->persistent != info->not_persistent||
> > > + (!mddev->persistent) != info->not_persistent ||
> >
> >
> > this looks odd,
> > would it be possible the check for == instead (and drop the !) ?
> > and it someone care for readability: It would be helpful to
> > make some more ifs here.
>
> The original issue looks like a false positive. If all of the other cases
> have no parentheses on the left and use !=, isn't it better to leave the
> code as is?
>
Leaving the code as it is would leave at least one compiler generating warnings.
I don't like warnings.
So if the compiler can be "fixed" that would be good. But it seems unlikely.
May we could do:
> > > + mddev->persistent != !info->not_persistent ||
That would keep the two "not"s together, avoid the need for parentheses, and
probably avoid the compiler warning.
(I think I had it the way it is because it almost reads
" not ...peristent != ... not_persistent"
but that isn't a strong reason)
Firo: Can you confirm that this version doesn't upset the compiler?
If if doesn't I'd definitely prefer this one. (and definitely leave out the
comment change - just change the code)
Thanks,
NeilBrown
> julia
>
> >
> > re,
> > wh
> >
> > > mddev->chunk_sectors != info->chunk_size >> 9 ||
> > > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */
> > > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT
> > > + to change */
> > > ((state^info->state) & 0xfffffe00)
> > > )
> > > return -EINVAL;
> > --
> > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
--
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: [PATCH] md: fix a build warning
From: Neil Brown @ 2015-06-10 20:34 UTC (permalink / raw)
To: walter harms; +Cc: Firo Yang, linux-raid, kernel-janitors
In-Reply-To: <557874B9.9010500@bfs.de>
On Wed, 10 Jun 2015 19:32:41 +0200
walter harms <wharms@bfs.de> wrote:
>
>
> Am 10.06.2015 17:20, schrieb Firo Yang:
> > Warning like this:
> >
> > drivers/md/md.c: In function ‘update_array_info’:
> > drivers/md/md.c:6394:26: warning: logical not is only applied
> > to the left hand side of comparison [-Wlogical-not-parentheses]
> > !mddev->persistent != info->not_persistent||
> >
> > I fix it by enclosing !mddev->persistent with parentheses
> >
> > By the way, I also fixed a line over 80 characters warning outputed
> > by ./scripts/checkpatch.pl
> >
> > Signed-off-by: Firo Yang <firogm@gmail.com>
> > ---
> > drivers/md/md.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index dd85be9..b420d82 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
> > mddev->ctime != info->ctime ||
> > mddev->level != info->level ||
> > /* mddev->layout != info->layout || */
> > - !mddev->persistent != info->not_persistent||
> > + (!mddev->persistent) != info->not_persistent ||
>
>
> this looks odd,
> would it be possible the check for == instead (and drop the !) ?
> and it someone care for readability: It would be helpful to
> make some more ifs here.
The first attempt Firo summited did exactly that. I said no.
I like the visual consistency of LHS != RHS.
More 'if's would just waste more vertical space.
Thanks,
NeilBrown
>
> re,
> wh
>
> > mddev->chunk_sectors != info->chunk_size >> 9 ||
> > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */
> > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT
> > + to change */
> > ((state^info->state) & 0xfffffe00)
> > )
> > return -EINVAL;
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" 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: Migrating a RAID 5 from 4x2TB to 3x6TB ?
From: Pierre Wieser @ 2015-06-10 19:37 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <167089395.613.1433791723592.JavaMail.zimbra@wieser.fr>
Hi all,
> I currently have an almost full RAID 5 built with 4 x 2 TB disks.
> I wonder if it would be possible to migrate it to a bigger RAID 5
> with 3 x 6TB new disks.
Due to all suggestions, and I'd wish another time thank all contributions,
I've spent some hours reading the mailing list archives, surfing the web
with more appropiate keywords, and so on..
So, here what I now plan to do:
First, I hace cancelled my order for the new 6TB desktop-grade disks,
replacing it with 4TB WD RedPro, and one 6TB desktop-grade (see below its use)
As the full RAID5 array I planned to migrate is already my backup system,
I cannot rely on a restore :(. So the first thing is to rsync the current
array to the directly attached 6TB disk. I don't thing I have a free SATA
port on my motherboard, but at worst I will be able to use the one currently
used for the DVD drive.
I've chosen to build new RAID10 arrays.
I've moved away the RAID6 suggestion due to its known bad write performance,
and also because I'm willing/able to put a bit more money to get better perfs.
The 4x4TB new disks will be partitioned as:
- 512MB to be a RAID1 array mounted as /boot
- 8GB to be a RAID10 array used as swap
- two 25 GB parts to be two RAID10 arrays used as root filesystem
(plus place for an alternate when upgrading the OS)
- the rest of the disk will be splitted in four equal parts (about 930 MB
I think), each of which being member of a separate data RAID10 array.
I am conscious that this seems as a waste of space, and especially for the
/boot partition. But this scheme will let me:
a) have banalized disks: all disks have same rules, are partitioned identically
b) replace my system disk which is not part of any RAID system as of today,
thus gaining actually both a SATA port for the RAID systems and more security
for the boot and root filesystems
c) also because I use to use LVM on top of RAID to get advantages of its
flexibility (so several PVs which may or may not be aggregated later)
Other suggestions include the use of smartctl tool. I've checked that the
daemon was already running. But I didn't use the '-x' option that I understand
is hardly an option !
I plan to build these RAID devices out of CentOS 7 standard install process
(I'm currently downloading a CentOS Live iso), thus presenting to the install
some predefined partitions.
I expect about 5-10 days to get these orders delivered. So more news at this time :)
Thank you all for your help. I keep reading the list that I discovered for
the occasion...
Regards
Pierre
^ permalink raw reply
* Re: block: add a bi_error field to struct bio
From: anup modak @ 2015-06-10 18:29 UTC (permalink / raw)
To: Mike Snitzer
Cc: Christoph Hellwig, Jens Axboe, linux-raid, dm-devel, linux-btrfs
In-Reply-To: <20150610165023.GA31918@redhat.com>
So Is it safe to use raid 6 with btrfs for home storage now? I will
have backup on ZFS.
On Wed, Jun 10, 2015 at 11:50 AM, Mike Snitzer <snitzer@redhat.com> wrote:
> On Wed, Jun 10 2015 at 12:04pm -0400,
> Christoph Hellwig <hch@lst.de> wrote:
>
>> On Wed, Jun 10, 2015 at 12:01:12PM -0400, Mike Snitzer wrote:
>> > > I'll queue a patch to rename 'error' to 'error_bits' where appropriate.
>> >
>> > See: https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=f368f463e4cef696ad6b102dbaf5c10dfca7cc63
>>
>> Can we wait with this until we're done with bi_error?
>
> Sure, I've moved it out to dm-4.3
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" 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: [PATCH] md: fix a build warning
From: walter harms @ 2015-06-10 18:04 UTC (permalink / raw)
To: Julia Lawall; +Cc: Firo Yang, neilb, linux-raid, kernel-janitors
In-Reply-To: <alpine.DEB.2.10.1506101935390.5463@hadrien>
Am 10.06.2015 19:36, schrieb Julia Lawall:
>
>
> On Wed, 10 Jun 2015, walter harms wrote:
>
>>
>>
>> Am 10.06.2015 17:20, schrieb Firo Yang:
>>> Warning like this:
>>>
>>> drivers/md/md.c: In function ‘update_array_info’:
>>> drivers/md/md.c:6394:26: warning: logical not is only applied
>>> to the left hand side of comparison [-Wlogical-not-parentheses]
>>> !mddev->persistent != info->not_persistent||
>>>
>>> I fix it by enclosing !mddev->persistent with parentheses
>>>
>>> By the way, I also fixed a line over 80 characters warning outputed
>>> by ./scripts/checkpatch.pl
>>>
>>> Signed-off-by: Firo Yang <firogm@gmail.com>
>>> ---
>>> drivers/md/md.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>> index dd85be9..b420d82 100644
>>> --- a/drivers/md/md.c
>>> +++ b/drivers/md/md.c
>>> @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
>>> mddev->ctime != info->ctime ||
>>> mddev->level != info->level ||
>>> /* mddev->layout != info->layout || */
>>> - !mddev->persistent != info->not_persistent||
>>> + (!mddev->persistent) != info->not_persistent ||
>>
>>
>> this looks odd,
>> would it be possible the check for == instead (and drop the !) ?
>> and it someone care for readability: It would be helpful to
>> make some more ifs here.
>
> The original issue looks like a false positive. If all of the other cases
> have no parentheses on the left and use !=, isn't it better to leave the
> code as is?
>
i do not think so, the name indicate persistent/not_persistent devices.
Maybe both parties can agree what should be stored, ppl are bad
at those not_not things and that is an unnecessary trouble spot.
re,
wh
> julia
>
>>
>> re,
>> wh
>>
>>> mddev->chunk_sectors != info->chunk_size >> 9 ||
>>> - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */
>>> + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT
>>> + to change */
>>> ((state^info->state) & 0xfffffe00)
>>> )
>>> return -EINVAL;
>> --
>> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" 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: [PATCH] md: fix a build warning
From: Julia Lawall @ 2015-06-10 17:36 UTC (permalink / raw)
To: walter harms; +Cc: Firo Yang, neilb, linux-raid, kernel-janitors
In-Reply-To: <557874B9.9010500@bfs.de>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2098 bytes --]
On Wed, 10 Jun 2015, walter harms wrote:
>
>
> Am 10.06.2015 17:20, schrieb Firo Yang:
> > Warning like this:
> >
> > drivers/md/md.c: In function ‘update_array_info’:
> > drivers/md/md.c:6394:26: warning: logical not is only applied
> > to the left hand side of comparison [-Wlogical-not-parentheses]
> > !mddev->persistent != info->not_persistent||
> >
> > I fix it by enclosing !mddev->persistent with parentheses
> >
> > By the way, I also fixed a line over 80 characters warning outputed
> > by ./scripts/checkpatch.pl
> >
> > Signed-off-by: Firo Yang <firogm@gmail.com>
> > ---
> > drivers/md/md.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index dd85be9..b420d82 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
> > mddev->ctime != info->ctime ||
> > mddev->level != info->level ||
> > /* mddev->layout != info->layout || */
> > - !mddev->persistent != info->not_persistent||
> > + (!mddev->persistent) != info->not_persistent ||
>
>
> this looks odd,
> would it be possible the check for == instead (and drop the !) ?
> and it someone care for readability: It would be helpful to
> make some more ifs here.
The original issue looks like a false positive. If all of the other cases
have no parentheses on the left and use !=, isn't it better to leave the
code as is?
julia
>
> re,
> wh
>
> > mddev->chunk_sectors != info->chunk_size >> 9 ||
> > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */
> > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT
> > + to change */
> > ((state^info->state) & 0xfffffe00)
> > )
> > return -EINVAL;
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" 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: [PATCH] md: fix a build warning
From: walter harms @ 2015-06-10 17:32 UTC (permalink / raw)
To: Firo Yang; +Cc: neilb, linux-raid, kernel-janitors
In-Reply-To: <1433949658-8455-1-git-send-email-firogm@gmail.com>
Am 10.06.2015 17:20, schrieb Firo Yang:
> Warning like this:
>
> drivers/md/md.c: In function ‘update_array_info’:
> drivers/md/md.c:6394:26: warning: logical not is only applied
> to the left hand side of comparison [-Wlogical-not-parentheses]
> !mddev->persistent != info->not_persistent||
>
> I fix it by enclosing !mddev->persistent with parentheses
>
> By the way, I also fixed a line over 80 characters warning outputed
> by ./scripts/checkpatch.pl
>
> Signed-off-by: Firo Yang <firogm@gmail.com>
> ---
> drivers/md/md.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index dd85be9..b420d82 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
> mddev->ctime != info->ctime ||
> mddev->level != info->level ||
> /* mddev->layout != info->layout || */
> - !mddev->persistent != info->not_persistent||
> + (!mddev->persistent) != info->not_persistent ||
this looks odd,
would it be possible the check for == instead (and drop the !) ?
and it someone care for readability: It would be helpful to
make some more ifs here.
re,
wh
> mddev->chunk_sectors != info->chunk_size >> 9 ||
> - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */
> + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT
> + to change */
> ((state^info->state) & 0xfffffe00)
> )
> return -EINVAL;
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" 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: block: add a bi_error field to struct bio
From: Mike Snitzer @ 2015-06-10 16:50 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, linux-raid, dm-devel, linux-btrfs
In-Reply-To: <20150610160443.GA14268@lst.de>
On Wed, Jun 10 2015 at 12:04pm -0400,
Christoph Hellwig <hch@lst.de> wrote:
> On Wed, Jun 10, 2015 at 12:01:12PM -0400, Mike Snitzer wrote:
> > > I'll queue a patch to rename 'error' to 'error_bits' where appropriate.
> >
> > See: https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=f368f463e4cef696ad6b102dbaf5c10dfca7cc63
>
> Can we wait with this until we're done with bi_error?
Sure, I've moved it out to dm-4.3
^ permalink raw reply
* Re: 4.1-rc6 radi5 OOPS
From: Jes Sorensen @ 2015-06-10 16:27 UTC (permalink / raw)
To: Neil Brown; +Cc: linux-raid, Xiao Ni
In-Reply-To: <20150610115721.64c474fa@home.neil.brown.name>
Neil Brown <neilb@suse.de> writes:
> On Wed, 10 Jun 2015 10:19:42 +1000 Neil Brown <neilb@suse.de> wrote:
>
>> So it looks like some sort of race. I have other evidence of a race
>> with the resync/reshape thread starting/stopping. If I track that
>> down it'll probably fix this issue too.
>
> I think I have found just such a race. If you request a reshape just
> as a recovery completes, you can end up with two reshapes running.
> This causes confusion :-)
>
> Can you try this patch? If I can remember how to reproduce my race
> I'll test it on that too.
>
> Thanks,
> NeilBrown
Hi Neil,
Thanks for the patch - I tried with this applied, but it still crashed
for me :( I had to mangle it manually, somehow it got modified in the
email.
Note this was a mangled RHEL kernel, but it's the same crash I see on
the upstream kernel.
[ 754.303561] md: using 128k window, over a total of 19456k.
[ 754.309706] mddev->dev_sectors: 0x9800, reshape_sectors: 0x0200 stripe_addr: fffffffffffffdff, sector_nr 0, readpos 511, writepos -513, safepos 512
[ 754.324486] ------------[ cut here ]------------
[ 754.329649] kernel BUG at drivers/md/raid5.c:5388!
Cheers,
Jes
^ permalink raw reply
* Re: [PATCH] md: fix a build warning
From: Dan Carpenter @ 2015-06-10 16:07 UTC (permalink / raw)
To: Firo Yang; +Cc: neilb, linux-raid, kernel-janitors
In-Reply-To: <1433949658-8455-1-git-send-email-firogm@gmail.com>
On Wed, Jun 10, 2015 at 11:20:58PM +0800, Firo Yang wrote:
> By the way, I also fixed a line over 80 characters warning outputed
> by ./scripts/checkpatch.pl
Don't do this. It's not on the same line, it's not really related at
all.
> mddev->chunk_sectors != info->chunk_size >> 9 ||
> - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */
> + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT
> + to change */
The new comment style isn't correct.
regards,
dan carpenter
^ permalink raw reply
* Re: block: add a bi_error field to struct bio
From: Christoph Hellwig @ 2015-06-10 16:04 UTC (permalink / raw)
To: Mike Snitzer
Cc: Christoph Hellwig, Jens Axboe, linux-raid, dm-devel, linux-btrfs
In-Reply-To: <20150610160112.GB31140@redhat.com>
On Wed, Jun 10, 2015 at 12:01:12PM -0400, Mike Snitzer wrote:
> > I'll queue a patch to rename 'error' to 'error_bits' where appropriate.
>
> See: https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=f368f463e4cef696ad6b102dbaf5c10dfca7cc63
Can we wait with this until we're done with bi_error?
^ permalink raw reply
* Re: block: add a bi_error field to struct bio
From: Mike Snitzer @ 2015-06-10 16:01 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, linux-raid, dm-devel, linux-btrfs
In-Reply-To: <20150610152649.GA31140@redhat.com>
On Wed, Jun 10 2015 at 11:26am -0400,
Mike Snitzer <snitzer@redhat.com> wrote:
> On Wed, Jun 10 2015 at 4:11am -0400,
> Christoph Hellwig <hch@lst.de> wrote:
>
> > On Thu, Jun 04, 2015 at 11:31:07AM -0400, Mike Snitzer wrote:
> > > This patch _really_ concerns me because just in DM alone I found you
> > > took liberties that you shouldn't have and created a regression. First
> > > issue is a real bug (your proposed dm-io.c:dmio_complete change missed
> > > that dm-io uses error_bits and not traditional error code like expected)
> >
> > Point taken. I already wanted to complain about the mess due to the bio
> > error abuse with it's own values in DM in the first posting, guess I
> > need to add that to the second one. I don't think overloading common
> > interfaces with your private error codes is a good idea, but let's
> > leave that for a separate discussion.
>
> I'll queue a patch to rename 'error' to 'error_bits' where appropriate.
See: https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=f368f463e4cef696ad6b102dbaf5c10dfca7cc63
^ permalink raw reply
* Re: block: add a bi_error field to struct bio
From: Mike Snitzer @ 2015-06-10 15:26 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, linux-raid, dm-devel, linux-btrfs
In-Reply-To: <20150610081138.GA3841@lst.de>
On Wed, Jun 10 2015 at 4:11am -0400,
Christoph Hellwig <hch@lst.de> wrote:
> On Thu, Jun 04, 2015 at 11:31:07AM -0400, Mike Snitzer wrote:
> > This patch _really_ concerns me because just in DM alone I found you
> > took liberties that you shouldn't have and created a regression. First
> > issue is a real bug (your proposed dm-io.c:dmio_complete change missed
> > that dm-io uses error_bits and not traditional error code like expected)
>
> Point taken. I already wanted to complain about the mess due to the bio
> error abuse with it's own values in DM in the first posting, guess I
> need to add that to the second one. I don't think overloading common
> interfaces with your private error codes is a good idea, but let's
> leave that for a separate discussion.
I'll queue a patch to rename 'error' to 'error_bits' where appropriate.
> > the other issue being you added extra branching that isn't needed and
> > made review more tedious (dm.c:clone_endio).
>
> I think the code is better than what it was before, but it's still
> a bit of a mess. What do you think of the patch below which I'd
> like to add before the big bi_error patch as a preparatory one?
If you're referring to the mix of error variables I totally agree. Just
don't think we need the extra branching.
> > For DM, please add Signed-off-by: Mike Snitzer <snitzer@redhat.com> once
> > you've folded in this patch, thanks!
>
> FYI, that wasn't a foldable patch but updated hunks of the old one. Not
> really a problem, but a little confusing.
Yeap, should have been clearer they were meant to replace your hunks.
> >From f095cbeba5135afa6cf102718319f0d0c1e7b422 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Wed, 10 Jun 2015 10:04:45 +0200
> Subject: dm: use a single error code variable in clone_endio
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> clone_endio currently uses two variables for tracking error state, with
> values getting bounceѕ forth and back between the two, which makes the
> code hard to read.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/md/dm.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 2161ed9..8467976 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -956,7 +956,6 @@ static void disable_write_same(struct mapped_device *md)
>
> static void clone_endio(struct bio *bio, int error)
> {
> - int r = error;
> struct dm_target_io *tio = container_of(bio, struct dm_target_io, clone);
> struct dm_io *io = tio->io;
> struct mapped_device *md = tio->io->md;
> @@ -966,23 +965,22 @@ static void clone_endio(struct bio *bio, int error)
> error = -EIO;
>
> if (endio) {
> - r = endio(tio->ti, bio, error);
> - if (r < 0 || r == DM_ENDIO_REQUEUE)
> - /*
> - * error and requeue request are handled
> - * in dec_pending().
> - */
> - error = r;
> - else if (r == DM_ENDIO_INCOMPLETE)
> + error = endio(tio->ti, bio, error);
> + if (error == DM_ENDIO_INCOMPLETE) {
> /* The target will handle the io */
> return;
> - else if (r) {
> - DMWARN("unimplemented target endio return value: %d", r);
> + }
> +
> + if (error > 0 && error != DM_ENDIO_REQUEUE) {
> + DMWARN("unimplemented target endio return value: %d",
> + error);
> BUG();
> }
> +
> + /* Error and requeue request are handled in dec_pending(). */
> }
>
> - if (unlikely(r == -EREMOTEIO && (bio->bi_rw & REQ_WRITE_SAME) &&
> + if (unlikely(error == -EREMOTEIO && (bio->bi_rw & REQ_WRITE_SAME) &&
> !bdev_get_queue(bio->bi_bdev)->limits.max_write_same_sectors))
> disable_write_same(md);
>
> --
> 1.9.1
Unfortunately by dropping the original error (e.g. -EREMOTEIO) on the
floor (in the 'if (endio) {' branch) you're breaking the REQ_WRITE_SAME
check.
Your new bi_error patch gets away with the redundant error code cleanup
because we can directly check the bio's bi_error for -EREMOTEIO. So
feel free to fold the simplified 'if (error > 0 && error != DM_ENDIO_REQUEUE) {'
in to your new patch -- but not seeing the point of making this prep
patch in advance.
--
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
* [PATCH] md: fix a build warning
From: Firo Yang @ 2015-06-10 15:20 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, kernel-janitors, Firo Yang
Warning like this:
drivers/md/md.c: In function ‘update_array_info’:
drivers/md/md.c:6394:26: warning: logical not is only applied
to the left hand side of comparison [-Wlogical-not-parentheses]
!mddev->persistent != info->not_persistent||
I fix it by enclosing !mddev->persistent with parentheses
By the way, I also fixed a line over 80 characters warning outputed
by ./scripts/checkpatch.pl
Signed-off-by: Firo Yang <firogm@gmail.com>
---
drivers/md/md.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index dd85be9..b420d82 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
mddev->ctime != info->ctime ||
mddev->level != info->level ||
/* mddev->layout != info->layout || */
- !mddev->persistent != info->not_persistent||
+ (!mddev->persistent) != info->not_persistent ||
mddev->chunk_sectors != info->chunk_size >> 9 ||
- /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */
+ /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT
+ to change */
((state^info->state) & 0xfffffe00)
)
return -EINVAL;
--
2.4.2
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" 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 related
* Re: [dm-devel] [PATCH] block: add a bi_error field to struct bio
From: Christoph Hellwig @ 2015-06-10 8:45 UTC (permalink / raw)
To: Neil Brown
Cc: Christoph Hellwig, Jens Axboe, linux-raid, dm-devel, linux-btrfs
In-Reply-To: <20150610125054.3da2abcc@home.neil.brown.name>
On Wed, Jun 10, 2015 at 12:50:54PM +1000, Neil Brown wrote:
> This introduces a use-after-free. put_buf(r1_bio) can result in bio_put on
> 'bio'.
> It is safe to move the put_buf call after the md_done_sync(), but it is
> probably best to leave the 'update' variable as it. i.e. Just change:
>
> - int uptodate = test_bit(BIO_UPTODATE, &bio->bi_flags);
> + int uptodate = !bio->bi_error;
>
>
> I can't see any other problems with the md changes.
Thanks, I'll keep the local uptodate variable for now.
^ permalink raw reply
* Re: block: add a bi_error field to struct bio
From: Christoph Hellwig @ 2015-06-10 8:11 UTC (permalink / raw)
To: Mike Snitzer; +Cc: Jens Axboe, linux-raid, dm-devel, linux-btrfs
In-Reply-To: <20150604153106.GA31567@redhat.com>
On Thu, Jun 04, 2015 at 11:31:07AM -0400, Mike Snitzer wrote:
> This patch _really_ concerns me because just in DM alone I found you
> took liberties that you shouldn't have and created a regression. First
> issue is a real bug (your proposed dm-io.c:dmio_complete change missed
> that dm-io uses error_bits and not traditional error code like expected)
Point taken. I already wanted to complain about the mess due to the bio
error abuse with it's own values in DM in the first posting, guess I
need to add that to the second one. I don't think overloading common
interfaces with your private error codes is a good idea, but let's
leave that for a separate discussion.
> the other issue being you added extra branching that isn't needed and
> made review more tedious (dm.c:clone_endio).
I think the code is better than what it was before, but it's still
a bit of a mess. What do you think of the patch below which I'd
like to add before the big bi_error patch as a preparatory one?
> For DM, please add Signed-off-by: Mike Snitzer <snitzer@redhat.com> once
> you've folded in this patch, thanks!
FYI, that wasn't a foldable patch but updated hunks of the old one. Not
really a problem, but a little confusing.
From f095cbeba5135afa6cf102718319f0d0c1e7b422 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Wed, 10 Jun 2015 10:04:45 +0200
Subject: dm: use a single error code variable in clone_endio
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
clone_endio currently uses two variables for tracking error state, with
values getting bounceѕ forth and back between the two, which makes the
code hard to read.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/md/dm.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 2161ed9..8467976 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -956,7 +956,6 @@ static void disable_write_same(struct mapped_device *md)
static void clone_endio(struct bio *bio, int error)
{
- int r = error;
struct dm_target_io *tio = container_of(bio, struct dm_target_io, clone);
struct dm_io *io = tio->io;
struct mapped_device *md = tio->io->md;
@@ -966,23 +965,22 @@ static void clone_endio(struct bio *bio, int error)
error = -EIO;
if (endio) {
- r = endio(tio->ti, bio, error);
- if (r < 0 || r == DM_ENDIO_REQUEUE)
- /*
- * error and requeue request are handled
- * in dec_pending().
- */
- error = r;
- else if (r == DM_ENDIO_INCOMPLETE)
+ error = endio(tio->ti, bio, error);
+ if (error == DM_ENDIO_INCOMPLETE) {
/* The target will handle the io */
return;
- else if (r) {
- DMWARN("unimplemented target endio return value: %d", r);
+ }
+
+ if (error > 0 && error != DM_ENDIO_REQUEUE) {
+ DMWARN("unimplemented target endio return value: %d",
+ error);
BUG();
}
+
+ /* Error and requeue request are handled in dec_pending(). */
}
- if (unlikely(r == -EREMOTEIO && (bio->bi_rw & REQ_WRITE_SAME) &&
+ if (unlikely(error == -EREMOTEIO && (bio->bi_rw & REQ_WRITE_SAME) &&
!bdev_get_queue(bio->bi_bdev)->limits.max_write_same_sectors))
disable_write_same(md);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" 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 related
* Re: RAID1 removing failed disk returns EBUSY
From: XiaoNi @ 2015-06-10 6:26 UTC (permalink / raw)
To: NeilBrown; +Cc: Joe Lawrence, linux-raid, Bill Kuzeja
In-Reply-To: <1914953233.3814567.1422951056539.JavaMail.zimbra@redhat.com>
On 02/03/2015 04:10 PM, Xiao Ni wrote:
>
> ----- Original Message -----
>> From: "NeilBrown" <neilb@suse.de>
>> To: "Xiao Ni" <xni@redhat.com>
>> Cc: "Joe Lawrence" <joe.lawrence@stratus.com>, linux-raid@vger.kernel.org, "Bill Kuzeja" <william.kuzeja@stratus.com>
>> Sent: Monday, February 2, 2015 2:36:01 PM
>> Subject: Re: RAID1 removing failed disk returns EBUSY
>>
>> On Thu, 29 Jan 2015 07:14:16 -0500 (EST) Xiao Ni <xni@redhat.com> wrote:
>>
>>>
>>> ----- Original Message -----
>>>> From: "NeilBrown" <neilb@suse.de>
>>>> To: "Xiao Ni" <xni@redhat.com>
>>>> Cc: "Joe Lawrence" <joe.lawrence@stratus.com>,
>>>> linux-raid@vger.kernel.org, "Bill Kuzeja" <william.kuzeja@stratus.com>
>>>> Sent: Thursday, January 29, 2015 11:52:17 AM
>>>> Subject: Re: RAID1 removing failed disk returns EBUSY
>>>>
>>>> On Sun, 18 Jan 2015 21:33:50 -0500 (EST) Xiao Ni <xni@redhat.com> wrote:
>>>>
>>>>>
>>>>> ----- Original Message -----
>>>>>> From: "Joe Lawrence" <joe.lawrence@stratus.com>
>>>>>> To: "Xiao Ni" <xni@redhat.com>
>>>>>> Cc: "NeilBrown" <neilb@suse.de>, linux-raid@vger.kernel.org, "Bill
>>>>>> Kuzeja" <william.kuzeja@stratus.com>
>>>>>> Sent: Friday, January 16, 2015 11:10:31 PM
>>>>>> Subject: Re: RAID1 removing failed disk returns EBUSY
>>>>>>
>>>>>> On Fri, 16 Jan 2015 00:20:12 -0500
>>>>>> Xiao Ni <xni@redhat.com> wrote:
>>>>>>> Hi Joe
>>>>>>>
>>>>>>> Thanks for reminding me. I didn't do that. Now it can remove
>>>>>>> successfully after writing
>>>>>>> "idle" to sync_action.
>>>>>>>
>>>>>>> I thought wrongly that the patch referenced in this mail is
>>>>>>> fixed
>>>>>>> for
>>>>>>> the problem.
>>>>>> So it sounds like even with 3.18 and a new mdadm, this bug still
>>>>>> persists?
>>>>>>
>>>>>> -- Joe
>>>>>>
>>>>>> --
>>>>> Hi Joe
>>>>>
>>>>> I'm a little confused now. Does the patch
>>>>> 45eaf45dfa4850df16bc2e8e7903d89021137f40 from linux-stable
>>>>> resolve the problem?
>>>>>
>>>>> My environment is:
>>>>>
>>>>> [root@dhcp-12-133 mdadm]# mdadm --version
>>>>> mdadm - v3.3.2-18-g93d3bd3 - 18th December 2014 (this is the newest
>>>>> upstream)
>>>>> [root@dhcp-12-133 mdadm]# uname -r
>>>>> 3.18.2
>>>>>
>>>>>
>>>>> My steps are:
>>>>>
>>>>> [root@dhcp-12-133 mdadm]# lsblk
>>>>> sdb 8:16 0 931.5G 0 disk
>>>>> └─sdb1 8:17 0 5G 0 part
>>>>> sdc 8:32 0 186.3G 0 disk
>>>>> sdd 8:48 0 931.5G 0 disk
>>>>> └─sdd1 8:49 0 5G 0 part
>>>>> [root@dhcp-12-133 mdadm]# mdadm -CR /dev/md0 -l1 -n2 /dev/sdb1
>>>>> /dev/sdd1
>>>>> --assume-clean
>>>>> mdadm: Note: this array has metadata at the start and
>>>>> may not be suitable as a boot device. If you plan to
>>>>> store '/boot' on this device please ensure that
>>>>> your boot-loader understands md/v1.x metadata, or use
>>>>> --metadata=0.90
>>>>> mdadm: Defaulting to version 1.2 metadata
>>>>> mdadm: array /dev/md0 started.
>>>>>
>>>>> Then I unplug the disk.
>>>>>
>>>>> [root@dhcp-12-133 mdadm]# lsblk
>>>>> sdc 8:32 0 186.3G 0 disk
>>>>> sdd 8:48 0 931.5G 0 disk
>>>>> └─sdd1 8:49 0 5G 0 part
>>>>> └─md0 9:0 0 5G 0 raid1
>>>>> [root@dhcp-12-133 mdadm]# echo faulty >
>>>>> /sys/block/md0/md/dev-sdb1/state
>>>>> [root@dhcp-12-133 mdadm]# echo remove >
>>>>> /sys/block/md0/md/dev-sdb1/state
>>>>> -bash: echo: write error: Device or resource busy
>>>>> [root@dhcp-12-133 mdadm]# echo idle > /sys/block/md0/md/sync_action
>>>>> [root@dhcp-12-133 mdadm]# echo remove >
>>>>> /sys/block/md0/md/dev-sdb1/state
>>>>>
>>>> I cannot reproduce this - using linux 3.18.2. I'd be surprised if mdadm
>>>> version affects things.
>>> Hi Neil
>>>
>>> I'm very curious, because it can reproduce in my machine 100%.
>>>
>>>> This error (Device or resoource busy) implies that rdev->raid_disk is >=
>>>> 0
>>>> (tested in state_store()).
>>>>
>>>> ->raid_disk is set to -1 by remove_and_add_spares() providing:
>>>> 1/ it isn't Blocked (which is very unlikely)
>>>> 2/ hot_remove_disk succeeds, which it will if nr_pending is zero, and
>>>> 3/ nr_pending is zero.
>>> I remember I have tired to check those reasons. But it's really is the
>>> reason 1
>>> which is very unlikely.
>>>
>>> I add some code in the function array_state_show
>>>
>>> array_state_show(struct mddev *mddev, char *page) {
>>> enum array_state st = inactive;
>>> struct md_rdev *rdev;
>>>
>>> rdev_for_each_rcu(rdev, mddev) {
>>> printk(KERN_ALERT "search for %s\n",
>>> rdev->bdev->bd_disk->disk_name);
>>> if (test_bit(Blocked, &rdev->flags))
>>> printk(KERN_ALERT "rdev is Blocked\n");
>>> else
>>> printk(KERN_ALERT "rdev is not Blocked\n");
>>> }
>>>
>>> When I echo 1 > /sys/block/sdc/device/delete, then I ran command:
>>>
>>> [root@dhcp-12-133 md]# cat /sys/block/md0/md/array_state
>>> read-auto
>> ^^^^^^^^^
>>
>> I think that is half the explanation.
>> You must have the md_mod.start_ro parameter set to '1'.
>>
>>
>>> [root@dhcp-12-133 md]# dmesg
>>> [ 2679.559185] search for sdc
>>> [ 2679.559189] rdev is Blocked
>>> [ 2679.559190] search for sdb
>>> [ 2679.559190] rdev is not Blocked
>>>
>>> So sdc is Blocked
>> and that is the other half - thanks.
>> (yes, I was wrong. Sometimes it is easier than being right, but still
>> yields results).
>>
>> When a device fails, it is Blocked until the metadata is updated to record
>> the failure. This ensures that no writes succeed without writing to that
>> device, until we a certain that no read will try reading from that device,
>> even after a crash/restart.
>>
>> Blocked is cleared after the metadata is written, but read-auto (and
>> read-only) devices never write out their metadata. So blocked doesn't get
>> cleared.
>>
>> When you "echo idle > .../sync_action" one of the side effects is to with
>> from 'read-auto' to fully active. This allows the metadata to be written,
>> Blocked to be cleared, and the device to be removed.
>>
>> If you
>> echo none > /sys/block/md0/md/dev-sdc/slot
>>
>> first, then the remove will work.
>>
>> We could possibly fix it with something like the following, but I'm not sure
>> I like it. There is no guarantee that I can see which would ensure the
>> superblock got updated before the first write if the array switch to
>> read/write.
>>
>> NeilBrown
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 9233c71138f1..b3d1e8e5e067 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -7528,7 +7528,7 @@ static int remove_and_add_spares(struct mddev *mddev,
>> rdev_for_each(rdev, mddev)
>> if ((this == NULL || rdev == this) &&
>> rdev->raid_disk >= 0 &&
>> - !test_bit(Blocked, &rdev->flags) &&
>> + (!test_bit(Blocked, &rdev->flags) || mddev->ro) &&
>> (test_bit(Faulty, &rdev->flags) ||
>> ! test_bit(In_sync, &rdev->flags)) &&
>> atomic_read(&rdev->nr_pending)==0) {
>>
>>
>>
> Hi Neil
>
> I have tried the patch and the problem can be fixed by it. But I'm sorry that I can't
> give more advices for better idea about this. I'm not familiar with the metadata part about
> the md. I'll try to get more time to read the code about md.
>
Hi Neil
I don't see the patch in linux-stable, do you miss this?
Best Regards
Xiao
--
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: [PATCH 00/11] mdadm tool: add the support for cluster-md
From: Guoqing Jiang @ 2015-06-10 5:53 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: neilb, linux-raid, rgoldwyn
In-Reply-To: <1433914934-21195-1-git-send-email-gqjiang@suse.com>
Hi Neil,
Sorry, I forgot to add V4 label in the subject.
Thanks,
Guoqing
Guoqing Jiang wrote:
> V4 changes:
> 1. update documation for syntax correct and add missed infos
> 2. code modification for remove duplicated code and coding style etc
> 3. make cluster name only appear in bitmap
> 4. add UUIDUpdate handling to write_bitmap0
>
> V3 changes:
> 1. re-orgnize some codes to ensure mdadm compiles after each patch is applied
> 2. change the code for super1.c for first patch since it has side effect for
> non-cluster condition
>
> V2 changes:
> 1. re-arrange the squence of patches
> 2. add some memembers into sb_le_to_cpu
> 3. handle some logic change and comments from Neil
>
> Basic background for Cluster MD: Cluster MD is a shared-device RAID for a
> cluster, currently, the implementation is limited to RAID1 but with further
> work (and some positive feedback), it could be extend to other RAID levels.
>
> The kernel part code of cluster-md has been sent to maillist several month
> ago by Goldywyn, and to make cluster-md works, the mdadm tools also need to
> do some changes accordingly.
>
> This patch set extends mdadm tool to aware cluster MD scenario, and handle
> related cluster-md scenario.
>
> 1. the first part (0001-0007) comes from Goldwyn, which add initial
> support for cluster-md, those changes included make mdadm awares nodes,
> home-cluster and n bitmaps for clustered mode, also let mdadm can
> confirm disk which is added by another node.
>
>
> 2. the second part is for support change cluster-name and node nums under
> assemble mode. Which extend write-bitmap to handle above cases, and also
> use the extended write_bitmap for update uuid. [PATCH V2 10/10] is just compiled
> test only.
>
> BTW: this series could be apply on top of commit "2a6493 Grow: fix a couple of typos".
>
> Some reltated links:
> [1] http://marc.info/?l=linux-raid&m=141891941330336&w=2
> [2] http://marc.info/?l=linux-raid&m=141935561418770&w=2
>
> Guoqing Jiang (11):
> Create n bitmaps for clustered mode
> Add nodes option while creating md
> home-cluster while creating an array
> Show all bitmaps while examining bitmap
> Add a new clustered disk
> Convert a bitmap=none device to clustered
> Skip clustered devices in incremental
> mdadm: add the ability to change cluster name
> mdadm: change the num of cluster node
> Reuse calc_bitmap_size to reduce code size
> Reuse the write_bitmap for update uuid
>
> Assemble.c | 12 ++++--
> Create.c | 5 ++-
> Grow.c | 12 ++++--
> Incremental.c | 5 +++
> Makefile | 1 +
> Manage.c | 39 ++++++++++++++++--
> ReadMe.c | 3 ++
> bitmap.c | 71 +++++++++++++++++++--------------
> bitmap.h | 7 +++-
> config.c | 27 ++++++++++++-
> md_p.h | 7 ++++
> md_u.h | 1 +
> mdadm.8.in | 42 +++++++++++++++++++-
> mdadm.c | 64 ++++++++++++++++++++++++++++--
> mdadm.h | 20 +++++++++-
> super0.c | 15 ++++++-
> super1.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++------------
> util.c | 60 ++++++++++++++++++++++++++++
> 18 files changed, 438 insertions(+), 78 deletions(-)
>
>
^ permalink raw reply
* [PATCH 11/11] Reuse the write_bitmap for update uuid
From: Guoqing Jiang @ 2015-06-10 5:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, rgoldwyn
In-Reply-To: <1433914934-21195-1-git-send-email-gqjiang@suse.com>
To handle different situations, it can support updating the uuid of
the bitmap. This patch also removes the redundant bitmap_update_uuid.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
Assemble.c | 5 ++---
bitmap.c | 20 --------------------
mdadm.h | 2 +-
super0.c | 11 +++++++++++
super1.c | 4 ++++
5 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/Assemble.c b/Assemble.c
index d163eaa..2662261 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -662,9 +662,8 @@ static int load_devices(struct devs *devices, char *devmap,
if (strcmp(c->update, "uuid")==0 &&
ident->bitmap_fd >= 0 && !bitmap_done) {
- if (bitmap_update_uuid(ident->bitmap_fd,
- content->uuid,
- tst->ss->swapuuid) != 0)
+ copy_uuid(tst->devs->uuid, content->uuid, tst->ss->swapuuid);
+ if (tst->ss->write_bitmap(tst, dfd, UUIDUpdate))
pr_err("Could not update uuid on external bitmap.\n");
else
bitmap_done = 1;
diff --git a/bitmap.c b/bitmap.c
index d21e5cc..575fcb9 100644
--- a/bitmap.c
+++ b/bitmap.c
@@ -457,23 +457,3 @@ out:
unlink(filename); /* possibly corrupted, better get rid of it */
return rv;
}
-
-int bitmap_update_uuid(int fd, int *uuid, int swap)
-{
- struct bitmap_super_s bm;
- if (lseek(fd, 0, 0) != 0)
- return 1;
- if (read(fd, &bm, sizeof(bm)) != sizeof(bm))
- return 1;
- if (bm.magic != __cpu_to_le32(BITMAP_MAGIC))
- return 1;
- copy_uuid(bm.uuid, uuid, swap);
- if (lseek(fd, 0, 0) != 0)
- return 2;
- if (write(fd, &bm, sizeof(bm)) != sizeof(bm)) {
- lseek(fd, 0, 0);
- return 2;
- }
- lseek(fd, 0, 0);
- return 0;
-}
diff --git a/mdadm.h b/mdadm.h
index 97892e6..7b9bb28 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -358,6 +358,7 @@ enum bitmap_update {
NoUpdate,
NameUpdate,
NodeNumUpdate,
+ UUIDUpdate,
};
/* structures read from config file */
@@ -1273,7 +1274,6 @@ extern int CreateBitmap(char *filename, int force, char uuid[16],
int major);
extern int ExamineBitmap(char *filename, int brief, struct supertype *st);
extern int Write_rules(char *rule_name);
-extern int bitmap_update_uuid(int fd, int *uuid, int swap);
extern unsigned long bitmap_sectors(struct bitmap_super_s *bsb);
extern int Dump_metadata(char *dev, char *dir, struct context *c,
struct supertype *st);
diff --git a/super0.c b/super0.c
index 6ad9d39..49267d1 100644
--- a/super0.c
+++ b/super0.c
@@ -1180,12 +1180,23 @@ static int write_bitmap0(struct supertype *st, int fd, enum bitmap_update update
unsigned long long dsize;
unsigned long long offset;
mdp_super_t *sb = st->sb;
+ bitmap_super_t *bms = (bitmap_super_t*)(((char*)sb) + MD_SB_BYTES);
int rv = 0;
int towrite, n;
void *buf;
+ switch (update) {
+ case UUIDUpdate:
+ memset((char *)bms->uuid, 0, sizeof(bms->uuid));
+ strncpy((char *)bms->uuid, (char *)st->devs->uuid, sizeof(bms->uuid));
+ break;
+ case NoUpdate:
+ default:
+ break;
+ }
+
if (!get_dev_size(fd, NULL, &dsize))
return 1;
diff --git a/super1.c b/super1.c
index 8128750..4ce7773 100644
--- a/super1.c
+++ b/super1.c
@@ -2232,6 +2232,10 @@ static int write_bitmap1(struct supertype *st, int fd, enum bitmap_update update
bms->nodes = __cpu_to_le32(st->nodes);
break;
+ case UUIDUpdate:
+ memset((char *)bms->uuid, 0, sizeof(bms->uuid));
+ strncpy((char *)bms->uuid, (char *)st->devs->uuid, sizeof(bms->uuid));
+ break;
case NoUpdate:
default:
break;
--
1.7.12.4
^ permalink raw reply related
* [PATCH 10/11] Reuse calc_bitmap_size to reduce code size
From: Guoqing Jiang @ 2015-06-10 5:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, rgoldwyn
In-Reply-To: <1433914934-21195-1-git-send-email-gqjiang@suse.com>
We can use the new added calc_bitmap_size func to remove some
redundant lines.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
super1.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/super1.c b/super1.c
index faccfcd..8128750 100644
--- a/super1.c
+++ b/super1.c
@@ -698,12 +698,8 @@ static int copy_metadata1(struct supertype *st, int from, int to)
/* have the header, can calculate
* correct bitmap bytes */
bitmap_super_t *bms;
- int bits;
bms = (void*)buf;
- bits = __le64_to_cpu(bms->sync_size) / (__le32_to_cpu(bms->chunksize)>>9);
- bytes = (bits+7) >> 3;
- bytes += sizeof(bitmap_super_t);
- bytes = ROUND_UP(bytes, 512);
+ bytes = calc_bitmap_size(bms, 512);
if (n > bytes)
n = bytes;
}
@@ -2258,11 +2254,7 @@ static int write_bitmap1(struct supertype *st, int fd, enum bitmap_update update
memset(buf, 0xff, 4096);
memcpy(buf, (char *)bms, sizeof(bitmap_super_t));
- towrite = __le64_to_cpu(bms->sync_size) / (__le32_to_cpu(bms->chunksize)>>9);
- towrite = (towrite+7) >> 3; /* bits to bytes */
- towrite += sizeof(bitmap_super_t);
- /* we need the bitmaps to be at 4k boundary */
- towrite = ROUND_UP(towrite, 4096);
+ towrite = calc_bitmap_size(bms, 4096);
while (towrite > 0) {
n = towrite;
if (n > 4096)
--
1.7.12.4
^ permalink raw reply related
* [PATCH 09/11] mdadm: change the num of cluster node
From: Guoqing Jiang @ 2015-06-10 5:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, rgoldwyn
In-Reply-To: <1433914934-21195-1-git-send-email-gqjiang@suse.com>
This extends nodes option for assemble mode, make the num of
cluster node could be change by user.
Before that, it is necessary to ensure there are enough space
for those nodes, calc_bitmap_size is introduced to calculate
the bitmap size of each node.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
Assemble.c | 6 +++++-
ReadMe.c | 2 +-
mdadm.8.in | 8 ++++++++
mdadm.c | 5 ++++-
mdadm.h | 1 +
super1.c | 37 +++++++++++++++++++++++++++++++++++++
6 files changed, 56 insertions(+), 3 deletions(-)
diff --git a/Assemble.c b/Assemble.c
index 12ac299..d163eaa 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -629,7 +629,11 @@ static int load_devices(struct devs *devices, char *devmap,
else if (strcmp(c->update, "home-cluster") == 0) {
tst->cluster_name = c->homecluster;
tst->ss->write_bitmap(tst, dfd, NameUpdate);
- } else
+ } else if (strcmp(c->update, "nodes") == 0) {
+ tst->nodes = c->nodes;
+ err = tst->ss->write_bitmap(tst, dfd, NodeNumUpdate);
+ }
+ else
err = tst->ss->update_super(tst, content, c->update,
devname, c->verbose,
ident->uuid_set,
diff --git a/ReadMe.c b/ReadMe.c
index c854cd5..d1830e1 100644
--- a/ReadMe.c
+++ b/ReadMe.c
@@ -140,7 +140,7 @@ struct option long_options[] = {
{"homehost", 1, 0, HomeHost},
{"symlinks", 1, 0, Symlinks},
{"data-offset",1, 0, DataOffset},
- {"nodes",1, 0, Nodes},
+ {"nodes",1, 0, Nodes}, /* also for --assemble */
{"home-cluster",1, 0, ClusterName},
/* For assemble */
diff --git a/mdadm.8.in b/mdadm.8.in
index 99b02a3..8b7768d 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -1097,6 +1097,7 @@ argument given to this flag can be one of
.BR summaries ,
.BR uuid ,
.BR name ,
+.BR nodes ,
.BR homehost ,
.BR home-cluster ,
.BR resync ,
@@ -1149,6 +1150,13 @@ The
.B name
option will change the
.I name
+of the array as stored in the superblock and bitmap. This option only
+works for clustered environment.
+
+The
+.B nodes
+option will change the
+.I nodes
of the array as stored in the superblock. This is only supported for
version-1 superblocks.
diff --git a/mdadm.c b/mdadm.c
index 5e9b302..7b85570 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -589,6 +589,7 @@ int main(int argc, char *argv[])
}
ident.raid_disks = s.raiddisks;
continue;
+ case O(ASSEMBLE, Nodes):
case O(CREATE, Nodes):
c.nodes = parse_num(optarg);
if (c.nodes <= 0) {
@@ -744,6 +745,8 @@ int main(int argc, char *argv[])
continue;
if (strcmp(c.update, "home-cluster")==0)
continue;
+ if (strcmp(c.update, "nodes")==0)
+ continue;
if (strcmp(c.update, "devicesize")==0)
continue;
if (strcmp(c.update, "no-bitmap")==0)
@@ -782,7 +785,7 @@ int main(int argc, char *argv[])
Name, c.update);
}
fprintf(outf, "Valid --update options are:\n"
- " 'sparc2.2', 'super-minor', 'uuid', 'name', 'resync',\n"
+ " 'sparc2.2', 'super-minor', 'uuid', 'name', 'nodes', 'resync',\n"
" 'summaries', 'homehost', 'home-cluster', 'byteorder', 'devicesize',\n"
" 'no-bitmap', 'metadata', 'revert-reshape'\n"
" 'bbl', 'no-bbl'\n"
diff --git a/mdadm.h b/mdadm.h
index d8b0749..97892e6 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -357,6 +357,7 @@ enum prefix_standard {
enum bitmap_update {
NoUpdate,
NameUpdate,
+ NodeNumUpdate,
};
/* structures read from config file */
diff --git a/super1.c b/super1.c
index 699f1ec..faccfcd 100644
--- a/super1.c
+++ b/super1.c
@@ -134,6 +134,20 @@ struct misc_dev_info {
|MD_FEATURE_NEW_OFFSET \
)
+/* return how many bytes are needed for bitmap, for cluster-md each node
+ * should have it's own bitmap */
+static unsigned int calc_bitmap_size(bitmap_super_t *bms, unsigned int boundary)
+{
+ unsigned long long bits, bytes;
+
+ bits = __le64_to_cpu(bms->sync_size) / (__le32_to_cpu(bms->chunksize)>>9);
+ bytes = (bits+7) >> 3;
+ bytes += sizeof(bitmap_super_t);
+ bytes = ROUND_UP(bytes, boundary);
+
+ return bytes;
+}
+
static unsigned int calc_sb_1_csum(struct mdp_superblock_1 * sb)
{
unsigned int disk_csum, csum;
@@ -2190,6 +2204,7 @@ static int write_bitmap1(struct supertype *st, int fd, enum bitmap_update update
int towrite, n;
struct align_fd afd;
unsigned int i = 0;
+ unsigned long long total_bm_space, bm_space_per_node;
switch (update) {
case NameUpdate:
@@ -2199,6 +2214,28 @@ static int write_bitmap1(struct supertype *st, int fd, enum bitmap_update update
strncpy((char *)bms->cluster_name, st->cluster_name, 64);
}
break;
+ case NodeNumUpdate:
+ /* cluster md only supports superblock 1.2 now */
+ if (st->minor_version != 2) {
+ pr_err("Warning: cluster md only works with superblock 1.2\n");
+ return -EINVAL;
+ }
+
+ /* Each node has an independent bitmap, it is necessary to calculate the
+ * space is enough or not, first get how many bytes for the total bitmap */
+ bm_space_per_node = calc_bitmap_size(bms, 4096);
+
+ total_bm_space = 512 * (__le64_to_cpu(sb->data_offset) - __le64_to_cpu(sb->super_offset));
+ total_bm_space = total_bm_space - 4096; /* leave another 4k for superblock */
+
+ if (bm_space_per_node * st->nodes > total_bm_space) {
+ pr_err("Warning: The max num of nodes can't exceed %llu\n",
+ total_bm_space / bm_space_per_node);
+ return -ENOMEM;
+ }
+
+ bms->nodes = __cpu_to_le32(st->nodes);
+ break;
case NoUpdate:
default:
break;
--
1.7.12.4
^ permalink raw reply related
* [PATCH 08/11] mdadm: add the ability to change cluster name
From: Guoqing Jiang @ 2015-06-10 5:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, rgoldwyn
In-Reply-To: <1433914934-21195-1-git-send-email-gqjiang@suse.com>
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. let the cluster name also show in examine_super1 and detail_super1
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
Assemble.c | 5 ++++-
Grow.c | 2 +-
mdadm.8.in | 6 ++++++
mdadm.c | 5 ++++-
mdadm.h | 7 ++++++-
super0.c | 4 ++--
super1.c | 23 +++++++++++++++++++++--
7 files changed, 44 insertions(+), 8 deletions(-)
diff --git a/Assemble.c b/Assemble.c
index 42710a8..12ac299 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -626,7 +626,10 @@ static int load_devices(struct devs *devices, char *devmap,
if (strcmp(c->update, "byteorder") == 0)
err = 0;
- else
+ else if (strcmp(c->update, "home-cluster") == 0) {
+ tst->cluster_name = c->homecluster;
+ tst->ss->write_bitmap(tst, dfd, NameUpdate);
+ } else
err = tst->ss->update_super(tst, content, c->update,
devname, c->verbose,
ident->uuid_set,
diff --git a/Grow.c b/Grow.c
index 90a7fe9..857c7e1 100644
--- a/Grow.c
+++ b/Grow.c
@@ -412,7 +412,7 @@ int Grow_addbitmap(char *devname, int fd, struct context *c, struct shape *s)
bitmapsize, offset_setable,
major)
)
- st->ss->write_bitmap(st, fd2);
+ st->ss->write_bitmap(st, fd2, NoUpdate);
else {
pr_err("failed to create internal bitmap - chunksize problem.\n");
close(fd2);
diff --git a/mdadm.8.in b/mdadm.8.in
index 3dd000c..99b02a3 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -1098,6 +1098,7 @@ argument given to this flag can be one of
.BR uuid ,
.BR name ,
.BR homehost ,
+.BR home-cluster ,
.BR resync ,
.BR byteorder ,
.BR devicesize ,
@@ -1160,6 +1161,11 @@ same as updating the UUID.
For version-1 superblocks, this involves updating the name.
The
+.B home\-cluster
+option will change the cluster name as recorded in the superblock and
+bitmap. This option only works for clustered environment.
+
+The
.B resync
option will cause the array to be marked
.I dirty
diff --git a/mdadm.c b/mdadm.c
index f08d8c9..5e9b302 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -598,6 +598,7 @@ int main(int argc, char *argv[])
}
continue;
case O(CREATE, ClusterName):
+ case O(ASSEMBLE, ClusterName):
c.homecluster = optarg;
if (strlen(c.homecluster) > 64) {
pr_err("Cluster name too big.\n");
@@ -741,6 +742,8 @@ int main(int argc, char *argv[])
continue;
if (strcmp(c.update, "homehost")==0)
continue;
+ if (strcmp(c.update, "home-cluster")==0)
+ continue;
if (strcmp(c.update, "devicesize")==0)
continue;
if (strcmp(c.update, "no-bitmap")==0)
@@ -780,7 +783,7 @@ int main(int argc, char *argv[])
}
fprintf(outf, "Valid --update options are:\n"
" 'sparc2.2', 'super-minor', 'uuid', 'name', 'resync',\n"
- " 'summaries', 'homehost', 'byteorder', 'devicesize',\n"
+ " 'summaries', 'homehost', 'home-cluster', 'byteorder', 'devicesize',\n"
" 'no-bitmap', 'metadata', 'revert-reshape'\n"
" 'bbl', 'no-bbl'\n"
);
diff --git a/mdadm.h b/mdadm.h
index 00c726e..d8b0749 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -354,6 +354,11 @@ enum prefix_standard {
IEC
};
+enum bitmap_update {
+ NoUpdate,
+ NameUpdate,
+};
+
/* structures read from config file */
/* List of mddevice names and identifiers
* Identifiers can be:
@@ -850,7 +855,7 @@ extern struct superswitch {
/* if add_internal_bitmap succeeded for existing array, this
* writes it out.
*/
- int (*write_bitmap)(struct supertype *st, int fd);
+ int (*write_bitmap)(struct supertype *st, int fd, enum bitmap_update update);
/* Free the superblock and any other allocated data */
void (*free_super)(struct supertype *st);
diff --git a/super0.c b/super0.c
index deb5999..6ad9d39 100644
--- a/super0.c
+++ b/super0.c
@@ -900,7 +900,7 @@ static int write_init_super0(struct supertype *st)
rv = store_super0(st, di->fd);
if (rv == 0 && (sb->state & (1<<MD_SB_BITMAP_PRESENT)))
- rv = st->ss->write_bitmap(st, di->fd);
+ rv = st->ss->write_bitmap(st, di->fd, NoUpdate);
if (rv)
pr_err("failed to write superblock to %s\n",
@@ -1175,7 +1175,7 @@ static void locate_bitmap0(struct supertype *st, int fd)
lseek64(fd, offset, 0);
}
-static int write_bitmap0(struct supertype *st, int fd)
+static int write_bitmap0(struct supertype *st, int fd, enum bitmap_update update)
{
unsigned long long dsize;
unsigned long long offset;
diff --git a/super1.c b/super1.c
index fd728d2..699f1ec 100644
--- a/super1.c
+++ b/super1.c
@@ -256,6 +256,7 @@ static int awrite(struct align_fd *afd, void *buf, int len)
static void examine_super1(struct supertype *st, char *homehost)
{
struct mdp_superblock_1 *sb = st->sb;
+ bitmap_super_t *bms = (bitmap_super_t*)(((char*)sb)+MAX_SB_SIZE);
time_t atime;
unsigned int d;
int role;
@@ -289,6 +290,8 @@ static void examine_super1(struct supertype *st, char *homehost)
strncmp(sb->set_name, homehost, l) == 0)
printf(" (local to host %s)", homehost);
printf("\n");
+ if (bms->nodes > 0)
+ printf("Cluster Name : %s", bms->cluster_name);
atime = __le64_to_cpu(sb->ctime) & 0xFFFFFFFFFFULL;
printf(" Creation Time : %.24s\n", ctime(&atime));
c=map_num(pers, __le32_to_cpu(sb->level));
@@ -740,6 +743,7 @@ err:
static void detail_super1(struct supertype *st, char *homehost)
{
struct mdp_superblock_1 *sb = st->sb;
+ bitmap_super_t *bms = (bitmap_super_t*)(((char*)sb) + MAX_SB_SIZE);
int i;
int l = homehost ? strlen(homehost) : 0;
@@ -748,6 +752,8 @@ static void detail_super1(struct supertype *st, char *homehost)
sb->set_name[l] == ':' &&
strncmp(sb->set_name, homehost, l) == 0)
printf(" (local to host %s)", homehost);
+ if (bms->nodes > 0)
+ printf("Cluster Name : %64s", bms->cluster_name);
printf("\n UUID : ");
for (i=0; i<16; i++) {
if ((i&3)==0 && i != 0) printf(":");
@@ -1691,7 +1697,7 @@ static int write_init_super1(struct supertype *st)
sb->sb_csum = calc_sb_1_csum(sb);
rv = store_super1(st, di->fd);
if (rv == 0 && (__le32_to_cpu(sb->feature_map) & 1))
- rv = st->ss->write_bitmap(st, di->fd);
+ rv = st->ss->write_bitmap(st, di->fd, NoUpdate);
close(di->fd);
di->fd = -1;
if (rv)
@@ -2175,7 +2181,7 @@ static void locate_bitmap1(struct supertype *st, int fd)
lseek64(fd, offset<<9, 0);
}
-static int write_bitmap1(struct supertype *st, int fd)
+static int write_bitmap1(struct supertype *st, int fd, enum bitmap_update update)
{
struct mdp_superblock_1 *sb = st->sb;
bitmap_super_t *bms = (bitmap_super_t*)(((char*)sb)+MAX_SB_SIZE);
@@ -2185,6 +2191,19 @@ static int write_bitmap1(struct supertype *st, int fd)
struct align_fd afd;
unsigned int i = 0;
+ switch (update) {
+ case NameUpdate:
+ /* update cluster name */
+ if (st->cluster_name) {
+ memset((char *)bms->cluster_name, 0, sizeof(bms->cluster_name));
+ strncpy((char *)bms->cluster_name, st->cluster_name, 64);
+ }
+ break;
+ case NoUpdate:
+ default:
+ break;
+ }
+
init_afd(&afd, fd);
locate_bitmap1(st, fd);
--
1.7.12.4
^ permalink raw reply related
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