Linux RAID subsystem development
 help / color / mirror / Atom feed
* Re: Big-endian RAID5 recovery problem
From: Roman Mamedov @ 2017-05-02  8:59 UTC (permalink / raw)
  To: Adam Thompson; +Cc: MUUG Roundtable, linux-raid
In-Reply-To: <5c6e6e5d93d4056839e4f370e00a8e08@mail.athompso.net>

On Mon, 01 May 2017 16:39:07 -0500
Adam Thompson <athompso@athompso.net> wrote:

> I can't find any modern big-endian Linux systems... looks like all the 
> ARM distros run in little-endian mode.

Here are QEMU images for debian-mips (should be big-endian, as opposed to
debian-mipsel): https://people.debian.org/~aurel32/qemu/mips/

Of course it will run purely in software, but most likely more than fast enough
to copy away the data.

Not entirely sure that particular emulated MIPS system support more than 4
drives), but it appears that a starting point could be (man qemu-system):

           Instead of -hda, -hdb, -hdc, -hdd, you can use:

                   qemu-system-i386 -drive file=file,index=0,media=disk
                   qemu-system-i386 -drive file=file,index=1,media=disk
                   qemu-system-i386 -drive file=file,index=2,media=disk
                   qemu-system-i386 -drive file=file,index=3,media=disk

with indexes 0..5, as you need the boot disk, all 4 drives, and one more as
the backup destination.

May or may not be the best way, but IMO beats trying to hex-edit the
superblock right away.

-- 
With respect,
Roman

^ permalink raw reply

* [PATCH] change back 0644 permission for Grow.c
From: Zhilong Liu @ 2017-05-02  9:27 UTC (permalink / raw)
  To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu

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
-- 
2.6.6


^ permalink raw reply

* Re: Recovery on new 2TB disk: finish=7248.4min (raid1)
From: Nix @ 2017-05-02 10:02 UTC (permalink / raw)
  To: Wols Lists
  Cc: Roman Mamedov, John Stoffel, Mateusz Korniak, Ron Leach,
	linux-raid
In-Reply-To: <590822E2.1080009@youngman.org.uk>

On 2 May 2017, Wols Lists outgrape:

> On 01/05/17 23:03, Roman Mamedov wrote:
>>> > That's what I understood you to mean, but you are aware that SOME raid 
>>> > management still has to be done with echo > /sys/... ?
>>> > 
>>> > So mdadm isn't perfect, not  by a long chalk, yet :-)
>> Well, why not post some examples of what you find yourself doing often
>> via /sys, that's not available in mdadm (maybe as a new thread).
>
> I *should* do, rather than I *do* do, but your everyday general
> maintenance tasks, like scrubbing?

