* [PATCH 0/7] Fix fallout from changes to FUA and PREFLUSH definitions
From: Jan Kara @ 2017-05-02 15:03 UTC (permalink / raw)
To: linux-fsdevel
Cc: cluster-devel, Theodore Ts'o, Mike Snitzer, linux-raid,
reiserfs-devel, linux-f2fs-devel, Christoph Hellwig, dm-devel,
Jaegeuk Kim, David Sterba, Jan Kara, linux-ext4, Shaohua Li,
Steven Whitehouse, linux-btrfs
Hello,
this series addresses a performance issue caused by commit b685d3d65ac7 "block:
treat REQ_FUA and REQ_PREFLUSH as synchronous". We know for certain this
problem significanly regresses (over 10%, in some cases up to 100%) ext4 and
btrfs for dbench4 and reaim benchmarks. Based on this I have fixed up also
other places which suffer from the same problem however those changes are
untested so maintainers please have a look whether the change makes sense to
you and also whether I possibly didn't miss some cases where REQ_SYNC should be
also added. Patches in this series are completely independent so if maintainers
agree with the change, feel free to take it through your tree.
The core of the problem is that above mentioned commit removed REQ_SYNC flag
from WRITE_{FUA|PREFLUSH|...} definitions. generic_make_request_checks()
however strips REQ_FUA and REQ_PREFLUSH flags from a bio when the storage
doesn't report volatile write cache and thus write effectively becomes
asynchronous which can lead to performance regressions.
A side note for ext4: The two patches for ext4 & jbd2 are on top of the change
that got merged in the ext4 tree already.
Honza
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply
* [PATCH 7/7] md: Make flush bios explicitely sync
From: Jan Kara @ 2017-05-02 15:03 UTC (permalink / raw)
To: linux-fsdevel
Cc: Christoph Hellwig, Jan Kara, linux-raid, Shaohua Li, Mike Snitzer,
dm-devel
In-Reply-To: <20170502150351.29452-1-jack@suse.cz>
Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
definitions. generic_make_request_checks() however strips REQ_FUA and
REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
write cache and thus write effectively becomes asynchronous which can
lead to performance regressions
Fix the problem by making sure all bios which are synchronous are
properly marked with REQ_SYNC.
CC: linux-raid@vger.kernel.org
CC: Shaohua Li <shli@kernel.org>
CC: Mike Snitzer <snitzer@redhat.com>
CC: dm-devel@redhat.com
Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
Signed-off-by: Jan Kara <jack@suse.cz>
---
drivers/md/dm-snap-persistent.c | 3 ++-
drivers/md/md.c | 2 +-
drivers/md/raid5-cache.c | 4 ++--
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/md/dm-snap-persistent.c b/drivers/md/dm-snap-persistent.c
index b93476c3ba3f..b92ab4cb0710 100644
--- a/drivers/md/dm-snap-persistent.c
+++ b/drivers/md/dm-snap-persistent.c
@@ -741,7 +741,8 @@ static void persistent_commit_exception(struct dm_exception_store *store,
/*
* Commit exceptions to disk.
*/
- if (ps->valid && area_io(ps, REQ_OP_WRITE, REQ_PREFLUSH | REQ_FUA))
+ if (ps->valid && area_io(ps, REQ_OP_WRITE,
+ REQ_SYNC | REQ_PREFLUSH | REQ_FUA))
ps->valid = 0;
/*
diff --git a/drivers/md/md.c b/drivers/md/md.c
index f6ae1d67bcd0..9c40ce0642c2 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -753,7 +753,7 @@ void md_super_write(struct mddev *mddev, struct md_rdev *rdev,
test_bit(FailFast, &rdev->flags) &&
!test_bit(LastDev, &rdev->flags))
ff = MD_FAILFAST;
- bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH | REQ_FUA | ff;
+ bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH | REQ_FUA | ff;
atomic_inc(&mddev->pending_writes);
submit_bio(bio);
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 3f307be01b10..0bd5d0c88cee 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -1625,7 +1625,7 @@ static int r5l_log_write_empty_meta_block(struct r5l_log *log, sector_t pos,
mb->checksum = cpu_to_le32(crc32c_le(log->uuid_checksum,
mb, PAGE_SIZE));
if (!sync_page_io(log->rdev, pos, PAGE_SIZE, page, REQ_OP_WRITE,
- REQ_FUA, false)) {
+ REQ_SYNC | REQ_FUA, false)) {
__free_page(page);
return -EIO;
}
@@ -2198,7 +2198,7 @@ r5c_recovery_rewrite_data_only_stripes(struct r5l_log *log,
mb->checksum = cpu_to_le32(crc32c_le(log->uuid_checksum,
mb, PAGE_SIZE));
sync_page_io(log->rdev, ctx->pos, PAGE_SIZE, page,
- REQ_OP_WRITE, REQ_FUA, false);
+ REQ_OP_WRITE, REQ_SYNC | REQ_FUA, false);
sh->log_start = ctx->pos;
list_add_tail(&sh->r5c, &log->stripe_in_journal_list);
atomic_inc(&log->stripe_in_journal_count);
--
2.12.0
^ permalink raw reply related
* Re: Recover array after I panicked
From: Anthony Youngman @ 2017-05-02 15:49 UTC (permalink / raw)
To: Patrik Dahlström, Andreas Klauer; +Cc: Brad Campbell, linux-raid
In-Reply-To: <37245817-43d3-7c93-98fe-8b8ce54396ad@powerlamerz.org>
On 02/05/17 14:08, Patrik Dahlström wrote:
>
>
> On 04/29/2017 11:56 AM, Andreas Klauer wrote:
>> On Sat, Apr 29, 2017 at 12:46:15AM +0200, Patrik Dahlström wrote:
>> You could create the RAID first and the overlay afterwards.
>> Or you can create loop devices on the busy devices and use those.
> I solved it by creating loop devices and use dd. I let it finish, then
> ran fsck and resize2fs on the partition and now my data is back!
>
> Thank you all for your invaluable help!
>
> Would it be worth it for me to write up an article on the wiki on how to
> re-create messed up metadata?
>
Hey - I was planning to do that! :-)
Seriously, yes of course, please do. That would be wonderful. It belongs
best after "Recovering a damaged RAID" in the "When things go Wrogn"
section, i think.
Just skim the contribution guidelines before you start, they're nothing
onerous, just use good English and use "I" a lot, take responsibility
for what you write. It's meant to be easy reading but informative. A
case study of what you did would be marvelous.
I need to edit the page about that script you ran, and put a big big
warning in front of it not to run it recklessly :-)
(Contact me off-line if you haven't got an account and have difficulty
creating one.)
Cheers,
Wol
^ permalink raw reply
* Re: md-cluster Oops 4.9.13
From: Marc Smith @ 2017-05-02 18:30 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: linux-raid
In-Reply-To: <58ED83B0.6080209@suse.com>
Hi,
I was finally able to test this, and I modified the original patch to
apply cleanly against 4.9.13. I tried several times to reproduce this
oops, and wasn't successful, so it looks good. Here is the modified
patch:
diff -Naur a/drivers/md/bitmap.c b/drivers/md/bitmap.c
--- a/drivers/md/bitmap.c 2017-02-26 05:11:18.000000000 -0500
+++ b/drivers/md/bitmap.c 2017-04-14 11:22:18.325093619 -0400
@@ -1734,6 +1734,20 @@
kfree(bitmap);
}
+void bitmap_wait_behind_writes(struct mddev *mddev)
+{
+ struct bitmap *bitmap = mddev->bitmap;
+
+ /* wait for behind writes to complete */
+ if (bitmap && atomic_read(&bitmap->behind_writes) > 0) {
+ pr_debug("md:%s: behind writes in progress - waiting
to stop.\n",
+ mdname(mddev));
+ /* need to kick something here to make sure I/O goes? */
+ wait_event(bitmap->behind_wait,
+ atomic_read(&bitmap->behind_writes) == 0);
+ }
+}
+
void bitmap_destroy(struct mddev *mddev)
{
struct bitmap *bitmap = mddev->bitmap;
@@ -1741,6 +1755,8 @@
if (!bitmap) /* there was no bitmap */
return;
+ bitmap_wait_behind_writes(mddev);
+
mutex_lock(&mddev->bitmap_info.mutex);
spin_lock(&mddev->lock);
mddev->bitmap = NULL; /* disconnect from the md device */
diff -Naur a/drivers/md/bitmap.h b/drivers/md/bitmap.h
--- a/drivers/md/bitmap.h 2017-02-26 05:11:18.000000000 -0500
+++ b/drivers/md/bitmap.h 2017-04-14 10:49:03.999868295 -0400
@@ -269,6 +269,7 @@
int chunksize, int init);
int bitmap_copy_from_slot(struct mddev *mddev, int slot,
sector_t *lo, sector_t *hi, bool clear_bits);
+void bitmap_wait_behind_writes(struct mddev *mddev);
#endif
#endif
diff -Naur a/drivers/md/md.c b/drivers/md/md.c
--- a/drivers/md/md.c 2017-02-26 05:11:18.000000000 -0500
+++ b/drivers/md/md.c 2017-04-14 10:57:52.344539569 -0400
@@ -5513,15 +5513,7 @@
static void mddev_detach(struct mddev *mddev)
{
- struct bitmap *bitmap = mddev->bitmap;
- /* wait for behind writes to complete */
- if (bitmap && atomic_read(&bitmap->behind_writes) > 0) {
- printk(KERN_INFO "md:%s: behind writes in progress -
waiting to stop.\n",
- mdname(mddev));
- /* need to kick something here to make sure I/O goes? */
- wait_event(bitmap->behind_wait,
- atomic_read(&bitmap->behind_writes) == 0);
- }
+ bitmap_wait_behind_writes(mddev);
if (mddev->pers && mddev->pers->quiesce) {
mddev->pers->quiesce(mddev, 1);
mddev->pers->quiesce(mddev, 0);
@@ -5534,6 +5526,7 @@
static void __md_stop(struct mddev *mddev)
{
struct md_personality *pers = mddev->pers;
+ bitmap_destroy(mddev);
mddev_detach(mddev);
/* Ensure ->event_work is done */
flush_workqueue(md_misc_wq);
@@ -5554,7 +5547,6 @@
* This is called from dm-raid
*/
__md_stop(mddev);
- bitmap_destroy(mddev);
if (mddev->bio_set)
bioset_free(mddev->bio_set);
}
@@ -5692,7 +5684,6 @@
if (mode == 0) {
printk(KERN_INFO "md: %s stopped.\n", mdname(mddev));
- bitmap_destroy(mddev);
if (mddev->bitmap_info.file) {
struct file *f = mddev->bitmap_info.file;
spin_lock(&mddev->lock);
--Marc
On Tue, Apr 11, 2017 at 9:32 PM, Guoqing Jiang <gqjiang@suse.com> wrote:
>
>
> On 04/10/2017 09:25 PM, Marc Smith wrote:
>>
>> Hi,
>>
>> Sorry for the delay... I was hoping to cherry-pick this and test
>> against 4.9.x, but it didn't apply cleanly, although it looks trivial
>> to do it by hand. Is it recommended/okay to test this patch against
>> 4.9.x? Will the fix eventually be merged into 4.9.x?
>
>
> I think you can have a try with the patch then see what will happen, the
> better
> way is try with the latest code though people don't like always update
> kernel,
> but it is not a material for stable 4.9.x from my understanding.
>
> Thanks,
> Guoqing
>
>
>>
>>
>> --Marc
>>
>> On Tue, Apr 4, 2017 at 11:01 PM, Guoqing Jiang <jgq516@gmail.com> wrote:
>>>
>>>
>>> On 04/04/2017 10:06 PM, Marc Smith wrote:
>>>>
>>>> Hi,
>>>>
>>>> I encountered an oops this morning when stopping a MD array
>>>> (md-cluster)... there were 4 md-cluster array started, and they were
>>>> in the middle of a rebuild. I stopped the first one and then stopped
>>>> the second one immediately after and got the oops, here is a
>>>> transcript of what was on my terminal session:
>>>>
>>>> [root@brimstone-1b ~]# mdadm --stop /dev/md/array1
>>>> mdadm: stopped /dev/md/array1
>>>> [root@brimstone-1b ~]# mdadm --stop /dev/md/array2
>>>>
>>>> Message from syslogd@brimstone-1b at Tue Apr 4 09:54:40 2017 ...
>>>> brimstone-1b kernel: [649162.174685] BUG: unable to handle kernel NULL
>>>> pointer dereference at 0000000000000098
>>>>
>>>> Using Linux 4.9.13 and here is the output from the kernel messages:
>>>>
>>>> --snip--
>>>> [649158.014731] dlm: 5b3b8f94-7875-b323-5bb8-29fa6866f4a8: leaving the
>>>> lockspace group...
>>>> [649158.015233] dlm: 5b3b8f94-7875-b323-5bb8-29fa6866f4a8: group event
>>>> done 0 0
>>>> [649158.015303] dlm: 5b3b8f94-7875-b323-5bb8-29fa6866f4a8:
>>>> release_lockspace final free
>>>> [649158.015331] md: unbind<nvme0n1p1>
>>>> [649158.042540] md: export_rdev(nvme0n1p1)
>>>> [649158.042546] md: unbind<nvme1n1p1>
>>>> [649158.048501] md: export_rdev(nvme1n1p1)
>>>> [649161.759022] md127: detected capacity change from 1000068874240 to 0
>>>> [649161.759025] md: md127 stopped.
>>>> [649162.174685] BUG: unable to handle kernel NULL pointer dereference
>>>> at 0000000000000098
>>>> [649162.174727] IP: [<ffffffff81868b40>] recv_daemon+0x1e9/0x373
>>>
>>>
>>> Looks like the recv_daemon is still running after stop array, commit
>>> 48df498 "md: move bitmap_destroy to the beginning of __md_stop"
>>> ensure it won't happen.
>>>
>>>
>>> [snip]
>>>
>>>> Perhaps this is already fixed in later versions? Let me know if you
>>>> need any additional information.
>>>
>>>
>>> Could you pls try with the latest version? Please let me know if you
>>> still see it, thanks.
>>>
>>> Regards,
>>> Guoqing
>>>
>
^ permalink raw reply
* Fwd: Please Help! Degraded Raid6
From: Bernd @ 2017-05-02 21:11 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <C89BFE2F-0C7A-47CA-A970-EA90C0FF053A@gmx.de>
-------- Ursprüngliche Nachricht --------
Von: Bernd <berndman80@gmx.de>
Gesendet: 2. Mai 2017 22:52:38 MESZ
An: linux-raid@vger.kernel.org
CC: neilb@suse.de
Betreff: Please Help! Degraded Raid6
Hello,
I have a problem with my degraded raid6 and hope you can help me. Please excuse if my english is not so well...
A few days ago i wanted to boot my server and while booting the electricity fuse dropped out. And my raid 6 was in the state active degraded not started. It was degraded with 7 of 8 devices before the crash. I was able to boot with the 7 devices and had all my data. Now when booting all devices are spares and when ich do mdadm --run --force /dev/md0 the state is active degraded not started. Im not able to run the raid and can't touch my data. Add or readd the removed device is denied. System removed it a time ago without my help...think i have to recreate with one missing device. And think a have lost my superblocks...Please help. Dont know what to do. Don't want loose all my data! ...thank you for your help Bernd
This is what i tried:
berndman@berndman:~$ sudo mdadm --detail /dev/md0
/dev/md0:
Version : 1.2
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Used Dev Size : 1953509376 (1863.01 GiB 2000.39 GB)
Raid Devices : 8
Total Devices : 7
Persistence : Superblock is persistent
Update Time : Fri Mar 31 14:56:50 2017
State : active, degraded, Not Started
Active Devices : 7
Working Devices : 7
Failed Devices : 0
Spare Devices : 0
Layout : left-symmetric
Chunk Size : 4096K
Name : berndman:0 (local to host berndman)
UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Events : 1686
Number Major Minor RaidDevice State
0 8 81 0 active sync /dev/sdf1
1 8 49 1 active sync /dev/sdd1
8 8 1 2 active sync /
dev/sda1
3 8 65 3 active sync /dev/sde1
4 8 129 4 active sync /dev/sdi1
5 8 33 5 active sync /dev/sdc1
6 8 17 6 active sync /dev/sdb1
7 0 0 7 removed
berndman@berndman:~$ sudo cat /proc/mdstat
Personalities : [linear] [multipath] [raid0] [raid1] [raid6] [raid5] [raid4] [raid10]
md0 : inactive sdi1[4] sda1[8] sdd1[1] sde1[3] sdf1[0] sdc1[5] sdb1[6]
13674587752 blocks super 1.2
unused devices: <none>
berndman@berndman:~$ sudo mdadm --run /dev/md0
mdadm: failed to run array /dev/md0: Input/output error
berndman@berndman:~$ sudo mdadm --examine /dev/sd[a-z]
/dev/sda:
MBR Magic : aa55
Partition[0] : 3907027120 sectors at 2048 (type fd)
/dev/sdb:
MBR Magic : aa55
Partition[0] : 3907027120 sectors at 2048 (type fd)
/dev/sdc:
MBR Magic : aa55
Partition[0] : 3907027120 sectors at 2048 (type fd)
/dev/sdd:
MBR Magic : aa55
Partition[0] : 3907027120 sectors at 2048 (type fd)
/dev/sde:
MBR Magic : aa55
Partition[0] : 3907027120 sectors at 2048 (type fd)
/dev/sdf:
MBR Magic : aa55
Partition[0] : 3907027120 sectors at 2048 (type fd)
/dev/sdg:
MBR Magic : aa55
Partition[0] : 73969664 sectors at 2048 (type 83)
Partition[1] : 4190210 sectors at 73973758 (type 05)
/dev/sdh:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x2
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907027120 (1863.02 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
Recovery Offset : 0 sectors
State : clean
Device UUID : 0be2a583:659bbe6e:7d8c93dc:6592505b
Update Time : Fri Jul 10 22:32:34 2015
Checksum : b60eeb71 - correct
Events : 607
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 7
Array State : AAAAAAAA ('A' == active, '.' == missing)
/dev/sdi:
MBR Magic : aa55
Partition[0] : 3907027120 sectors at 2048 (type fd)
berndman@berndman:~$ sudo mdadm --assemble --run --force /dev/md0 /dev/sd[abcdefi]1
mdadm: cannot open device /dev/sda1: Device or resource busy
mdadm: /dev/sda1 has no superblock - assembly aborted
berndman@berndman:~$
^ permalink raw reply
* Re: [PATCH 0/6] md: Fine-tuning for some function implementations
From: NeilBrown @ 2017-05-02 21:36 UTC (permalink / raw)
To: SF Markus Elfring, linux-raid, Shaohua Li; +Cc: LKML, kernel-janitors
In-Reply-To: <f05bfc4e-7935-7e87-cc85-f0d9c5e3846a@users.sourceforge.net>
[-- Attachment #1: Type: text/plain, Size: 1274 bytes --]
On Tue, May 02 2017, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 2 May 2017 16:12:34 +0200
>
> Some update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (6):
> Replace seven seq_printf() calls by seq_putc()
> Replace 17 seq_printf() calls by seq_puts()
Why does anyone care whether printf or putc/puts is used? Really it
doesn't matter *at* *all*.
I don't object to the patch but if it would up to me I probably wouldn't
bother applying it it either.
Sometimes I just want to "print" something and I don't want to care
whether it is a constant string or a single-byte constant string, or
something more general.
I see these changes as worse than white-space fixes.
NeilBrown
> Adjust four function calls together with a variable assignment
> Use seq_puts() in faulty_status()
> Adjust six function calls together with a variable assignment in faulty_status()
> Add some spaces for better code readability
>
> drivers/md/faulty.c | 48 ++++++++++++++++++++++----------------
> drivers/md/md.c | 66 +++++++++++++++++++++++++++++------------------------
> 2 files changed, 64 insertions(+), 50 deletions(-)
>
> --
> 2.12.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [dm-devel] [mdadm PATCH 4/4] Create: tell udev device is not ready when first created.
From: NeilBrown @ 2017-05-02 21:42 UTC (permalink / raw)
To: Peter Rajnoha, Jes Sorensen; +Cc: linux-raid, dm-devel
In-Reply-To: <65882238-57f0-dcef-645b-0ab1ab9cdc91@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 13902 bytes --]
I'm sorry, but I didn't read all your words.
You seemed to be telling me about extra complexity in udev, and extra
complexity that you think belongs in mdadm, which together might achieve
your vision for how things should work.
But to me, complexity is the enemy. Give me "simple" any day.
NeilBrown
On Tue, May 02 2017, Peter Rajnoha wrote:
> On 05/01/2017 06:35 AM, NeilBrown wrote:
>> On Fri, Apr 28 2017, Peter Rajnoha wrote:
>>
>>> On 04/28/2017 05:55 AM, NeilBrown wrote:
>>>> On Wed, Apr 26 2017, Peter Rajnoha wrote:
>>>>
>>>>> On 04/20/2017 11:35 PM, NeilBrown wrote:
>>>>>> If we wanted an more permanent udev rule, we would need to record the
>>>>>> devices that should be ignored in the filesystem somewhere else.
>>>>>> Maybe in /run/mdadm.
>>>>>> e.g.
>>>>>>
>>>>>> KERNEL=="md*", TEST="/run/mdadm/creating-$kernel", ENV{SYSTEMD_READY}="0"
>>>>>>
>>>>>> Then we could have something like the following (untested) in mdadm.
>>>>>> Does that seem more suitable?
>>>>>>
>>>>>
>>>>> Yes, please, if possible, go for a permanent udev rule surely - this
>>>>> will make it much easier for other foreign tools to hook in properly if
>>>>> needed and it would also be much easier to debug.
>>>>
>>>> I'm leaning towards the files-in-/run/mdadm approach too. I'll make a
>>>> proper patch.
>>>>
>>>>>
>>>>> But, wouldn't it be better if we could just pass this information ("not
>>>>> initialized yet") as RUN_ARRAY md ioctl parameter? In that case the md
>>>>> driver in kernel could add the variable to the uevent it generates which
>>>>> userspace udev rules could check for easily. This way, we don't need to
>>>>> hassle with creating files in filesystem and the information would be
>>>>> directly a part of the uevent the md kernel driver generates (so
>>>>> directly accessible in udev rules too). Also, possibly adding more
>>>>> variables for other future scenarios if needed.
>>>>
>>>> When would we clear the "not initialised yet" flag in the kernel, and
>>>> how? And would that be enough.
>>>
>>> The flag wouldn't be stored in kernel, md kernel driver would just pass
>>> the flag with the uevent as it received in with ioctl/sysfs request to
>>> create a new dev. The udev in userspace would handle the state
>>> transition then from "flagged as not-initialized" state to "initilized"
>>> by checking the sequence of events as they come.
>>>
>>> We should have this sequence I assume:
>>>
>>> 1) "add" (creating dev, not usable yet)
>>> 2) "change" (activated dev, but not initialized yet)
>>> 3) "synthetic change" (after wiping the dev and closing it, the WATCH
>>> rule fires)
>>>
>>
>> "Should" is arguable, but there are no guarantees of this sequence.
>>
>> A particular cause of irregular sequencing is when an array is assembled
>> by the initrd, then after the real root is mounted, something runs
>> udevadm trigger --action=add
>> (e.g. systemd-udev-triggger.service).
>>
>> In that case, 'add' is the first and only message that the
>> full-root-available udev sees, so it is not safe to ignore the array as
>> "not usable yet".
>>
>>
>
> As for initrd case, there's OPTIONS+="db_persist" rule which is already
> used by dracut to keep udev db records when switching from initrd to
> root fs and replacing the udev daemon instance (other devices which do
> not have this option set will have udev db records cleared when switched
> to root fs). Not sure why it's not documented in udev yet (I already
> asked for that years ago), but the support is definitely there. So
> there's already a way to keep these records and to not lose track of the
> current state when switching from initrd. This makes it still possible
> to detect whether the synthetic "add" (or "change") event is sent before
> or after proper device initialization and to make it possible to react
> on the trigger (with synthetic uevents generated) still while ignoring
> the "add" event if that comes from the kernel before the device is
> initialized. And even if we're switching from initrd to root fs and
> restarting udev daemon.
>
>>>>
>>>> When mdadm creates an md array, at least 3 uevents get generated.
>>>> The first is generated when the md device object is created, either by
>>>> opening the /dev/mdXX file, or by writing magic to somewhere in sysfs.
>>>> The second is generated when the array is started and the content is
>>>> visible.
>>>> The third is generated when mdadm closes the file descriptor. It opens
>>>> /dev/mdXX for O_RDWR and performs ioctls on this, and then closes it.
>>>> Because udev uses inotify to watch for a 'close for a writable file
>>>> descriptor', this generates another event.
>>>>
>>>> We need to ensure that none of these cause udev to run anything that
>>>> inspects the contents of the array.
>>>> Of the three, only the second one is directly under the control of the
>>>> md module, so only that one can add an environment variable.
>>>>
>>>> It might be possible to avoid the last one (by not opening for write),
>>>> but I cannot see a way to avoid the first one.
>>>
>>> So the first event is the "add" event ("md device object created") - in
>>> this case, the device is not yet usable anyway I suppose, so we can skip
>>> udev scans for this event right away (unless it's the synthetic "add"
>>> event which is generated by writing "add" to /sys/block/.../uevent file
>>> or alternatively using udevadm trigger - but we should be able to
>>> recognize this event "synthetic add event" because it always comes after
>>> the activating "change" event, otherwise we can skip scans).
>>
>> "not yet usable anyway" is not reliable. It is very easy for mdadm to
>> finish making the array usable before udev gets around to processing the
>> initial "add" event.
>
> But in that case, there's certainly going to be a "change" event right
> after. We shouldn't skip the "add" completely and pretend it's the
> "change" now - there are rules for "add" events and rules for "change"
> events. The fact that things can change underneath while the event is
> being processed is something we need to count with in udev environment.
> The rules for "add" and "change" can be different. Also, various udev
> listeners/event subscribers can rely on this - otherwise we can't come
> up with proper state machine to check the sequence of events.
>
>>
>> You seem to be suggesting that udev rules should try to reverse-engineer
>> the sequence of events and deduce what is meant from that sequence. I
>> doubt that would be very robust.
>>
>>
>
> I wouldn't say reverse-engineering. The rules expect certain sequence
> during device setup and initialization. And if the events are coming out
> of order, we can at least detect something is wrong. If we have "add"
> for device addition and then "change" to complete the device setup
> (which is happening for dm and md devices), this rule is static and
> stable. As for the out-of-band synthetic uevents (the udevadm trigger or
> echo add/change > /sys/.../uevent, the WATCH rule...), this is different
> - the event has its source in userspace which is causing it, not the
> kernel driver directly. Synthetic uevents are separate - we can
> recognize them, but that kernel patch I mentioned will help us here even
> more to make the rules easier to detect this.
>
>>>
>>> The second event, which is the "change" event, would be marked with the
>>> new "not initialized" flag. And so we skip the scans in udev too.
>>
>> (one obvious problem with this approach is that it cannot work with old
>> kernels, while my approach only requires an update to mdadm)
>>
>
> The fact we need to update the kernel is true, but does it really
> matter? Such kernel patch is going to be small and easy to backport if
> needed for distributions with older kernels.
>
> This is all about providing the straightforward way of detecting the
> uevents - if we have only ADD and CHANGE while certain subsystem needs
> more to differentiate uevents further (like md and dm which can't simply
> map its events to those 2 events only), adding a variable to the event
> to make this difference clear is the most straightforward and clean way
> to do that I think. We don't need to rely on anything outside udev
> environment itself - it's directly a part of it as a variable in uevent
> which is already happening with certain events - e.g. change with with
> DISK_RO=1 set and there are lots of other existing examples...
>
> (Another way would be to introduce completely new uevents besides
> add/change/remove. That's also possible, but harder-to-deploy solution.)
>
>>>
>>> Then mdadm opens the devive, clears any old content/signatures the data
>>> area may contain, then closes it - this generates the third event -
>>> which is the "synthetic change" event (as a result of the inotify WATCH
>>> rule). And this one would drop the "not initialized" flag in udev db and
>>> the scans in udev are now enabled.
>>
>> Nope, it would be incorrect for mdadm to clear any old content.
>> Sometimes people want to ignore old content. Sometimes they very
>> definitely want to use it. It would be wrong for any code to try to
>> guess what is wanted.
>>
>>
>
> The mdadm is not going to guess - it can have an option to
> enable/disable the wiping on demand directly on command line (which is
> also what is actually done in LVM).
>
> Otherwise, if mdadm is not going to wipe/initialize the device itself,
> then it needs to wait for the external tool to do it (based on external
> configuration or on some manual wipefs-like call). So the sequence would be:
>
> 1) mdadm creating the device
> 2) mdadm setting up the device, marking it as "not initialized yet"
> 4a) mdadm waiting for the external decision to be made about wiping part
> 4b) external tool doing the wiping (or not) based on configuration or
> user's will
> 5) mdadm continuing and finishing when the wiping part is complete
>
> I expect mdadm to return only if the device is properly initialized
> otherwise it's harder for other tools called after mdadm to start
> working with the device - they need to poll for the state laboriously
> and check for readiness.
>
>>>
>>> So we should be able to handle all three kinds of events I think.
>>>
>>> Now, as for even better synthetic event recognition, I've proposed a
>>> patch recently where udev as well as any other tool generating these
>>> synthetic events can add variables for more detailed event
>>> identification, so this one should help us in the future even more in
>>> these situations: https://lkml.org/lkml/2017/3/15/461. With this, we can
>>> even disable the WATCH rule till the device is properly initialized and
>>> the tool can generate the event itself by writing the
>>> /sys/block/.../uevent file with a variable that the tool can then wait
>>> for even (so the tool doesn't exit till the device is not properly
>>> initialized). Once this initialization is all done, the WATCH rule can
>>> be enabled for the dev. Also, with this, we don't need to be afraid that
>>> some other tool fired the WATCH rule by chance if it opened the dev for
>>> RW and closed it by mistake before we had a chance to initialize it
>>> (which would fire the synthetic change event before the
>>> wiping/initialization).
>>
>> It sounds to me like you are adding a lot of complexity to an
>> already-fragile system. I'm not filled with confidence - sorry.
>>
>
> It would be great if we agree on some standard here. A variable directly
> in the uevent is one of such standards (of course, then we need to agree
> on variable naming and their meaning - but that's what I'm heading to).
>
> Once the kernel patch is in, I'll propose a patch for the udev daemon to
> add variable to detect the events coming from WATCH rule, the events
> coming from the trigger etc. So we can make a proper difference and
> rules can react to the events the most suitable way. But important here
> is that it's going to be marked in standard way. This, I believe,
> removes the fragility of the system where each subsystem is doing it's
> own magic to handle these events in non-standard ways... or saying it's
> impossible to detect and make a difference among events at all and hence
> giving up.
>
> I'm not saying that using out-of-band information channels like custom
> file somewhere in filesystem doesn't do its job, but it's surely
> creating an exception. Creating rules with such exceptions can cause
> situations which are harder to debug and harder to follow correctly if
> we're tracking whole stacks with different types of devices.
>
> In addition to that, with the possibility to add extra variables in
> synthesized uevents we're also able to wait for the event (via udev
> monitor) and that way, we can wait for the udev rules to be applied
> before continuing further with the tool that caused the synthesized
> uevent. Then, we can avoid code where we wait for some random time, for
> example (from the suggested patch):
>
> + /* Give udev a moment to process the Change event caused
> + * by the close.
> + */
> + usleep(100*1000);
> + udev_unblock();
>
> In this case, for example, we can generate an event by writing to the
> /sys/.../uevent file instead of relying on the WATCH rule and then wait
> for that properly since we can set a variable which we recognize when it
> gets back to us via uevent monitor.
>
> So it's not adding that much complexity. Compare that to the time we
> spent on debugging these things when the timing doesn't happen to be ideal.
>
> --
> Peter
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: Fwd: Please Help! Degraded Raid6
From: Andreas Klauer @ 2017-05-02 22:01 UTC (permalink / raw)
To: Bernd; +Cc: linux-raid
In-Reply-To: <C7A7AD68-1ABB-4186-8228-BC89B95E4571@gmx.de>
On Tue, May 02, 2017 at 11:11:10PM +0200, Bernd wrote:
> berndman@berndman:~$ sudo mdadm --examine /dev/sd[a-z]
You're using the wrong devices in your commands
so the output is not the least bit interesting.
> /dev/sda:
> MBR Magic : aa55
> Partition[0] : 3907027120 sectors at 2048 (type fd)
> /dev/sdb:
> MBR Magic : aa55
> Partition[0] : 3907027120 sectors at 2048 (type fd)
> /dev/sdh:
> Magic : a92b4efc
> Version : 1.2
> Feature Map : 0x2
> Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Well except for this but need all of them.
> Update Time : Fri Jul 10 22:32:34 2015
I hope it's not yet another case of a drive dropped out back
in 2015 and you never doing anything about it in two years...
Monitoring and instant email notification, is it that difficult?
Regards
Andreas Klauer
^ permalink raw reply
* Re: [PATCH] change back 0644 permission for Grow.c
From: Liuzhilong @ 2017-05-03 3:16 UTC (permalink / raw)
To: Jes Sorensen, Zhilong Liu; +Cc: linux-raid
In-Reply-To: <832ae4d1-4127-6b48-06b8-6b709a191b38@gmail.com>
On 05/02/2017 09:45 PM, Jes Sorensen wrote:
> On 05/02/2017 05:27 AM, Zhilong Liu wrote:
>> Fixes commit:
>> 26714713cd2b ("mdadm: Change timestamps to unsigned data type.")
>>
>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>> ---
>> Grow.c | 0
>> 1 file changed, 0 insertions(+), 0 deletions(-)
>> mode change 100755 => 100644 Grow.c
>>
>> diff --git a/Grow.c b/Grow.c
>> old mode 100755
>> new mode 100644
>>
>
> Hi Zhilong,
>
> Not quite sure what you expect me to do with this one......
>
> Maybe including the patch would help :)
>
commit: 26714713cd2bad9e0bf7f4669f6cc4659ceaab6c
it changes the permission of Grow.c and mdadm.h from 644 to 755,
the mdadm.h has changed permission back to 644 according to the
commit: 26714713cd2bad9e0bf7f4669f6cc4659ceaab6c from Neil.
Thus I sent this patch to change back 0644 permission for Grow.c.
The source file doesn't need the executable permission.
Thanks,
-Zhilong
> Cheers,
> Jes
>
> --
> 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 25/27] block: remove the discard_zeroes_data flag
From: Nicholas A. Bellinger @ 2017-05-03 3:33 UTC (permalink / raw)
To: hch@lst.de
Cc: Bart Van Assche, agk@redhat.com, lars.ellenberg@linbit.com,
snitzer@redhat.com, martin.petersen@oracle.com,
philipp.reisner@linbit.com, axboe@kernel.dk, shli@kernel.org,
linux-scsi@vger.kernel.org, dm-devel@redhat.com,
drbd-dev@lists.linbit.com, linux-block@vger.kernel.org,
target-devel@vger.kernel.org, linux-raid@vger.kernel.org
In-Reply-To: <20170502072328.GA11522@lst.de>
On Tue, 2017-05-02 at 09:23 +0200, hch@lst.de wrote:
> On Tue, May 02, 2017 at 12:16:13AM -0700, Nicholas A. Bellinger wrote:
> > Or, another options is use bdev_write_zeroes_sectors() to determine when
> > dev_attrib->unmap_zeroes_data should be set.
>
> Yes, that in combination with your patch to use bdev_write_zeroes_sectors
> for zeroing from write same seems like the right fix.
The larger target/iblock conversion patch looks like post v4.12 material
at this point, so to avoid breakage wrt to existing LBPRZ behavior, I'll
plan to push the following patch post -rc1.
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index d2f089c..e7caf78 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -851,7 +851,7 @@ bool target_configure_unmap_from_queue(struct se_dev_attrib *attrib,
attrib->unmap_granularity = q->limits.discard_granularity / block_size;
attrib->unmap_granularity_alignment = q->limits.discard_alignment /
block_size;
- attrib->unmap_zeroes_data = 0;
+ attrib->unmap_zeroes_data = (q->limits.max_write_zeroes_sectors);
return true;
}
EXPORT_SYMBOL(target_configure_unmap_from_queue);
^ permalink raw reply related
* Re: [PATCH v6 0/4] Broadcom SBA RAID support
From: Anup Patel @ 2017-05-03 3:45 UTC (permalink / raw)
To: Vinod Koul
Cc: Jassi Brar, Rob Herring, Mark Rutland, Herbert Xu,
David S . Miller, Dan Williams, Ray Jui, Scott Branden, Jon Mason,
Rob Rice, BCM Kernel Feedback, dmaengine-u79uwXL29TY76Z2rM5mHXA,
Device Tree, Linux ARM Kernel, Linux Kernel,
linux-crypto-u79uwXL29TY76Z2rM5mHXA, linux-raid
In-Reply-To: <20170330044940.GU9308@localhost>
Hi Vinod,
The Broadcom FlexRM patchset have been
merged in v4.11.
I think you now can take this patchset in next
merge window. Right??
Regards,
Anup
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 0/4] Broadcom SBA RAID support
From: Vinod Koul @ 2017-05-03 4:59 UTC (permalink / raw)
To: Anup Patel
Cc: Jassi Brar, Rob Herring, Mark Rutland, Herbert Xu,
David S . Miller, Dan Williams, Ray Jui, Scott Branden, Jon Mason,
Rob Rice, BCM Kernel Feedback, dmaengine, Device Tree,
Linux ARM Kernel, Linux Kernel, linux-crypto, linux-raid
In-Reply-To: <CAALAos8hVtCqzpv3nvPAGnm=WKMfcXeBEFm5Yb0gWwYGLE4SeA@mail.gmail.com>
On Wed, May 03, 2017 at 09:15:20AM +0530, Anup Patel wrote:
> Hi Vinod,
>
> The Broadcom FlexRM patchset have been
> merged in v4.11.
>
> I think you now can take this patchset in next
> merge window. Right??
Sure, please rebase and resend after -rc1 is out
--
~Vinod
^ permalink raw reply
* Re: [PATCH v6 0/4] Broadcom SBA RAID support
From: Anup Patel @ 2017-05-03 5:09 UTC (permalink / raw)
To: Vinod Koul
Cc: Jassi Brar, Rob Herring, Mark Rutland, Herbert Xu,
David S . Miller, Dan Williams, Ray Jui, Scott Branden, Jon Mason,
Rob Rice, BCM Kernel Feedback, dmaengine, Device Tree,
Linux ARM Kernel, Linux Kernel, linux-crypto, linux-raid
In-Reply-To: <20170503045926.GV6263@localhost>
On Wed, May 3, 2017 at 10:29 AM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Wed, May 03, 2017 at 09:15:20AM +0530, Anup Patel wrote:
>> Hi Vinod,
>>
>> The Broadcom FlexRM patchset have been
>> merged in v4.11.
>>
>> I think you now can take this patchset in next
>> merge window. Right??
>
> Sure, please rebase and resend after -rc1 is out
Sure, I will do that.
Regards,
Anup
^ permalink raw reply
* Re:
From: H.A @ 2017-05-03 6:23 UTC (permalink / raw)
To: Recipients
With profound love in my heart, I Kindly Oblige your interest to very important proposal.. It is Truly Divine and require your utmost attention..........
S hlubokou láskou v mém srdci, Laskave jsem prinutit svuj zájem k návrhu .. Je velmi duležité, skutecne Divine a vyžadují vaši nejvyšší pozornost.
Kontaktujte me prímo pres: helenaroberts99@gmail.com pro úplné podrobnosti.complete.
HELINA .A ROBERTS
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
^ permalink raw reply
* Re: [PATCH 0/6] md: Fine-tuning for some function implementations
From: Nikola Pajkovsky @ 2017-05-03 7:38 UTC (permalink / raw)
To: NeilBrown
Cc: SF Markus Elfring, linux-raid, Shaohua Li, LKML, kernel-janitors
In-Reply-To: <87efw7q6tm.fsf@notabene.neil.brown.name>
NeilBrown <neilb@suse.com> writes:
> On Tue, May 02 2017, SF Markus Elfring wrote:
>
>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Tue, 2 May 2017 16:12:34 +0200
>>
>> Some update suggestions were taken into account
>> from static source code analysis.
>>
>> Markus Elfring (6):
>> Replace seven seq_printf() calls by seq_putc()
>> Replace 17 seq_printf() calls by seq_puts()
>
> Why does anyone care whether printf or putc/puts is used? Really it
> doesn't matter *at* *all*.
It could matter, but this patchset does not state it all. Does not
have any perf tests and so on.
f7a5f132b447 ("proc: faster /proc/*/status")
68ba0326b4e1 ("proc: much faster /proc/vmstat")
> I don't object to the patch but if it would up to me I probably wouldn't
> bother applying it it either.
> Sometimes I just want to "print" something and I don't want to care
> whether it is a constant string or a single-byte constant string, or
> something more general.
> I see these changes as worse than white-space fixes.
>
> NeilBrown
>
>
>> Adjust four function calls together with a variable assignment
>> Use seq_puts() in faulty_status()
>> Adjust six function calls together with a variable assignment in faulty_status()
>> Add some spaces for better code readability
>>
>> drivers/md/faulty.c | 48 ++++++++++++++++++++++----------------
>> drivers/md/md.c | 66 +++++++++++++++++++++++++++++------------------------
>> 2 files changed, 64 insertions(+), 50 deletions(-)
>>
>> --
>> 2.12.2
--
Nikola
^ permalink raw reply
* Re: [mdadm PATCH] Create: tell udev md device is not ready when first created.
From: Peter Rajnoha @ 2017-05-03 14:13 UTC (permalink / raw)
To: Jes Sorensen, NeilBrown; +Cc: linux-raid, dm-devel
In-Reply-To: <c5fe5d16-5ca6-94b0-fd5b-0b6009d78bab@gmail.com>
On 05/02/2017 03:32 PM, Jes Sorensen wrote:
> On 04/28/2017 05:28 AM, Peter Rajnoha wrote:
>> On 04/28/2017 07:05 AM, NeilBrown wrote:
>>>
>>> When an array is created the content is not initialized,
>>> so it could have remnants of an old filesystem or md array
>>> etc on it.
>>> udev will see this and might try to activate it, which is almost
>>> certainly not what is wanted.
>>>
>>> So create a mechanism for mdadm to communicate with udev to tell
>>> it that the device isn't ready. This mechanism is the existance
>>> of a file /run/mdadm/created-mdXXX where mdXXX is the md device name.
>>>
>>> When creating an array, mdadm will create the file.
>>> A new udev rule file, 01-md-raid-creating.rules, will detect the
>>> precense of thst file and set ENV{SYSTEMD_READY}="0".
>>> This is fairly uniformly used to suppress actions based on the
>>> contents of the device.
>>
>> The scans in udev are primarily directed by blkid call which detects the
>> signatures and based on this information various other udev rules fire.
>>
>> The blkid as well as wipefs uses common libblkid library to detect these
>> signatures - is mdadm going to use libblkid to wipe the signatures on MD
>> device initialization or is it relying on external tools to do this? How
>> is mdadm actually initializing/wiping the newly created MD device?
>
> mdadm doesn't wipe data and it isn't supposed to. Being able to create
> an array from drives with existing data is a feature.
>
> It is the responsibility of the system administrator to handle drives
> with existing data, in the same way the administrator is expected to
> handle insertion of USB drives or external drives being powered on.
There's a difference though - when you're *creating* a completely new
device that is an abstraction over existing devices, you (most of the
time) expect that new device to be initialized. For those corner cases
where people do need to keep the old data, there can be an option to do
that. When you're inserting existing drives, you're not creating them -
when those device come from factory (they're "created"), they never
contain garbage and old data when you buy them.
--
Peter
^ permalink raw reply
* Re: [dm-devel] [mdadm PATCH 4/4] Create: tell udev device is not ready when first created.
From: Peter Rajnoha @ 2017-05-03 14:27 UTC (permalink / raw)
To: Jes Sorensen, NeilBrown; +Cc: linux-raid, dm-devel
In-Reply-To: <88ba4ec0-9c4d-aa07-b9b5-66cad893e130@gmail.com>
On 05/02/2017 03:40 PM, Jes Sorensen wrote:
> On 05/02/2017 07:40 AM, Peter Rajnoha wrote:
>> On 05/01/2017 06:35 AM, NeilBrown wrote:
>>> On Fri, Apr 28 2017, Peter Rajnoha wrote:
>>>> Then mdadm opens the devive, clears any old content/signatures the data
>>>> area may contain, then closes it - this generates the third event -
>>>> which is the "synthetic change" event (as a result of the inotify WATCH
>>>> rule). And this one would drop the "not initialized" flag in udev db
>>>> and
>>>> the scans in udev are now enabled.
>>>
>>> Nope, it would be incorrect for mdadm to clear any old content.
>>> Sometimes people want to ignore old content. Sometimes they very
>>> definitely want to use it. It would be wrong for any code to try to
>>> guess what is wanted.
>>
>> The mdadm is not going to guess - it can have an option to
>> enable/disable the wiping on demand directly on command line (which is
>> also what is actually done in LVM).
>
> I know the anaconda team keeps pushing for the nonsense of being able to
> wipe drives on creation. It is wrong, it is broken, and it is not going
> to happen.
>
I'm not thinking about anaconda at the moment at all. It's just one of
the many users of mdadm. I'm thinking about a fix in general for all the
users which expect the device to be initialized properly when mdadm
--create returns.
>> Otherwise, if mdadm is not going to wipe/initialize the device itself,
>> then it needs to wait for the external tool to do it (based on external
>> configuration or on some manual wipefs-like call). So the sequence
>> would be:
>>
>> 1) mdadm creating the device
>> 2) mdadm setting up the device, marking it as "not initialized yet"
>> 4a) mdadm waiting for the external decision to be made about wiping part
>> 4b) external tool doing the wiping (or not) based on configuration or
>> user's will
>> 5) mdadm continuing and finishing when the wiping part is complete
>>
>> I expect mdadm to return only if the device is properly initialized
>> otherwise it's harder for other tools called after mdadm to start
>> working with the device - they need to poll for the state laboriously
>> and check for readiness.
>
> What defines readiness? Some believe a raid1 array must be fully
> assembled with all members, other setups are happy to have one running
> drive in place.....
>
With "ready" I mean the time when it's safe to do a scan without seeing
old data (garbage) that may confuse udev hooks and udev event listeners.
That scan is done at some time - at the time the activating "change"
uevent comes and this rule does not pass
ATTR{md/array_state}=="|clear|inactive" (if it passes, the device is not
scanned yet).
> 4a/4b in your list here once again has no place in mdadm. Please kindly
> tell the anaconda team to go back and handle this properly instead.
The mdadm is creating the dev and so it should be responsible primarily
for providing a device which is cleared and ready for use without
causing any confusion on event-based system where various scans are
executed based on incoming udev events.
Alternatively, if mdadm is not going to be the place where the wiping
happens, I'd expect at least the sequence above (which is more complex,
yes, that's why I think having the wiping directly in mdadm is much
easier solution).
If you don't wipe the data and you don't give time for others to hook in
to do that, you make it harder for others (they need to deactivate all
the stack/garbage that is found in the data area after previous use).
Also, we can't reliably call wiping on the underlying components first,
because once they become MD components, the data are for the MD device
has an offset and new data range is revealed from the underlying devices
which may expose old signatures which were not visible on those
underlying devices before the MD device got created.
--
Peter
^ permalink raw reply
* Re: [mdadm PATCH] Create: tell udev md device is not ready when first created.
From: Peter Rajnoha @ 2017-05-03 14:32 UTC (permalink / raw)
To: Jes Sorensen, NeilBrown; +Cc: linux-raid, dm-devel
In-Reply-To: <01f50f57-c55e-e87f-b5a1-ded4647368b2@gmail.com>
On 05/02/2017 03:42 PM, Jes Sorensen wrote:
> On 04/28/2017 01:05 AM, NeilBrown wrote:
>>
>> When an array is created the content is not initialized,
>> so it could have remnants of an old filesystem or md array
>> etc on it.
>> udev will see this and might try to activate it, which is almost
>> certainly not what is wanted.
>>
>> So create a mechanism for mdadm to communicate with udev to tell
>> it that the device isn't ready. This mechanism is the existance
>> of a file /run/mdadm/created-mdXXX where mdXXX is the md device name.
>>
>> When creating an array, mdadm will create the file.
>> A new udev rule file, 01-md-raid-creating.rules, will detect the
>> precense of thst file and set ENV{SYSTEMD_READY}="0".
>> This is fairly uniformly used to suppress actions based on the
>> contents of the device.
>>
>> Signed-off-by: NeilBrown <neilb@suse.com>
>> ---
>> Assemble.c | 2 +-
>> Build.c | 2 +-
>> Create.c | 9 +++++++-
>> Incremental.c | 4 ++--
>> Makefile | 4 ++--
>> lib.c | 29 +++++++++++++++++++++++++
>> mdadm.h | 4 +++-
>> mdopen.c | 52
>> ++++++++++++++++++++++++++++-----------------
>> udev-md-raid-creating.rules | 7 ++++++
>> 9 files changed, 86 insertions(+), 27 deletions(-)
>> create mode 100644 udev-md-raid-creating.rules
>
> Applied!
>
How is this solving the problem then? The patch is flagging a device as
not ready, then clearing the flag after some time. Where does the wiping
happen actually?
--
Peter
^ permalink raw reply
* Re: [PATCH 25/27] block: remove the discard_zeroes_data flag
From: Mike Snitzer @ 2017-05-03 14:33 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: hch@lst.de, Bart Van Assche, agk@redhat.com,
lars.ellenberg@linbit.com, martin.petersen@oracle.com,
philipp.reisner@linbit.com, axboe@kernel.dk, shli@kernel.org,
linux-scsi@vger.kernel.org, dm-devel@redhat.com,
drbd-dev@lists.linbit.com, linux-block@vger.kernel.org,
target-devel@vger.kernel.org, linux-raid@vger.kernel.org
In-Reply-To: <1493782395.23202.84.camel@haakon3.risingtidesystems.com>
On Tue, May 02 2017 at 11:33pm -0400,
Nicholas A. Bellinger <nab@linux-iscsi.org> wrote:
> On Tue, 2017-05-02 at 09:23 +0200, hch@lst.de wrote:
> > On Tue, May 02, 2017 at 12:16:13AM -0700, Nicholas A. Bellinger wrote:
> > > Or, another options is use bdev_write_zeroes_sectors() to determine when
> > > dev_attrib->unmap_zeroes_data should be set.
> >
> > Yes, that in combination with your patch to use bdev_write_zeroes_sectors
> > for zeroing from write same seems like the right fix.
>
> The larger target/iblock conversion patch looks like post v4.12 material
> at this point, so to avoid breakage wrt to existing LBPRZ behavior, I'll
> plan to push the following patch post -rc1.
>
> diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
> index d2f089c..e7caf78 100644
> --- a/drivers/target/target_core_device.c
> +++ b/drivers/target/target_core_device.c
> @@ -851,7 +851,7 @@ bool target_configure_unmap_from_queue(struct se_dev_attrib *attrib,
> attrib->unmap_granularity = q->limits.discard_granularity / block_size;
> attrib->unmap_granularity_alignment = q->limits.discard_alignment /
> block_size;
> - attrib->unmap_zeroes_data = 0;
> + attrib->unmap_zeroes_data = (q->limits.max_write_zeroes_sectors);
> return true;
> }
> EXPORT_SYMBOL(target_configure_unmap_from_queue);
>
Completely a nit but: why the extra parenthesis?
^ permalink raw reply
* Re: [dm-devel] [mdadm PATCH 4/4] Create: tell udev device is not ready when first created.
From: Jes Sorensen @ 2017-05-03 14:41 UTC (permalink / raw)
To: Peter Rajnoha, NeilBrown; +Cc: linux-raid, dm-devel
In-Reply-To: <4b422ccc-b666-e9ae-4067-cb0c4e407c04@redhat.com>
On 05/03/2017 10:27 AM, Peter Rajnoha wrote:
> On 05/02/2017 03:40 PM, Jes Sorensen wrote:
>> On 05/02/2017 07:40 AM, Peter Rajnoha wrote:
>>> On 05/01/2017 06:35 AM, NeilBrown wrote:
>>>> On Fri, Apr 28 2017, Peter Rajnoha wrote:
>>>>> Then mdadm opens the devive, clears any old content/signatures the data
>>>>> area may contain, then closes it - this generates the third event -
>>>>> which is the "synthetic change" event (as a result of the inotify WATCH
>>>>> rule). And this one would drop the "not initialized" flag in udev db
>>>>> and
>>>>> the scans in udev are now enabled.
>>>>
>>>> Nope, it would be incorrect for mdadm to clear any old content.
>>>> Sometimes people want to ignore old content. Sometimes they very
>>>> definitely want to use it. It would be wrong for any code to try to
>>>> guess what is wanted.
>>>
>>> The mdadm is not going to guess - it can have an option to
>>> enable/disable the wiping on demand directly on command line (which is
>>> also what is actually done in LVM).
>>
>> I know the anaconda team keeps pushing for the nonsense of being able to
>> wipe drives on creation. It is wrong, it is broken, and it is not going
>> to happen.
>
> I'm not thinking about anaconda at the moment at all. It's just one of
> the many users of mdadm. I'm thinking about a fix in general for all the
> users which expect the device to be initialized properly when mdadm
> --create returns.
Well mdadm --create already initializes the array properly, so problem
solved.
I saw this request repeatedly when I worked at Red Hat, and I got it
again from my replacement who had been asked to do the same dirty work
that I refused to do. Now we see the same request from you, so it is
rather easy to assume it came from there again.
>>> Otherwise, if mdadm is not going to wipe/initialize the device itself,
>>> then it needs to wait for the external tool to do it (based on external
>>> configuration or on some manual wipefs-like call). So the sequence
>>> would be:
>>>
>>> 1) mdadm creating the device
>>> 2) mdadm setting up the device, marking it as "not initialized yet"
>>> 4a) mdadm waiting for the external decision to be made about wiping part
>>> 4b) external tool doing the wiping (or not) based on configuration or
>>> user's will
>>> 5) mdadm continuing and finishing when the wiping part is complete
>>>
>>> I expect mdadm to return only if the device is properly initialized
>>> otherwise it's harder for other tools called after mdadm to start
>>> working with the device - they need to poll for the state laboriously
>>> and check for readiness.
>>
>> What defines readiness? Some believe a raid1 array must be fully
>> assembled with all members, other setups are happy to have one running
>> drive in place.....
>
> With "ready" I mean the time when it's safe to do a scan without seeing
> old data (garbage) that may confuse udev hooks and udev event listeners.
> That scan is done at some time - at the time the activating "change"
> uevent comes and this rule does not pass
> ATTR{md/array_state}=="|clear|inactive" (if it passes, the device is not
> scanned yet).
I am not sure what you mean by old data here?
>> 4a/4b in your list here once again has no place in mdadm. Please kindly
>> tell the anaconda team to go back and handle this properly instead.
>
> The mdadm is creating the dev and so it should be responsible primarily
> for providing a device which is cleared and ready for use without
> causing any confusion on event-based system where various scans are
> executed based on incoming udev events.
This is exactly what it does. There is no difference to how mdadm does
it compared to how it works if you plugin a new USB drive which may have
data on it or if you assemble an iSCSI device that has metadata on it.
Are you also going to suggest the iSCSI tools have a wipe metadata mode
added to them?
> Alternatively, if mdadm is not going to be the place where the wiping
> happens, I'd expect at least the sequence above (which is more complex,
> yes, that's why I think having the wiping directly in mdadm is much
> easier solution).
>
> If you don't wipe the data and you don't give time for others to hook in
> to do that, you make it harder for others (they need to deactivate all
> the stack/garbage that is found in the data area after previous use).
>
> Also, we can't reliably call wiping on the underlying components first,
> because once they become MD components, the data are for the MD device
> has an offset and new data range is revealed from the underlying devices
> which may expose old signatures which were not visible on those
> underlying devices before the MD device got created.
Yes there is a risk that creating an array that exactly matches old
metadata despite those signatures haven gotten wiped. This is a feature,
people use it to recover their data, and it is not going to be removed.
If you don't want this, disable automatic processing of udev events that
may do something on top of it. This is what anaconda should be doing.
Jes
^ permalink raw reply
* Re: [mdadm PATCH] Create: tell udev md device is not ready when first created.
From: Jes Sorensen @ 2017-05-03 14:44 UTC (permalink / raw)
To: Peter Rajnoha, NeilBrown; +Cc: linux-raid, dm-devel
In-Reply-To: <2f431da6-0dce-b485-4109-6f8669814430@redhat.com>
On 05/03/2017 10:13 AM, Peter Rajnoha wrote:
> On 05/02/2017 03:32 PM, Jes Sorensen wrote:
>> On 04/28/2017 05:28 AM, Peter Rajnoha wrote:
>>> On 04/28/2017 07:05 AM, NeilBrown wrote:
>>>>
>>>> When an array is created the content is not initialized,
>>>> so it could have remnants of an old filesystem or md array
>>>> etc on it.
>>>> udev will see this and might try to activate it, which is almost
>>>> certainly not what is wanted.
>>>>
>>>> So create a mechanism for mdadm to communicate with udev to tell
>>>> it that the device isn't ready. This mechanism is the existance
>>>> of a file /run/mdadm/created-mdXXX where mdXXX is the md device name.
>>>>
>>>> When creating an array, mdadm will create the file.
>>>> A new udev rule file, 01-md-raid-creating.rules, will detect the
>>>> precense of thst file and set ENV{SYSTEMD_READY}="0".
>>>> This is fairly uniformly used to suppress actions based on the
>>>> contents of the device.
>>>
>>> The scans in udev are primarily directed by blkid call which detects the
>>> signatures and based on this information various other udev rules fire.
>>>
>>> The blkid as well as wipefs uses common libblkid library to detect these
>>> signatures - is mdadm going to use libblkid to wipe the signatures on MD
>>> device initialization or is it relying on external tools to do this? How
>>> is mdadm actually initializing/wiping the newly created MD device?
>>
>> mdadm doesn't wipe data and it isn't supposed to. Being able to create
>> an array from drives with existing data is a feature.
>>
>> It is the responsibility of the system administrator to handle drives
>> with existing data, in the same way the administrator is expected to
>> handle insertion of USB drives or external drives being powered on.
>
> There's a difference though - when you're *creating* a completely new
> device that is an abstraction over existing devices, you (most of the
> time) expect that new device to be initialized. For those corner cases
> where people do need to keep the old data, there can be an option to do
> that. When you're inserting existing drives, you're not creating them -
> when those device come from factory (they're "created"), they never
> contain garbage and old data when you buy them.
Sorry, you are once again assuming *your* corner case to be the default
that everyone else expects. mdadm users (except for you and anaconda)
expect the current behavior.
Devices that come from the factory pretty much always contain old data,
plugin a USB drive and it will be filled with windows backup crap.
Jes
^ permalink raw reply
* Re: [dm-devel] [mdadm PATCH] Create: tell udev md device is not ready when first created.
From: Jes Sorensen @ 2017-05-03 14:45 UTC (permalink / raw)
To: Peter Rajnoha, NeilBrown; +Cc: linux-raid, dm-devel
In-Reply-To: <2fb51c71-4398-e206-4a94-5dcd13d78603@redhat.com>
On 05/03/2017 10:32 AM, Peter Rajnoha wrote:
> On 05/02/2017 03:42 PM, Jes Sorensen wrote:
>> On 04/28/2017 01:05 AM, NeilBrown wrote:
>>>
>>> When an array is created the content is not initialized,
>>> so it could have remnants of an old filesystem or md array
>>> etc on it.
>>> udev will see this and might try to activate it, which is almost
>>> certainly not what is wanted.
>>>
>>> So create a mechanism for mdadm to communicate with udev to tell
>>> it that the device isn't ready. This mechanism is the existance
>>> of a file /run/mdadm/created-mdXXX where mdXXX is the md device name.
>>>
>>> When creating an array, mdadm will create the file.
>>> A new udev rule file, 01-md-raid-creating.rules, will detect the
>>> precense of thst file and set ENV{SYSTEMD_READY}="0".
>>> This is fairly uniformly used to suppress actions based on the
>>> contents of the device.
>>>
>>> Signed-off-by: NeilBrown <neilb@suse.com>
>>> ---
>>> Assemble.c | 2 +-
>>> Build.c | 2 +-
>>> Create.c | 9 +++++++-
>>> Incremental.c | 4 ++--
>>> Makefile | 4 ++--
>>> lib.c | 29 +++++++++++++++++++++++++
>>> mdadm.h | 4 +++-
>>> mdopen.c | 52
>>> ++++++++++++++++++++++++++++-----------------
>>> udev-md-raid-creating.rules | 7 ++++++
>>> 9 files changed, 86 insertions(+), 27 deletions(-)
>>> create mode 100644 udev-md-raid-creating.rules
>>
>> Applied!
>>
>
> How is this solving the problem then? The patch is flagging a device as
> not ready, then clearing the flag after some time. Where does the wiping
> happen actually?
There is *NO* wiping happening, there is *NO* wiping going to happen.
Can we stop this nonsense now!
Thank you!
Jes
^ permalink raw reply
* Re: [PATCH] change back 0644 permission for Grow.c
From: Jes Sorensen @ 2017-05-03 14:51 UTC (permalink / raw)
To: Liuzhilong, Zhilong Liu; +Cc: linux-raid
In-Reply-To: <97f9931d-2af5-fad1-bf3b-ad476b23de72@suse.de>
On 05/02/2017 11:16 PM, Liuzhilong wrote:
>
>
> On 05/02/2017 09:45 PM, Jes Sorensen wrote:
>> On 05/02/2017 05:27 AM, Zhilong Liu wrote:
>>> Fixes commit:
>>> 26714713cd2b ("mdadm: Change timestamps to unsigned data type.")
>>>
>>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>>> ---
>>> Grow.c | 0
>>> 1 file changed, 0 insertions(+), 0 deletions(-)
>>> mode change 100755 => 100644 Grow.c
>>>
>>> diff --git a/Grow.c b/Grow.c
>>> old mode 100755
>>> new mode 100644
>>>
>>
>> Hi Zhilong,
>>
>> Not quite sure what you expect me to do with this one......
>>
>> Maybe including the patch would help :)
>>
>
> commit: 26714713cd2bad9e0bf7f4669f6cc4659ceaab6c
> it changes the permission of Grow.c and mdadm.h from 644 to 755,
> the mdadm.h has changed permission back to 644 according to the
> commit: 26714713cd2bad9e0bf7f4669f6cc4659ceaab6c from Neil.
> Thus I sent this patch to change back 0644 permission for Grow.c.
> The source file doesn't need the executable permission.
You are right indeed!
Applied!
Thanks,
Jes
^ permalink raw reply
* Re: on assembly and recovery of a hardware RAID
From: Alfred Matthews @ 2017-05-03 15:44 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <CAAZLhTdobNxkYNmp6y3cOrLAdMfTYt3kodzM0+a7E-G=q1QGtw@mail.gmail.com>
Sorry to disappear for a bit.
Of necessity I've migrated to another working system and I'm in Arch.
Utility here is fsck.hfs as opposed to hpfsck. I'd like to post that
output.
mdadm --build /dev/md0 --level=0 -n2 --chunk=8K /dev/sdb2 /dev/sdc2
works, then,
fsck.hfs -v /dev/md0
*** Checking volume MDB
drSigWord = 0xffffffff
drCrDate = Mon Feb 6 06:28:15 2040
drLsMod = Mon Feb 6 06:28:15 2040
drAtrb = BUSY | HLOCKED | UMOUNTED | BBSPARED | BVINCONSIS |
COPYPROT | SLOCKED
drNmFls = 65535
drVBMSt = 65535
drAllocPtr = 65535
drNmAlBlks = 65535
drAlBlkSiz = 4294967295
drClpSiz = 4294967295
drAlBlSt = 65535
drNxtCNID = 4294967295
drFreeBks = 65535
drVN = ""
drVolBkUp = Mon Feb 6 06:28:15 2040
drVSeqNum = 4294967295
drWrCnt = 4294967295
drXTClpSiz = 4294967295
drCTClpSiz = 4294967295
drNmRtDirs = 65535
drFilCnt = 4294967295
drDirCnt = 4294967295
drEmbedSigWord = 0xffff
drEmbedExtent = 65535[65535..131069]
drXTFlSize = 4294967295
drXTExtRec =
65535[65535..131069]+65535[65535..131069]+65535[65535..131069]
drCTFlSize = 4294967295
drCTExtRec =
65535[65535..131069]+65535[65535..131069]+65535[65535..131069]
Bad volume signature (0xffffffff); should be 0x4244. Fix? y
Volume creation date is in the future (Mon Feb 6 06:28:15 2040). Fix? y
Volume last modify date is in the future (Mon Feb 6 06:28:15 2040). Fix? y
Volume bitmap starts at unusual location (1), not 3. Fix? y
*** Checking volume structure
*** Checking extents overflow B*-tree
It's possible that I was unwise in accepting the changes from fsck.
>>
>> How is your C coding? You could
>> apt-get source hfsplus
>> and hack the code and try to build it yourself....
>>
Still fine. By now not clear if this is what's needed.
Al
^ permalink raw reply
* Re: [GIT PULL] MD update for 4.12
From: Linus Torvalds @ 2017-05-03 17:27 UTC (permalink / raw)
To: Shaohua Li
Cc: Linux Kernel Mailing List, linux-raid@vger.kernel.org, Neil Brown
In-Reply-To: <20170501225048.whxlxsoebnxcfyto@kernel.org>
On Mon, May 1, 2017 at 3:50 PM, Shaohua Li <shli@kernel.org> wrote:
>
> Please pull MD update for 4.12. There are conflicts between MD tree and block
> tree, so I did a merge before the pull request.
I pulled this, but *please* don't do those broken merges.
Why is it broken? Let me just count the merge message in its entirety:
"Merge branch 'md-next' into md-linus"
that's it. That doesn't really help anything. There is *no*
explanation for the merge. No "What", no "Why", nothing to explain why
you are merging with that _particular_ point in history.
In contrast, when I do merges, even if I don't have a lot of commit
message (and I try to have explanations - I will complain to
submaintainers if they don't give me enough information), there's very
much some implied information: "somebody asked Linus to pull this
point". I don't just randomly merge random points in development.
That merge, in contrast, very much merged completely random points in time.
In fact, the particular point you merged makes no sense at all and is
probably the *worst* possible kind of merge point. It's not just a
random point in time (me having just merged the LED drivers), it's a
point in the middle of the first day of the merge window. Which is
just about the *worst* possible thing to do, because that's often the
point that is the least reliable.
Seriously. DO NOT DO THIS!
I'm perfectly happy handling merge conflicts. I *much* prefer that
over this kind of "easy but crappy merge".
I appreciate it a lot if submaintainers do a "test merge" to see what
might conflict, and then explain what the conflict is and why it
happened in the pull request. That's absolutely lovely - it's a nice
heads-up about what happened, and it shows that you cared and you knew
about the conflict, and it makes me all warm and fuzzy.
But this kind of random bad merge of me tree during the most volatile
time of the merge window? That's just wrong. It doesn't even help me,
I probably spent about as much time looking at the conflct with your
merge in place as I would have when I resolved it, and then spent more
time writing this "please don't do this" rant than it would have taken
me to just do the merge.
Yes, occasionally the conflicts end up being nasty enough that I ask
people to verify my work. And _very_ occasionally I might ask you to
actually do the merge for me, but I don't recall when something was
really messy enough for that to happen. This did not look that messy
at all.
I have sent a variation of this email to people (and the mailing list)
probably approaching a hundred times over the 10+ years we've used
git.
Don't do back-merges unless you have a *really* good reason for it,
and if you have that reason, you had damn well *explain* that reason
in the merge message.
And "make the merge easier for Linus" is _never_ a good reason. If you
want to make life easier for me, please just do that test-merge and
explanation of what's going on: lots of maintainers do that, and I
really appreciate it. If you feel the merge isn't trivial, you can
even make the merge available as a ".. this is how I merged it" branch
- when people do that, I will actually end up doing the merge on my
own first (without looking at your merge), and then re-do the whole
thing *with* your merge in a test-branch, and verify the differences.
Because the differences (if there are any - it's seldom the case other
than trivial whitespace or other slight differences) are actually
really interesting markers for me, and tend to show where subtle
issues were.
But don't ask me to pull a pre-merged thing that just hides the conflicts.
Please please please don't do this.
Linus
^ 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