You can scrub with mdadm now. :) IIRC (I haven't started using it yet)
the syntax is something like

mdadm --misc --action=check /dev/md/my-array

or

mdadm --misc --action=repair /dev/md/my-array

(though frankly it has never been clear to me which is preferable for a
regular scrub. Probably check on a RAID-6, repair on a RAID-5 where such
failures are much more potentially catastrophic...)

^ 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-02 11:40 UTC (permalink / raw)
  To: NeilBrown, Jes Sorensen; +Cc: linux-raid, dm-devel
In-Reply-To: <8737cpry7u.fsf@notabene.neil.brown.name>

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

^ permalink raw reply

* Re: Recover array after I panicked
From: Patrik Dahlström @ 2017-05-02 13:08 UTC (permalink / raw)
  To: Andreas Klauer; +Cc: Brad Campbell, linux-raid
In-Reply-To: <20170429095656.GA4030@metamorpher.de>



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?

Best regards
// Patrik

^ permalink raw reply

* Re: Recover array after I panicked
From: Brad Campbell @ 2017-05-02 13:11 UTC (permalink / raw)
  To: Patrik Dahlström, Andreas Klauer; +Cc: linux-raid
In-Reply-To: <37245817-43d3-7c93-98fe-8b8ce54396ad@powerlamerz.org>

On 02/05/17 21: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?
>
> Best regards
> // Patrik
>

Yes please! And maybe an extra warning about getting full --detail and 
--examine information *before* making any alterations to an array!

Regards,
-- 
Dolphins are so intelligent that within a few weeks they can
train Americans to stand at the edge of the pool and throw them
fish.


^ permalink raw reply

* Re: [mdadm PATCH] Create: tell udev md device is not ready when first created.
From: Jes Sorensen @ 2017-05-02 13:32 UTC (permalink / raw)
  To: Peter Rajnoha, NeilBrown; +Cc: linux-raid, dm-devel
In-Reply-To: <6bd347e1-170c-48e9-1171-09ff143ace83@redhat.com>

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.

Jes


^ 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-02 13:40 UTC (permalink / raw)
  To: Peter Rajnoha, NeilBrown; +Cc: linux-raid, dm-devel
In-Reply-To: <65882238-57f0-dcef-645b-0ab1ab9cdc91@redhat.com>

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.

> 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.....

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.

Jes


^ permalink raw reply

* Re: [mdadm PATCH] Create: tell udev md device is not ready when first created.
From: Jes Sorensen @ 2017-05-02 13:42 UTC (permalink / raw)
  To: NeilBrown; +Cc: Peter Rajnoha, linux-raid, dm-devel
In-Reply-To: <87h919ruj5.fsf@notabene.neil.brown.name>

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!

I like this solution much better, even though I am not in love with the 
giant usleep() call. Would be nice to find a better way around that.

Sorry it took so long to get back to you on this, last week was a mess.

Thanks,
Jes



^ permalink raw reply

* Re: [PATCH] change back 0644 permission for Grow.c
From: Jes Sorensen @ 2017-05-02 13:45 UTC (permalink / raw)
  To: Zhilong Liu; +Cc: linux-raid
In-Reply-To: <1493717233-32524-1-git-send-email-zlliu@suse.com>

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 :)

Cheers,
Jes


^ permalink raw reply

* Re: [mdadm PATCH 1/2] util: unify fstat operations into function
From: Jes Sorensen @ 2017-05-02 13:50 UTC (permalink / raw)
  To: Zhilong Liu; +Cc: linux-raid
In-Reply-To: <20170502032646.20220-2-zlliu@suse.com>

On 05/01/2017 11:26 PM, Zhilong Liu wrote:
> declare new function to integrate repeated fstat operation,
> the fd and devname are necessary arguments, the dev_t *rdev
> is optional. according to parse the pointer of dev_t *rdev,
> if valid, assigned device number to rdev, if NULL, ignores.
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
>  Assemble.c    | 17 +++++------------
>  Build.c       |  5 +++--
>  Create.c      | 23 ++++++++++-------------
>  Grow.c        | 10 ++++------
>  Incremental.c | 33 ++++++++++++---------------------
>  Manage.c      |  2 +-
>  bitmap.c      | 10 +---------
>  mdadm.h       |  1 +
>  super-intel.c | 13 +++----------
>  util.c        | 17 +++++++++++++++++
>  10 files changed, 57 insertions(+), 74 deletions(-)
>
[snip]
> diff --git a/util.c b/util.c
> index 21a63c9..6294fff 100644
> --- a/util.c
> +++ b/util.c
> @@ -706,6 +706,23 @@ int check_raid(int fd, char *name)
>  	return 1;
>  }
>
> +int fstat_md_is_blkdev(int fd, char *devname, dev_t *rdev)
> +{
> +	struct stat stb;
> +
> +	if (fstat(fd, &stb) != 0) {
> +		pr_err("fstat failed for %s: %s\n", devname, strerror(errno));
> +		return 1;
> +	}
> +	if ((S_IFMT & stb.st_mode) != S_IFBLK) {
> +		pr_err("%s is not a block device.\n", devname);
> +		return 1;
> +	}
> +	if (rdev)
> +		*rdev = stb.st_rdev;
> +	return 0;
> +}
> +
>  int ask(char *mesg)
>  {
>  	char *add = "";
>

Hi Zhilong,

I like this approach better, however, I believe the return logic in the 
function is wrong. When the function is named fstat_md_is_blkdev() it 
should return 'true/1' when the device is a blk device, and 'false/0' 
when it is not.

I also think it should either be named md_fstat_is_blkdev() or 
fstat_is_blkdev() but that is a minor detail.

Cheers,
Jes


^ permalink raw reply

* Re: [mdadm PATCH 2/2] util: unify stat operations into function
From: Jes Sorensen @ 2017-05-02 13:50 UTC (permalink / raw)
  To: Zhilong Liu; +Cc: linux-raid
In-Reply-To: <20170502032646.20220-3-zlliu@suse.com>

On 05/01/2017 11:26 PM, Zhilong Liu wrote:
> declare new function to integrate repeated stat operations,
> the devname is necessary and the *rdev is optional argument,
> parse the pointer of dev_t *rdev, if valid, assigned device
> number to rdev, if NULL, ignores.
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
>  Assemble.c    |  7 ++-----
>  Build.c       | 25 ++++---------------------
>  Incremental.c | 21 ++++-----------------
>  Manage.c      | 11 +----------
>  Monitor.c     | 16 ++++------------
>  mdadm.h       |  1 +
>  super-ddf.c   | 10 ++++------
>  super-intel.c | 10 ++++------
>  util.c        | 17 +++++++++++++++++
>  9 files changed, 41 insertions(+), 77 deletions(-)

Same comment as for the fstat() version applies here.

Cheers,
Jes



^ permalink raw reply

* mdadm/mdmon Name vs argv[0]
From: Jes Sorensen @ 2017-05-02 14:19 UTC (permalink / raw)
  To: pawel.baldysiak; +Cc: linux-raid

Hi Pawel,

In the below commit you changed the handling of how mdadm/mdmon handles 
printing the name of the running process.

However, if people for some reason decided to change the names of 
mdadm/mdmon this would no longer be correct.

Do you remember if there is any reason why you didn't use argv[0] for this?

commit d56dd607ba433d9334f0fb4114fe081742ae4361
Author: Pawel Baldysiak <pawel.baldysiak@intel.com>
Date:   Wed Feb 11 22:25:03 2015 +0100

     Change way of printing name of a process

Cheers,
Jes

^ permalink raw reply

* [PATCH 0/6] md: Fine-tuning for some function implementations
From: SF Markus Elfring @ 2017-05-02 14:23 UTC (permalink / raw)
  To: linux-raid, Neil Brown, Shaohua Li; +Cc: LKML, kernel-janitors

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()
  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

^ permalink raw reply

* [PATCH 1/6] md: Replace seven seq_printf() calls by seq_putc()
From: SF Markus Elfring @ 2017-05-02 14:25 UTC (permalink / raw)
  To: linux-raid, Neil Brown, Shaohua Li; +Cc: LKML, kernel-janitors
In-Reply-To: <f05bfc4e-7935-7e87-cc85-f0d9c5e3846a@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 May 2017 14:01:17 +0200

A few single characters should be put into a sequence.
Thus use the corresponding function "seq_putc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/md.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 82f798be964f..7f0a35ee192a 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -7498,7 +7498,7 @@ static void status_unused(struct seq_file *seq)
 	if (!i)
 		seq_printf(seq, "<none>");
 
-	seq_printf(seq, "\n");
+	seq_putc(seq, '\n');
 }
 
 static int status_resync(struct seq_file *seq, struct mddev *mddev)
@@ -7552,12 +7552,13 @@ static int status_resync(struct seq_file *seq, struct mddev *mddev)
 	per_milli = res;
 	{
 		int i, x = per_milli/50, y = 20-x;
-		seq_printf(seq, "[");
+
+		seq_putc(seq, '[');
 		for (i = 0; i < x; i++)
-			seq_printf(seq, "=");
-		seq_printf(seq, ">");
+			seq_putc(seq, '=');
+		seq_putc(seq, '>');
 		for (i = 0; i < y; i++)
-			seq_printf(seq, ".");
+			seq_putc(seq, '.');
 		seq_printf(seq, "] ");
 	}
 	seq_printf(seq, " %s =%3u.%u%% (%llu/%llu)",
@@ -7678,7 +7679,7 @@ static int md_seq_show(struct seq_file *seq, void *v)
 			seq_printf(seq, "[%s] ", pers->name);
 
 		spin_unlock(&pers_lock);
-		seq_printf(seq, "\n");
+		seq_putc(seq, '\n');
 		seq->poll_event = atomic_read(&md_event_count);
 		return 0;
 	}
@@ -7754,8 +7755,7 @@ static int md_seq_show(struct seq_file *seq, void *v)
 			seq_printf(seq, "\n       ");
 
 		bitmap_status(seq, mddev->bitmap);
-
-		seq_printf(seq, "\n");
+		seq_putc(seq, '\n');
 	}
 	spin_unlock(&mddev->lock);
 
-- 
2.12.2

^ permalink raw reply related

* [PATCH 2/6] md: Replace 17 seq_printf() calls by seq_puts()
From: SF Markus Elfring @ 2017-05-02 14:26 UTC (permalink / raw)
  To: linux-raid, Neil Brown, Shaohua Li; +Cc: LKML, kernel-janitors
In-Reply-To: <f05bfc4e-7935-7e87-cc85-f0d9c5e3846a@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 May 2017 14:22:45 +0200

Strings which did not contain data format specifications should be put
into a sequence. Thus use the corresponding function "seq_puts".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/md.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 7f0a35ee192a..65233a91e5e2 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -7487,7 +7487,7 @@ static void status_unused(struct seq_file *seq)
 	int i = 0;
 	struct md_rdev *rdev;
 
-	seq_printf(seq, "unused devices: ");
+	seq_puts(seq, "unused devices: ");
 
 	list_for_each_entry(rdev, &pending_raid_disks, same_set) {
 		char b[BDEVNAME_SIZE];
@@ -7496,7 +7496,7 @@ static void status_unused(struct seq_file *seq)
 			      bdevname(rdev->bdev,b));
 	}
 	if (!i)
-		seq_printf(seq, "<none>");
+		seq_puts(seq, "<none>");
 
 	seq_putc(seq, '\n');
 }
@@ -7525,13 +7525,13 @@ static int status_resync(struct seq_file *seq, struct mddev *mddev)
 
 	if (resync == 0) {
 		if (mddev->recovery_cp < MaxSector) {
-			seq_printf(seq, "\tresync=PENDING");
+			seq_puts(seq, "\tresync=PENDING");
 			return 1;
 		}
 		return 0;
 	}
 	if (resync < 3) {
-		seq_printf(seq, "\tresync=DELAYED");
+		seq_puts(seq, "\tresync=DELAYED");
 		return 1;
 	}
 
@@ -7559,7 +7559,7 @@ static int status_resync(struct seq_file *seq, struct mddev *mddev)
 		seq_putc(seq, '>');
 		for (i = 0; i < y; i++)
 			seq_putc(seq, '.');
-		seq_printf(seq, "] ");
+		seq_puts(seq, "] ");
 	}
 	seq_printf(seq, " %s =%3u.%u%% (%llu/%llu)",
 		   (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery)?
@@ -7673,7 +7673,8 @@ static int md_seq_show(struct seq_file *seq, void *v)
 
 	if (v == (void*)1) {
 		struct md_personality *pers;
-		seq_printf(seq, "Personalities : ");
+
+		seq_puts(seq, "Personalities : ");
 		spin_lock(&pers_lock);
 		list_for_each_entry(pers, &pers_list, list)
 			seq_printf(seq, "[%s] ", pers->name);
@@ -7694,9 +7695,9 @@ static int md_seq_show(struct seq_file *seq, void *v)
 						mddev->pers ? "" : "in");
 		if (mddev->pers) {
 			if (mddev->ro==1)
-				seq_printf(seq, " (read-only)");
+				seq_puts(seq, " (read-only)");
 			if (mddev->ro==2)
-				seq_printf(seq, " (auto-read-only)");
+				seq_puts(seq, " (auto-read-only)");
 			seq_printf(seq, " %s", mddev->pers->name);
 		}
 
@@ -7707,17 +7708,17 @@ static int md_seq_show(struct seq_file *seq, void *v)
 			seq_printf(seq, " %s[%d]",
 				bdevname(rdev->bdev,b), rdev->desc_nr);
 			if (test_bit(WriteMostly, &rdev->flags))
-				seq_printf(seq, "(W)");
+				seq_puts(seq, "(W)");
 			if (test_bit(Journal, &rdev->flags))
-				seq_printf(seq, "(J)");
+				seq_puts(seq, "(J)");
 			if (test_bit(Faulty, &rdev->flags)) {
-				seq_printf(seq, "(F)");
+				seq_puts(seq, "(F)");
 				continue;
 			}
 			if (rdev->raid_disk < 0)
-				seq_printf(seq, "(S)"); /* spare */
+				seq_puts(seq, "(S)"); /* spare */
 			if (test_bit(Replacement, &rdev->flags))
-				seq_printf(seq, "(R)");
+				seq_puts(seq, "(R)");
 			sectors += rdev->sectors;
 		}
 		rcu_read_unlock();
@@ -7742,17 +7743,18 @@ static int md_seq_show(struct seq_file *seq, void *v)
 			seq_printf(seq, " super external:%s",
 				   mddev->metadata_type);
 		else
-			seq_printf(seq, " super non-persistent");
+			seq_puts(seq, " super non-persistent");
 
 		if (mddev->pers) {
 			mddev->pers->status(seq, mddev);
-			seq_printf(seq, "\n      ");
+			seq_puts(seq, "\n      ");
 			if (mddev->pers->sync_request) {
 				if (status_resync(seq, mddev))
-					seq_printf(seq, "\n      ");
+					seq_puts(seq, "\n      ");
 			}
-		} else
-			seq_printf(seq, "\n       ");
+		} else {
+			seq_puts(seq, "\n       ");
+		}
 
 		bitmap_status(seq, mddev->bitmap);
 		seq_putc(seq, '\n');
-- 
2.12.2


^ permalink raw reply related

* [PATCH 3/6] md: Adjust four function calls together with a variable assignment
From: SF Markus Elfring @ 2017-05-02 14:27 UTC (permalink / raw)
  To: linux-raid, Neil Brown, Shaohua Li; +Cc: LKML, kernel-janitors
In-Reply-To: <f05bfc4e-7935-7e87-cc85-f0d9c5e3846a@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 May 2017 15:07:21 +0200

The script "checkpatch.pl" pointed information out like the following.

ERROR: do not use assignment in if condition

Thus fix affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/md.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 65233a91e5e2..60580095a5de 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2153,7 +2153,8 @@ static int bind_rdev_to_array(struct md_rdev *rdev, struct mddev *mddev)
 	rdev->mddev = mddev;
 	pr_debug("md: bind<%s>\n", b);
 
-	if ((err = kobject_add(&rdev->kobj, &mddev->kobj, "dev-%s", b)))
+	err = kobject_add(&rdev->kobj, &mddev->kobj, "dev-%s", b);
+	if (err)
 		goto fail;
 
 	ko = &part_to_dev(rdev->bdev->bd_part)->kobj;
@@ -7306,7 +7307,8 @@ static int md_open(struct block_device *bdev, fmode_t mode)
 	}
 	BUG_ON(mddev != bdev->bd_disk->private_data);
 
-	if ((err = mutex_lock_interruptible(&mddev->open_mutex)))
+	err = mutex_lock_interruptible(&mddev->open_mutex);
+	if (err)
 		goto out;
 
 	if (test_bit(MD_CLOSING, &mddev->flags)) {
@@ -8927,10 +8929,12 @@ static int __init md_init(void)
 	if (!md_misc_wq)
 		goto err_misc_wq;
 
-	if ((ret = register_blkdev(MD_MAJOR, "md")) < 0)
+	ret = register_blkdev(MD_MAJOR, "md");
+	if (ret < 0)
 		goto err_md;
 
-	if ((ret = register_blkdev(0, "mdp")) < 0)
+	ret = register_blkdev(0, "mdp");
+	if (ret < 0)
 		goto err_mdp;
 	mdp_major = ret;
 
-- 
2.12.2

^ permalink raw reply related

* [PATCH 4/6] md: Use seq_puts() in faulty_status()
From: SF Markus Elfring @ 2017-05-02 14:28 UTC (permalink / raw)
  To: linux-raid, Neil Brown, Shaohua Li; +Cc: LKML, kernel-janitors
In-Reply-To: <f05bfc4e-7935-7e87-cc85-f0d9c5e3846a@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 May 2017 15:20:30 +0200

A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/faulty.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/faulty.c b/drivers/md/faulty.c
index b0536cfd8e17..f5536c91be5c 100644
--- a/drivers/md/faulty.c
+++ b/drivers/md/faulty.c
@@ -253,7 +253,7 @@ static void faulty_status(struct seq_file *seq, struct mddev *mddev)
 			   n, conf->period[ReadFixable]);
 
 	if ((n=atomic_read(&conf->counters[WriteAll])) != 0)
-		seq_printf(seq, " WriteAll");
+		seq_puts(seq, " WriteAll");
 
 	seq_printf(seq, " nfaults=%d", conf->nfaults);
 }
-- 
2.12.2

^ permalink raw reply related

* [PATCH 5/6] md: Adjust six function calls together with a variable assignment in faulty_status()
From: SF Markus Elfring @ 2017-05-02 14:29 UTC (permalink / raw)
  To: linux-raid, Neil Brown, Shaohua Li; +Cc: LKML, kernel-janitors
In-Reply-To: <f05bfc4e-7935-7e87-cc85-f0d9c5e3846a@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 May 2017 15:35:35 +0200

The script "checkpatch.pl" pointed information out like the following.

ERROR: do not use assignment in if condition

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/faulty.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/md/faulty.c b/drivers/md/faulty.c
index f5536c91be5c..2573009b1265 100644
--- a/drivers/md/faulty.c
+++ b/drivers/md/faulty.c
@@ -231,28 +231,33 @@ static void faulty_status(struct seq_file *seq, struct mddev *mddev)
 	struct faulty_conf *conf = mddev->private;
 	int n;
 
-	if ((n=atomic_read(&conf->counters[WriteTransient])) != 0)
+	n = atomic_read(&conf->counters[WriteTransient]);
+	if (n != 0)
 		seq_printf(seq, " WriteTransient=%d(%d)",
 			   n, conf->period[WriteTransient]);
 
-	if ((n=atomic_read(&conf->counters[ReadTransient])) != 0)
+	n = atomic_read(&conf->counters[ReadTransient]);
+	if (n != 0)
 		seq_printf(seq, " ReadTransient=%d(%d)",
 			   n, conf->period[ReadTransient]);
 
-	if ((n=atomic_read(&conf->counters[WritePersistent])) != 0)
+	n = atomic_read(&conf->counters[WritePersistent]);
+	if (n != 0)
 		seq_printf(seq, " WritePersistent=%d(%d)",
 			   n, conf->period[WritePersistent]);
 
-	if ((n=atomic_read(&conf->counters[ReadPersistent])) != 0)
+	n = atomic_read(&conf->counters[ReadPersistent]);
+	if (n != 0)
 		seq_printf(seq, " ReadPersistent=%d(%d)",
 			   n, conf->period[ReadPersistent]);
 
-
-	if ((n=atomic_read(&conf->counters[ReadFixable])) != 0)
+	n = atomic_read(&conf->counters[ReadFixable]);
+	if (n != 0)
 		seq_printf(seq, " ReadFixable=%d(%d)",
 			   n, conf->period[ReadFixable]);
 
-	if ((n=atomic_read(&conf->counters[WriteAll])) != 0)
+	n = atomic_read(&conf->counters[WriteAll]);
+	if (n != 0)
 		seq_puts(seq, " WriteAll");
 
 	seq_printf(seq, " nfaults=%d", conf->nfaults);
-- 
2.12.2

^ permalink raw reply related

* [PATCH 6/6] md: Add some spaces for better code readability
From: SF Markus Elfring @ 2017-05-02 14:31 UTC (permalink / raw)
  To: linux-raid, Neil Brown, Shaohua Li; +Cc: LKML, kernel-janitors
In-Reply-To: <f05bfc4e-7935-7e87-cc85-f0d9c5e3846a@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 May 2017 16:00:45 +0200

Use space characters and blank lines at some source code places
according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/faulty.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/md/faulty.c b/drivers/md/faulty.c
index 2573009b1265..434c3643c9c3 100644
--- a/drivers/md/faulty.c
+++ b/drivers/md/faulty.c
@@ -110,19 +110,20 @@ static int check_sector(struct faulty_conf *conf, sector_t start, sector_t end,
 {
 	/* If we find a ReadFixable sector, we fix it ... */
 	int i;
-	for (i=0; i<conf->nfaults; i++)
+
+	for (i = 0; i < conf->nfaults; i++)
 		if (conf->faults[i] >= start &&
 		    conf->faults[i] < end) {
 			/* found it ... */
 			switch (conf->modes[i] * 2 + dir) {
-			case WritePersistent*2+WRITE: return 1;
-			case ReadPersistent*2+READ: return 1;
-			case ReadFixable*2+READ: return 1;
-			case ReadFixable*2+WRITE:
+			case WritePersistent * 2 + WRITE: return 1;
+			case ReadPersistent * 2 + READ: return 1;
+			case ReadFixable * 2 + READ: return 1;
+			case ReadFixable * 2 + WRITE:
 				conf->modes[i] = NoPersist;
 				return 0;
-			case AllPersist*2+READ:
-			case AllPersist*2+WRITE: return 1;
+			case AllPersist * 2 + READ:
+			case AllPersist * 2 + WRITE: return 1;
 			default:
 				return 0;
 			}
@@ -134,9 +135,10 @@ static void add_sector(struct faulty_conf *conf, sector_t start, int mode)
 {
 	int i;
 	int n = conf->nfaults;
-	for (i=0; i<conf->nfaults; i++)
+
+	for (i = 0; i < conf->nfaults; i++)
 		if (conf->faults[i] == start) {
-			switch(mode) {
+			switch (mode) {
 			case NoPersist: conf->modes[i] = mode; return;
 			case WritePersistent:
 				if (conf->modes[i] == ReadPersistent ||
@@ -167,7 +169,7 @@ static void add_sector(struct faulty_conf *conf, sector_t start, int mode)
 	conf->faults[n] = start;
 	conf->modes[n] = mode;
 	if (conf->nfaults == n)
-		conf->nfaults = n+1;
+		conf->nfaults = n + 1;
 }
 
 static void faulty_make_request(struct mddev *mddev, struct bio *bio)
@@ -278,7 +280,8 @@ static int faulty_reshape(struct mddev *mddev)
 		conf->nfaults = 0;
 	else if (mode == ClearErrors) {
 		int i;
-		for (i=0 ; i < Modes ; i++) {
+
+		for (i = 0; i < Modes; i++) {
 			conf->period[i] = 0;
 			atomic_set(&conf->counters[i], 0);
 		}
@@ -317,7 +320,7 @@ static int faulty_run(struct mddev *mddev)
 	if (!conf)
 		return -ENOMEM;
 
-	for (i=0; i<Modes; i++) {
+	for (i = 0; i < Modes; i++) {
 		atomic_set(&conf->counters[i], 0);
 		conf->period[i] = 0;
 	}
-- 
2.12.2


^ permalink raw reply related

* Re: [PATCH] md/bitmap: use i_blocksize()
From: Geliang Tang @ 2017-05-02 14:33 UTC (permalink / raw)
  To: Shaohua Li; +Cc: linux-raid, linux-kernel, Coly Li
In-Reply-To: <20170122015030.d6sccz76asvvnvzz@OptiPlex>

On Sun, Jan 22, 2017 at 09:50:30AM +0800, Geliang Tang wrote:
> On Sat, Jan 21, 2017 at 10:13:07AM -0800, Shaohua Li wrote:
> > On Fri, Jan 20, 2017 at 10:29:52PM +0800, Geliang Tang wrote:
> > > Since i_blocksize() helper has been defined in fs.h, use it instead
> > > of open-coding.
> > 
> > which tree is this patch applied to? I can't find it in Linus's tree
> > 
> 
> This patch is against the last linux-next tree, next-20170120. It
> depends on the patch '2651ed7 fs: add i_blocksize()' which isn't merged
> into Linus's tree yet.
> 
> -Geliang

Hi Shaohua,

The i_blocksize() helper has been merged into Linus's tree already, and
this patch is still valid. Could you please reconsider it if this patch
can be applied?

Thanks.

-Geliang

^ permalink raw reply

* Re: [PATCH 1/1] IMSM: Initialize my_vol_raid_dev_num during vol creation
From: Jes Sorensen @ 2017-05-02 14:49 UTC (permalink / raw)
  To: Pawel Baldysiak; +Cc: linux-raid
In-Reply-To: <20170426090807.10328-1-pawel.baldysiak@intel.com>

On 04/26/2017 05:08 AM, Pawel Baldysiak wrote:
> This field was not initialized so far. This ID needs to be unique
> for every newly created array in container.
>
> Signed-off-by: Pawel Baldysiak <pawel.baldysiak@intel.com>
> ---
>  super-intel.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/super-intel.c b/super-intel.c
> index 0aed57c..d5c77c0 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -232,8 +232,13 @@ struct imsm_super {
>  	__u32 orig_family_num;		/* 0x40 - 0x43 original family num */
>  	__u32 pwr_cycle_count;		/* 0x44 - 0x47 simulated power cycle count for array */
>  	__u32 bbm_log_size;		/* 0x48 - 0x4B - size of bad Block Mgmt Log in bytes */
> -#define IMSM_FILLERS 35
> -	__u32 filler[IMSM_FILLERS];	/* 0x4C - 0xD7 RAID_MPB_FILLERS */
> +	__u16 num_raid_devs_created;	/* 0x4C - 0x4D Used for generating unique
> +					 * volume IDs for raid_dev created in this array
> +					 * (starts at 1)
> +					 */
> +	__u16 filler1;			/* 0x4E - 0x4F */
> +#define IMSM_FILLERS 34
> +	__u32 filler[IMSM_FILLERS];	/* 0x50 - 0xD7 RAID_MPB_FILLERS */
>  	struct imsm_disk disk[1];	/* 0xD8 diskTbl[numDisks] */
>  	/* here comes imsm_dev[num_raid_devs] */
>  	/* here comes BBM logs */
> @@ -5368,6 +5373,8 @@ static int init_super_imsm_volume(struct supertype *st, mdu_array_info_t *info,
>  		set_imsm_ord_tbl_ent(map, i, IMSM_ORD_REBUILD);
>  	}
>  	mpb->num_raid_devs++;
> +	mpb->num_raid_devs_created++;
> +	dev->my_vol_raid_dev_num = mpb->num_raid_devs_created;
>
>  	if (s->consistency_policy == UnSet ||
>  	    s->consistency_policy == CONSISTENCY_POLICY_RESYNC ||
>

Applied!

I didn't see this one as you got my email address wrong, but it's in my 
tree now.

Thanks,
Jes


^ permalink raw reply

* Re: MD raid microconference of Plumber Conf 2017
From: Jes Sorensen @ 2017-05-02 14:54 UTC (permalink / raw)
  To: Shaohua Li, Coly Li; +Cc: open list:SOFTWARE RAID (Multiple Disks) SUPPORT
In-Reply-To: <20170420163416.ia3bceztgbp2xsme@kernel.org>

On 04/20/2017 12:34 PM, Shaohua Li wrote:
> On Thu, Apr 20, 2017 at 10:57:26PM +0800, Coly Li wrote:
>> Hi MD developers,
>>
>> I just propose a microconference proposal to Linux Plumber Conf 2017,
>> the topic is "MD raid general discussion".
>>
>> The proposal wiki page can be found here:
>> http://wiki.linuxplumbersconf.org/2017:md_raid_general_discussion
>>
>> Please help to add more information to this wiki page (topics,
>> attendees, and other information). I do hope we can meet during LPC2017
>> and have face to face conversation, that will be great :-)
>
> Awesome, thanks Coly! I think I'll be there.

Thanks Coly!

I intend to be there too!

Jes



^ permalink raw reply

* Re: MD raid microconference of Plumber Conf 2017
From: Coly Li @ 2017-05-02 15:00 UTC (permalink / raw)
  To: Jes Sorensen, Shaohua Li; +Cc: open list:SOFTWARE RAID (Multiple Disks) SUPPORT
In-Reply-To: <87280db5-7081-a897-26f3-469a53d037e6@gmail.com>

On 2017/5/2 下午10:54, Jes Sorensen wrote:
> On 04/20/2017 12:34 PM, Shaohua Li wrote:
>> On Thu, Apr 20, 2017 at 10:57:26PM +0800, Coly Li wrote:
>>> Hi MD developers,
>>>
>>> I just propose a microconference proposal to Linux Plumber Conf 2017,
>>> the topic is "MD raid general discussion".
>>>
>>> The proposal wiki page can be found here:
>>> http://wiki.linuxplumbersconf.org/2017:md_raid_general_discussion
>>>
>>> Please help to add more information to this wiki page (topics,
>>> attendees, and other information). I do hope we can meet during LPC2017
>>> and have face to face conversation, that will be great :-)
>>
>> Awesome, thanks Coly! I think I'll be there.
> 
> Thanks Coly!
> 
> I intend to be there too!

Great news! Thank you all!

Coly


^ permalink raw reply

* [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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox