* Re: [RFC 01/10] crypto: factor async completion for general use
From: Gilad Ben-Yossef @ 2017-05-11 7:29 UTC (permalink / raw)
To: Eric Biggers
Cc: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, Ofir Drang, Gilad Ben-Yossef,
linux-crypto, linux-doc, Linux kernel mailing list, keyrings
In-Reply-To: <20170511035527.GA2936@zzz>
Hi Eric,
On Thu, May 11, 2017 at 6:55 AM, Eric Biggers <ebiggers3@gmail.com> wrote:
> Hi Gilad,
>
> On Sat, May 06, 2017 at 03:59:50PM +0300, Gilad Ben-Yossef wrote:
>> Invoking a possibly async. crypto op and waiting for completion
>> while correctly handling backlog processing is a common task
>> in the crypto API implementation and outside users of it.
>>
>> This patch re-factors one of the in crypto API implementation in
>> preparation for using it across the board instead of hand
>> rolled versions.
>
> Thanks for doing this! It annoyed me too that there wasn't a helper function
> for this. Just a few comments below:
>
Thank you for the review.
I will incorporate the feedback into v2.
...
> With regards to the wait being uninterruptible, I agree that this should be the
> default behavior, because I think users waiting for specific crypto requests are
> generally not prepared to handle the wait actually being interrupted. After
> interruption the crypto operation will still proceed in the background, and it
> will use buffers which the caller has in many cases already freed. However, I'd
> suggest taking a close look at anything that was actually doing an interruptible
> wait before, to see whether it was a bug or intentional (or "doesn't matter").
>
> And yes there could always be a crypto_wait_req_interruptible() introduced if
> some users need it.
So this one was a bit of a shocker. I though the _interruptible use
sites seemed
wrong in the sense of being needless. However, after reading your feedback and
reviewing the code I'm pretty sure every single one of them (including
the one I've
added in dm-verity-target.c this merge window) are down right dangerous and
can cause random data corruption... so thanks for pointing this out!
I though of this patch set as a "make the code pretty" for 4.13 kind
of patch set.
Looks like it's a bug fix now, maybe even stable material.
Anyway, I'll roll a v2 and we'll see.
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply
* Re: [PATCH 16/16] fs: switch ->s_uuid to uuid_t
From: Christoph Hellwig @ 2017-05-11 7:58 UTC (permalink / raw)
To: Amir Goldstein
Cc: linux-xfs, linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Mimi Zohar, linux-kernel,
Steven Whitehouse, David Howells, linux-fsdevel, Andy Shevchenko,
Shaohua Li, Christoph Hellwig
In-Reply-To: <CAOQ4uxga2U6OS8N5MnBoytyWP8Fbf8qQCS1cnsPqjoby=Mxc1w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, May 10, 2017 at 10:08:25PM +0300, Amir Goldstein wrote:
> On Wed, May 10, 2017 at 9:02 PM, Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> wrote:
> > For some file systems we still memcpy into it, but in various places this
> > already allows us to use the proper uuid helpers. More to come..
> > Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
> > ---
>
> Series looks good!
>
> My main concern is that filesystems with char uuid[16] are here to stay,
> or maybe, will be converted slowly.
I'll take a stab to convert them ASAP, and add s_uuid support for you
to more file systems once this initial series is done.
>
> Considering this, perhaps we should have a helper/macro to copy from
> char[16] to *uuid_t and maybe BUILD_BUG_ON the size comparison?
> #define bytes_to_uuid(dst, src) ...
If the mix of proper types and u8 arrays was to stay that'd be the way
to go, but I'd rather fix things up in a merge window or two instead
of leaving us in a transitionary state forever.
^ permalink raw reply
* Re: [PATCH 03/16] uuid: rename uuid types
From: Christoph Hellwig @ 2017-05-11 7:59 UTC (permalink / raw)
To: Amir Goldstein
Cc: linux-xfs, linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Mimi Zohar, linux-kernel,
Steven Whitehouse, David Howells, linux-fsdevel, Andy Shevchenko,
Shaohua Li, Christoph Hellwig
In-Reply-To: <CAOQ4uxjSpCgM=RK5jXkuY5HmseQT2EKh_huntvK2ADKBuNqi4A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, May 10, 2017 at 10:15:51PM +0300, Amir Goldstein wrote:
> > +static inline int uuid_cmp(const uuid_t u1, const uuid_t u2)
> > {
> > - return memcmp(&u1, &u2, sizeof(uuid_be));
> > + return memcmp(&u1, &u2, sizeof(uuid_t));
> > }
> >
>
>
> I think we should use this opportunity to change these to (uuid_t *, uuid_t *)
> which is more in line with the rest of the helpers including the new helpers
> hoisted from xfs.
Or just kill them off - uuid/guid_equals is the better interface for all
existing users as far as I can tell. My updated series already kills
off the be/uuid one, the le/guid one will need a bit more work, though.
^ permalink raw reply
* Re: [RFC 01/10] crypto: factor async completion for general use
From: Eric Biggers @ 2017-05-11 8:09 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, Ofir Drang, Gilad Ben-Yossef,
linux-crypto, linux-doc, Linux kernel mailing list, keyrings
In-Reply-To: <CAOtvUMezWBrHtGxJCBJHVks-XJdSCOMrRj3evsNdY=cB==wc-g@mail.gmail.com>
On Thu, May 11, 2017 at 10:29:47AM +0300, Gilad Ben-Yossef wrote:
> > With regards to the wait being uninterruptible, I agree that this should be the
> > default behavior, because I think users waiting for specific crypto requests are
> > generally not prepared to handle the wait actually being interrupted. After
> > interruption the crypto operation will still proceed in the background, and it
> > will use buffers which the caller has in many cases already freed. However, I'd
> > suggest taking a close look at anything that was actually doing an interruptible
> > wait before, to see whether it was a bug or intentional (or "doesn't matter").
> >
> > And yes there could always be a crypto_wait_req_interruptible() introduced if
> > some users need it.
>
> So this one was a bit of a shocker. I though the _interruptible use
> sites seemed
> wrong in the sense of being needless. However, after reading your feedback and
> reviewing the code I'm pretty sure every single one of them (including
> the one I've
> added in dm-verity-target.c this merge window) are down right dangerous and
> can cause random data corruption... so thanks for pointing this out!
>
> I though of this patch set as a "make the code pretty" for 4.13 kind
> of patch set.
> Looks like it's a bug fix now, maybe even stable material.
>
> Anyway, I'll roll a v2 and we'll see.
>
Any that are called only by kernel threads would theoretically be safe since
kernel threads don't ordinarily receive signals. But I think that at least the
drbg and gcm waits can be reached by user threads, since they can be called via
algif_rng and algif_aead respectively.
I recommend putting any important fixes first, so they can be backported without
depending on crypto_wait_req().
Eric
^ permalink raw reply
* Re: [PATCH 03/16] uuid: rename uuid types
From: Amir Goldstein @ 2017-05-11 8:31 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-xfs, linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Mimi Zohar, linux-kernel,
David Howells, linux-fsdevel, Andy Shevchenko, Shaohua Li,
Steven Whitehouse
In-Reply-To: <20170511075926.GC19837-jcswGhMUV9g@public.gmane.org>
On Thu, May 11, 2017 at 10:59 AM, Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> wrote:
> On Wed, May 10, 2017 at 10:15:51PM +0300, Amir Goldstein wrote:
>> > +static inline int uuid_cmp(const uuid_t u1, const uuid_t u2)
>> > {
>> > - return memcmp(&u1, &u2, sizeof(uuid_be));
>> > + return memcmp(&u1, &u2, sizeof(uuid_t));
>> > }
>> >
>>
>>
>> I think we should use this opportunity to change these to (uuid_t *, uuid_t *)
>> which is more in line with the rest of the helpers including the new helpers
>> hoisted from xfs.
>
> Or just kill them off - uuid/guid_equals is the better interface for all
> existing users as far as I can tell. My updated series already kills
> off the be/uuid one, the le/guid one will need a bit more work, though.
That works too. and for another argument, why need a helper uuid/guid_cmp
when one can use C comparison (i.e. u1 != u2) ? assuming that we have no
reason to abstract that fact that uuid_t/guid_t are a struct type.
^ permalink raw reply
* [mdadm PATCH] retire the APIs that driver no longer supports
From: Zhilong Liu @ 2017-05-11 8:52 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
refer to commit: e6e5f8f1267d ("Build: Stop
bothering about supporting md driver ...")
continue to retire the APIs that md driver
wasn't supported for very long period of time.
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
Build.c | 4 ----
Manage.c | 4 ----
2 files changed, 8 deletions(-)
diff --git a/Build.c b/Build.c
index ad59867..70ba068 100644
--- a/Build.c
+++ b/Build.c
@@ -24,10 +24,6 @@
#include "mdadm.h"
-#define REGISTER_DEV _IO (MD_MAJOR, 1)
-#define START_MD _IO (MD_MAJOR, 2)
-#define STOP_MD _IO (MD_MAJOR, 3)
-
int Build(char *mddev, struct mddev_dev *devlist,
struct shape *s, struct context *c)
{
diff --git a/Manage.c b/Manage.c
index 14276b7..467efb7 100644
--- a/Manage.c
+++ b/Manage.c
@@ -27,10 +27,6 @@
#include "md_p.h"
#include <ctype.h>
-#define REGISTER_DEV _IO (MD_MAJOR, 1)
-#define START_MD _IO (MD_MAJOR, 2)
-#define STOP_MD _IO (MD_MAJOR, 3)
-
int Manage_ro(char *devname, int fd, int readonly)
{
/* switch to readonly or rw
--
2.10.2
^ permalink raw reply related
* Re: [RFC 01/10] crypto: factor async completion for general use
From: Gilad Ben-Yossef @ 2017-05-11 8:55 UTC (permalink / raw)
To: Eric Biggers
Cc: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, Ofir Drang, Gilad Ben-Yossef,
linux-crypto, linux-doc, Linux kernel mailing list, keyrings
In-Reply-To: <20170511080921.GB7533@zzz>
On Thu, May 11, 2017 at 11:09 AM, Eric Biggers <ebiggers3@gmail.com> wrote:
> On Thu, May 11, 2017 at 10:29:47AM +0300, Gilad Ben-Yossef wrote:
>> > With regards to the wait being uninterruptible, I agree that this should be the
>> > default behavior, because I think users waiting for specific crypto requests are
>> > generally not prepared to handle the wait actually being interrupted. After
>> > interruption the crypto operation will still proceed in the background, and it
>> > will use buffers which the caller has in many cases already freed. However, I'd
>> > suggest taking a close look at anything that was actually doing an interruptible
>> > wait before, to see whether it was a bug or intentional (or "doesn't matter").
>> >
>> > And yes there could always be a crypto_wait_req_interruptible() introduced if
>> > some users need it.
>>
>> So this one was a bit of a shocker. I though the _interruptible use
>> sites seemed
>> wrong in the sense of being needless. However, after reading your feedback and
>> reviewing the code I'm pretty sure every single one of them (including
>> the one I've
>> added in dm-verity-target.c this merge window) are down right dangerous and
>> can cause random data corruption... so thanks for pointing this out!
>>
>> I though of this patch set as a "make the code pretty" for 4.13 kind
>> of patch set.
>> Looks like it's a bug fix now, maybe even stable material.
>>
>> Anyway, I'll roll a v2 and we'll see.
>>
>
> Any that are called only by kernel threads would theoretically be safe since
> kernel threads don't ordinarily receive signals. But I think that at least the
> drbg and gcm waits can be reached by user threads, since they can be called via
> algif_rng and algif_aead respectively.
>
> I recommend putting any important fixes first, so they can be backported without
> depending on crypto_wait_req().
>
OK, I'll send out a separate bug fix series first and rebase the
crypto_wait one on top
of it then.
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply
* Building mdadm in parallel fails
From: Paul Menzel @ 2017-05-11 9:05 UTC (permalink / raw)
To: linux-raid
Dear mdadm folks,
I hope this list is the correct forum to report this issue to.
Building the utility mdadm 4.0 with in parallel with 62 cores, gives the
error below. Building with `-j1` works.
```
$ make -j62 PREFIX=/usr EPREFIX=/usr BINDIR=/usr/bin SBINDIR=/usr/sbin
LIBEXECDIR=/usr/libexec SYSCONFDIR=/etc SHAREDSTATEDIR=/var
LOCALSTATEDIR=/var LIBDIR=/usr/lib INCLUDEDIR=/usr/include
DATAROOTDIR=/usr/share DATADIR=/usr/share INFODIR=/usr/share/info
LOCALEDIR=/usr/share/locale MANDIR=/usr/share/man
DOCDIR=/usr/share/doc/mdadm V=1 everything[…]
cc -o mdadm.Os -Wall -Werror -Wstrict-prototypes -Wextra
-Wno-unused-parameter -ggdb -DSendmail=\""/usr/sbin/sendmail -t"\"
-DCONFFILE=\"/etc/mdadm.conf\" -DCONFFILE2=\"/etc/mdadm/mdadm.conf\"
-DMAP_DIR=\"/run/mdadm\" -DMAP_FILE=\"map\" -DMDMON_DIR=\"/run/mdadm\"
-DFAILED_SLOTS_DIR=\"/run/mdadm/failed-slots\" -DNO_COROSYNC -DNO_DLM
-DUSE_PTHREADS -DBINDIR=\"/usr/bin\" -DHAVE_STDINT_H -Os mdadm.c
config.c policy.c mdstat.c ReadMe.c util.c maps.c lib.c Manage.c
Assemble.c Build.c Create.c Detail.c Examine.c Grow.c Monitor.c dlink.c
Kill.c Query.c Incremental.c Dump.c mdopen.c super0.c super1.c
super-ddf.c super-intel.c bitmap.c super-mbr.c super-gpt.c restripe.c
sysfs.c sha1.c mapfile.c crc32.c sg_io.c msg.c xmalloc.c
platform-intel.c probe_roms.c crc32c.c -ldl
> cc -o mdmon.O2 -Wall -Werror -Wstrict-prototypes -Wextra -Wno-unused-parameter -ggdb -DSendmail=\""/usr/sbin/sendmail -t"\" -DCONFFILE=\"/etc/mdadm.conf\" -DCONFFILE2=\"/etc/mdadm/mdadm.conf\" -DMAP_DIR=\"/run/mdadm\" -DMAP_FILE=\"map\" -DMDMON_DIR=\"/run/mdadm\" -DFAILED_SLOTS_DIR=\"/run/mdadm/failed-slots\" -DNO_COROSYNC -DNO_DLM -DUSE_PTHREADS -DBINDIR=\"/usr/bin\" -pthread -DHAVE_STDINT_H -O2 -D_FORTIFY_SOURCE=2 mdmon.c monitor.c managemon.c util.c maps.c mdstat.c sysfs.c policy.c lib.c Kill.c sg_io.c dlink.c ReadMe.c super-intel.c super-mbr.c super-gpt.c super-ddf.c sha1.c crc32.c msg.c bitmap.c xmalloc.c platform-intel.c probe_roms.c -ldl
sed -e 's/{DEFAULT_METADATA}/1.2/g' \
-e 's,{MAP_PATH},/run/mdadm/map,g' mdadm.8.in > mdadm.8
man -l md.4 > md.man
man -l mdadm.conf.5 > mdadm.conf.man
man -l mdmon.8 > mdmon.man
man -l raid6check.8 > raid6check.man
man -l mdadm.8 > mdadm.man
rm -f mdadm.o config.o policy.o mdstat.o ReadMe.o util.o maps.o lib.o
Manage.o Assemble.o Build.o Create.o Detail.o Examine.o Grow.o Monitor.o
dlink.o Kill.o Query.o Incremental.o Dump.o mdopen.o super0.o super1.o
super-ddf.o super-intel.o bitmap.o super-mbr.o super-gpt.o restripe.o
sysfs.o sha1.o mapfile.o crc32.o sg_io.o msg.o xmalloc.o
platform-intel.o probe_roms.o crc32c.o
> cc -DBINDIR=\"/usr/bin\" -Wall -Werror -Wstrict-prototypes -Wextra -Wno-unused-parameter -ggdb -DSendmail=\""/usr/sbin/sendmail -t"\" -DCONFFILE=\"/etc/mdadm.conf\" -DCONFFILE2=\"/etc/mdadm/mdadm.conf\" -DMAP_DIR=\"/run/mdadm\" -DMAP_FILE=\"map\" -DMDMON_DIR=\"/run/mdadm\" -DFAILED_SLOTS_DIR=\"/run/mdadm/failed-slots\" -DNO_COROSYNC -DNO_DLM -DUSE_PTHREADS -DMDASSEMBLE -DMDASSEMBLE_AUTO -static -DHAVE_STDINT_H -o mdassemble.static mdassemble.c Assemble.c Manage.c config.c policy.c dlink.c util.c maps.c lib.c xmalloc.c super0.c super1.c super-ddf.c super-intel.c sha1.c crc32.c sg_io.c mdstat.c platform-intel.c probe_roms.c sysfs.c super-mbr.c super-gpt.c mapfile.c mdopen.c pwgr.c
cc -ggdb -o raid6check raid6check.o restripe.o sysfs.o maps.o lib.o
xmalloc.o dlink.o
cc: error: restripe.o: No such file or directory
cc: error: sysfs.o: No such file or directory
cc: error: maps.o: No such file or directory
cc: error: lib.o: No such file or directory
cc: error: xmalloc.o: No such file or directory
cc: error: dlink.o: No such file or directory
Makefile:217: recipe for target 'raid6check' failed
make: *** [raid6check] Error 1
make: *** Waiting for unfinished jobs....
/usr/bin/ld: cannot find xmalloc.o: No such file or directory
collect2: error: ld returned 1 exit status
Makefile:214: recipe for target 'test_stripe' failed
make: *** [test_stripe] Error 1
make[1]: Leaving directory '/dev/shm/bee-joey/mdadm/mdadm-4.0-0/source'
mv mdassemble.static mdassemble.auto
```
Kind regards,
Paul
^ permalink raw reply
* Re: [PATCH 03/16] uuid: rename uuid types
From: Andy Shevchenko @ 2017-05-11 11:38 UTC (permalink / raw)
To: David Howells, Christoph Hellwig
Cc: linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li, Amir Goldstein,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Mimi Zohar,
Steven Whitehouse
In-Reply-To: <2612.1494440436-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
On Wed, 2017-05-10 at 19:20 +0100, David Howells wrote:
> Christoph Hellwig <hch@lst.de> wrote:
>
> > -#define NULL_UUID_LE
> > \
> > - UUID_LE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,
> > \
> > - 0x00, 0x00, 0x00, 0x00)
> > +#define NULL_GUID
> > \
> > + GUID(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,
> > \
> > + 0x00, 0x00, 0x00, 0x00)
> >
> > -#define NULL_UUID_BE
> > \
> > - UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,
> > \
> > - 0x00, 0x00, 0x00, 0x00)
> > +#define NULL_UUID
> > \
> > + UUID(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,
> > \
> > + 0x00, 0x00, 0x00, 0x00)
>
> These are UAPI and ought not to be renamed.
>
Good point. I see in the branch it's already fixed.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply
* Re: [PATCH 05/16] uuid: add the v1 layout to uuid_t
From: Andy Shevchenko @ 2017-05-11 11:40 UTC (permalink / raw)
To: Christoph Hellwig, Amir Goldstein,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Mimi Zohar,
Steven Whitehouse
In-Reply-To: <20170510180214.16852-6-hch-jcswGhMUV9g@public.gmane.org>
On Wed, 2017-05-10 at 20:02 +0200, Christoph Hellwig wrote:
> Turn the content of uuid_t into a union and add the fields for the v1
> interpretation to it.
In the branch it has fix for UUID() wrt union change, but there missed
similar for GUID().
+#define UUID(a, _b, c, d0, d1, d2, d3, d4, d5, d6,
d7) \
+{ .b = { \
^^^ This should be done for GUID() as well.
While here, can we indent \ to be the same as of the rest lines in
macro(s) ?
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply
* Re: [PATCH] IMSM: Correct --examine output for 4k disks
From: Mariusz Dabrowski @ 2017-05-11 11:49 UTC (permalink / raw)
To: Jes Sorensen, linux-raid; +Cc: Maksymilian Kunt
In-Reply-To: <e27a148f-f0bf-8713-ceca-f37b4fb2fb88@gmail.com>
On 05/10/2017 05:18 PM, Jes Sorensen wrote:
> On 05/10/2017 04:43 AM, Mariusz Dabrowski wrote:
>> On 05/09/2017 06:04 PM, Jes Sorensen wrote:
>> > On 05/09/2017 08:03 AM, Mariusz Dabrowski wrote:
>> >> From: Maksymilian Kunt <maksymilian.kunt@intel.com>
>> >>
>> >> "Array Size" and "Per Dev Size" are incorrect for disks with sector size
>> >> different than 512B.
>> >>
>> >> Calculate "Array Size" and "Per Dev Size" based on sector size. Additionally
>> >> print "Sector Size".
>> >>
>> >> Signed-off-by: Maksymilian Kunt <maksymilian.kunt@intel.com>
>> >> Signed-off-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
>> >> ---
>> >> super-intel.c | 7 +++++--
>> >> 1 file changed, 5 insertions(+), 2 deletions(-)
>> >
>> > I am a little wary of this change as it changes the order of the output. Does
>> > anyone have tools parsing this information which could break from this?
>> >
>> > Jes
>> >
>>
>> Hi Jes,
>>
>> some time ago we have added "RWH Policy" field to --examine output and we
>> haven't heard any complaints about that change.
>> Also it is better to parse output from --export option which is unchanged in
>> this patch.
>
> Hi Mariusz,
>
> That is correct, however the policy stuff was added to the end of the output, it
> didn't change the existing part. I am a little mixed on this one, but I do agree
> --export should be the recommended use case for scripts.
>
> Cheers,
> Jes
Are you willing to accept this patch if no one disagrees or should I remove this
additional field and resend the patch?
^ permalink raw reply
* Re: [PATCH 05/16] uuid: add the v1 layout to uuid_t
From: Christoph Hellwig @ 2017-05-11 12:07 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Mimi Zohar, Amir Goldstein,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Steven Whitehouse,
David Howells, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Shaohua Li,
Christoph Hellwig
In-Reply-To: <1494502838.6967.4.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On Thu, May 11, 2017 at 02:40:38PM +0300, Andy Shevchenko wrote:
> On Wed, 2017-05-10 at 20:02 +0200, Christoph Hellwig wrote:
> > Turn the content of uuid_t into a union and add the fields for the v1
> > interpretation to it.
>
> In the branch it has fix for UUID() wrt union change, but there missed
> similar for GUID().
Actually UUID needed this because uuid_t has a unіon inside, GUID
should not need it because it doesn't.
> While here, can we indent \ to be the same as of the rest lines in
> macro(s) ?
Sure.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply
* Re: [mdadm PATCH] Create: move STOP_ARRAY to abort_locked
From: Zhilong Liu @ 2017-05-11 13:01 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
In-Reply-To: <8275d868-ad51-209b-e46f-9d7f1c60d1a3@gmail.com>
On 05/09/2017 01:54 AM, Jes Sorensen wrote:
> On 05/07/2017 09:50 PM, Zhilong Liu wrote:
>>
>>
>> On 05/05/2017 11:31 AM, Liu Zhilong wrote:
>>>
>>>
>>> On 05/04/2017 10:54 PM, Jes Sorensen wrote:
>>>> On 05/04/2017 08:20 AM, Zhilong Liu wrote:
>>>>> Hi Jes,
>>>>>
>>>>> apply for review, this is a bug I ever encountered.
>>>>
>>>> Zhilong,
>>>>
>>>> Under what circumstances do you see this?
>>>>
>>>
>>> Issued the command:
>>> linux-g0sr:/home/test # ./mdadm -CR /dev/md0 -l1 -n2 -b internal
>>> /dev/loop[0-1] --size 63
>>> ... ... ...
>>> mdadm: Given bitmap chunk size not supported.
>>> linux-g0sr:/home/test # ls /dev/md0
>>> /dev/md0
>>> linux-g0sr:/home/test # ls /sys/block/md0/md/
>>> array_size bitmap component_size level metadata_version
>>> raid_disks reshape_position safe_mode_delay
>>> array_state chunk_size layout max_read_errors
>>> new_dev reshape_direction resync_start
>>>
>>> create_mddev() writes the devnm to
>>> /sys/module/md_mod/parameter/new_array,
>>> then in md.c, module_param_call() called the 'set' function of
>>> add_named_array(),
>>> md_alloc() init_and_add the kobject for devm, finally the devnm device
>>> has created
>>> and sysfs has registered after create_mddev executed successfully.
>>> Thus it's better
>>> to STOP_ARRAY in any case after create_mddev() invoked.
>>>
>>
>> this patch depends on the kernel commit:
>> 039b7225e6e9 ("md: allow creation of mdNNN arrays via
>> md_mod/parameters/new_array")
>> Neil's patch has set "mddev->hold_active = UNTIL_STOP", thus the
>> STOP_ARRAY can work
>> well on this situation.
>
> OK now I am confused - are you saying this change will only work after
> Neil's kernel patch has been applied? That would be no good, mdadm
> needs to work on older kernels too.
>
Yes, I do agree your kindly reminding. In order to fix this scenario that
exit the process abnormally but create_mddev() has invoked, so how
about declaring one utility function to do STOP_ARRAY like following,
it's a better way to stop the array if it opened by O_EXCL flag.
cut the piece code from Manage_stop():
... ...
/* As we have an O_EXCL open, any use of the device
* which blocks STOP_ARRAY is probably a transient use,
* so it is reasonable to retry for a while - 5 seconds.
*/
count = 25; err = 0;
while (count && fd >= 0
&& (err = ioctl(fd, STOP_ARRAY, NULL)) < 0
&& errno == EBUSY) {
usleep(200000);
count --;
}
... ...
Thanks,
-Zhilong
> Jes
>
>
>
^ permalink raw reply
* Re: Network based (iSCSI) RAID1 setup
From: Gionatan Danti @ 2017-05-11 15:09 UTC (permalink / raw)
To: Adam Goryachev, linux-raid
In-Reply-To: <651c37fe-68b2-2dd7-a429-e62c994de60b@websitemanagers.com.au>
On 10/05/2017 14:08, Adam Goryachev wrote:
> It depends on your definition of production, but for me, the answer is
> no. Once upon a time, I used MD to do RAID1 between a local SSD and a
> remote device with NBD and that worked well, (apart from the fact I
> needed to manually re-add the remote device after a reboot, or whenever
> it dropped out for any other reason). It did save me when the local SSD
> died, and I was able to keep running purely from the remote NBD device
> until I could get in and replace the local SSD.
>
> Today, I use DRBD, and would much prefer that compared to MD + NBD.
Thanks for your feedback, Adam. I agree with your, for the reasons
expressed below. Hoping to be useful for others, I'll document here my
findings.
>> I'm using two CentOS 7.3 x86-64 boxes, with kernel version
>> 3.10.0-514.16.1.el7.x86_64 and mdadm v3.4 - 28th January 2016. Here
>> you can find my current RAID1 setup, where /dev/sdb is the iSCSI disk:
>>
>> So, second question: how to enable auto re-add for the remote device
>> when it become available again? For example:
>
> I don't know, but I guess you need to work out what udev rules are
> triggered when the iscsi device is "connected", and then get that to
> trigger the MD add rules. Possibly you could try to create a partition
> on the iscsi, and then use sdb1 for the RAID array, there might be
> better handling by udev in that case (I really don't know, just making
> random suggestions here).
I worked out how to enable auto re-add: the key was to include a default
"POLICY action=re-add" in /etc/mdadm.conf; at this point, any removed
disk will be re-attached automatically when it newly become visible.
With a catch: in iSCSI, when the remote disk become unresponsive and it
is dropped, it is *not* removed (by udev) from the disk entries found in
/dev. As both the remove and the auto-readd processes depends on udev
events triggering changes to the /dev directory (ie: a drive
disappearing and/or reappearing), rebooting the remote host will cause
the iSCSI-imported disk to be marked as failed, but not as removed (due
to its entries in /dev being *not* removed); later, when the iSCSI disk
become visible again, it is re-added as a spare.
So, while mdadm by itself worked quite well with networked disks, I
agree with Adam in that, for production workloads, this is a too fragile
setup. Specifically:
- the default iSCSI timeout (120 seconds) it way too high and need to be
adjusted;
- as, by default, mdadm scans all disk devices for valid arrays, *both*
the local and remote machines can see the md array. This must be avoided
with the using of the ARRAY <ignore> directive in the mdadm.conf file on
the remote machines (ie: the one which exports the iSCSI drives);
- udev is clearly not geared for managing events caused by remote disks
which suddenly disconnect without advice.
In the end, no problems seems related to mdadm itself, which remains a
wonderful and extremely flexible tool to manage software RAIDs. Thank
you all for the hard works.
Regards.
--
Danti Gionatan
Supporto Tecnico
Assyoma S.r.l. - www.assyoma.it
email: g.danti@assyoma.it - info@assyoma.it
GPG public key ID: FF5F32A8
^ permalink raw reply
* Re: [PATCH] IMSM: Correct --examine output for 4k disks
From: Jes Sorensen @ 2017-05-11 15:44 UTC (permalink / raw)
To: Mariusz Dabrowski, linux-raid; +Cc: Maksymilian Kunt
In-Reply-To: <ce37f19d-85f7-b360-4b8f-c54cd5a2915e@intel.com>
On 05/11/2017 07:49 AM, Mariusz Dabrowski wrote:
> On 05/10/2017 05:18 PM, Jes Sorensen wrote:
>> On 05/10/2017 04:43 AM, Mariusz Dabrowski wrote:
>>> On 05/09/2017 06:04 PM, Jes Sorensen wrote:
>>> > On 05/09/2017 08:03 AM, Mariusz Dabrowski wrote:
>>> >> From: Maksymilian Kunt <maksymilian.kunt@intel.com>
>>> >>
>>> >> "Array Size" and "Per Dev Size" are incorrect for disks with
>>> sector size
>>> >> different than 512B.
>>> >>
>>> >> Calculate "Array Size" and "Per Dev Size" based on sector size.
>>> Additionally
>>> >> print "Sector Size".
>>> >>
>>> >> Signed-off-by: Maksymilian Kunt <maksymilian.kunt@intel.com>
>>> >> Signed-off-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
>>> >> ---
>>> >> super-intel.c | 7 +++++--
>>> >> 1 file changed, 5 insertions(+), 2 deletions(-)
>>> >
>>> > I am a little wary of this change as it changes the order of the
>>> output. Does
>>> > anyone have tools parsing this information which could break from
>>> this?
>>> >
>>> > Jes
>>> >
>>>
>>> Hi Jes,
>>>
>>> some time ago we have added "RWH Policy" field to --examine output
>>> and we
>>> haven't heard any complaints about that change.
>>> Also it is better to parse output from --export option which is
>>> unchanged in
>>> this patch.
>>
>> Hi Mariusz,
>>
>> That is correct, however the policy stuff was added to the end of the
>> output, it
>> didn't change the existing part. I am a little mixed on this one, but
>> I do agree
>> --export should be the recommended use case for scripts.
>>
>> Cheers,
>> Jes
>
> Are you willing to accept this patch if no one disagrees or should I
> remove this additional field and resend the patch?
I have applied it - nobody has spoken out against it and -E is the
correct way to pull the info out.
Jes
^ permalink raw reply
* Re: on assembly and recovery of a hardware RAID
From: Alfred Matthews @ 2017-05-11 17:18 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <CAAZLhTf=GHmSDcY8Eb9SzevofFbDD7f-N8DKZiFkq9j2XV4mQA@mail.gmail.com>
I'm bumping this for lack of reply. I know you all are busy.
fsck.hfs /dev/md0 after build sends a core dump. Not clear to me what
I'm doing wrong. Thanks.
On 3 May 2017 at 15:44, Alfred Matthews <asm13243546@gmail.com> wrote:
> 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: on assembly and recovery of a hardware RAID
From: NeilBrown @ 2017-05-11 22:27 UTC (permalink / raw)
To: Alfred Matthews; +Cc: linux-raid
In-Reply-To: <CAAZLhTf=GHmSDcY8Eb9SzevofFbDD7f-N8DKZiFkq9j2XV4mQA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2533 bytes --]
On Wed, May 03 2017, Alfred Matthews wrote:
> 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.
I wonder if they are the same thing, or completely different.
>
> 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.
Definitely possible.
Given how many of the numbers listed above are 0xffff or 0xffffffff, it
is unlikely that the tool found anything useful.
>
>>>
>>> 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.
I don't think I can be of further help, sorry.
NeilBrown
>
> Al
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* [PATCH v5] md/r5cache: gracefully handle journal device errors for writeback mode
From: Song Liu @ 2017-05-11 22:28 UTC (permalink / raw)
To: linux-raid
Cc: shli, neilb, kernel-team, dan.j.williams, hch, jes.sorensen,
Song Liu
For the raid456 with writeback cache, when journal device failed during
normal operation, it is still possible to persist all data, as all
pending data is still in stripe cache. However, it is necessary to handle
journal failure gracefully.
During journal failures, the following logic handles the graceful shutdown
of journal:
1. raid5_error() marks the device as Faulty and schedules async work
log->disable_writeback_work;
2. In disable_writeback_work (r5c_disable_writeback_async), the mddev is
suspended, set to write through, and then resumed. mddev_suspend()
flushes all cached stripes;
3. All cached stripes need to be flushed carefully to the RAID array.
This patch fixes issues within the process above:
1. In r5c_update_on_rdev_error() schedule disable_writeback_work for
journal failures;
2. In r5c_disable_writeback_async(), wait for MD_SB_CHANGE_PENDING,
since raid5_error() updates superblock.
3. In handle_stripe(), allow stripes with data in journal (s.injournal > 0)
to make progress during log_failed;
4. In delay_towrite(), if log failed only process data in the cache (skip
new writes in dev->towrite);
5. In __get_priority_stripe(), process loprio_list during journal device
failures.
6. In raid5_remove_disk(), wait for all cached stripes are flushed before
calling log_exit().
Signed-off-by: Song Liu <songliubraving@fb.com>
---
drivers/md/raid5-cache.c | 11 +++++++++--
drivers/md/raid5-log.h | 3 ++-
drivers/md/raid5.c | 29 +++++++++++++++++++++++------
3 files changed, 34 insertions(+), 9 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index a6a62e2..cc3f844 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -24,6 +24,7 @@
#include "md.h"
#include "raid5.h"
#include "bitmap.h"
+#include "raid5-log.h"
/*
* metadata/data stored in disk with 4k size unit (a block) regardless
@@ -680,6 +681,11 @@ static void r5c_disable_writeback_async(struct work_struct *work)
return;
pr_info("md/raid:%s: Disabling writeback cache for degraded array.\n",
mdname(mddev));
+
+ /* wait superblock change before suspend */
+ wait_event(mddev->sb_wait,
+ !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags));
+
mddev_suspend(mddev);
log->r5c_journal_mode = R5C_JOURNAL_MODE_WRITE_THROUGH;
mddev_resume(mddev);
@@ -2983,7 +2989,7 @@ static int r5l_load_log(struct r5l_log *log)
return ret;
}
-void r5c_update_on_rdev_error(struct mddev *mddev)
+void r5c_update_on_rdev_error(struct mddev *mddev, struct md_rdev *rdev)
{
struct r5conf *conf = mddev->private;
struct r5l_log *log = conf->log;
@@ -2991,7 +2997,8 @@ void r5c_update_on_rdev_error(struct mddev *mddev)
if (!log)
return;
- if (raid5_calc_degraded(conf) > 0 &&
+ if ((raid5_calc_degraded(conf) > 0 ||
+ test_bit(Journal, &rdev->flags)) &&
conf->log->r5c_journal_mode == R5C_JOURNAL_MODE_WRITE_BACK)
schedule_work(&log->disable_writeback_work);
}
diff --git a/drivers/md/raid5-log.h b/drivers/md/raid5-log.h
index 2709710..328d67a 100644
--- a/drivers/md/raid5-log.h
+++ b/drivers/md/raid5-log.h
@@ -28,7 +28,8 @@ extern void r5c_flush_cache(struct r5conf *conf, int num);
extern void r5c_check_stripe_cache_usage(struct r5conf *conf);
extern void r5c_check_cached_full_stripe(struct r5conf *conf);
extern struct md_sysfs_entry r5c_journal_mode;
-extern void r5c_update_on_rdev_error(struct mddev *mddev);
+extern void r5c_update_on_rdev_error(struct mddev *mddev,
+ struct md_rdev *rdev);
extern bool r5c_big_stripe_cached(struct r5conf *conf, sector_t sect);
extern struct dma_async_tx_descriptor *
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index f8055a7..0ac57a9 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2689,7 +2689,7 @@ static void raid5_error(struct mddev *mddev, struct md_rdev *rdev)
bdevname(rdev->bdev, b),
mdname(mddev),
conf->raid_disks - mddev->degraded);
- r5c_update_on_rdev_error(mddev);
+ r5c_update_on_rdev_error(mddev, rdev);
}
/*
@@ -3050,6 +3050,11 @@ sector_t raid5_compute_blocknr(struct stripe_head *sh, int i, int previous)
* When LOG_CRITICAL, stripes with injournal == 0 will be sent to
* no_space_stripes list.
*
+ * 3. during journal failure
+ * In journal failure, we try to flush all cached data to raid disks
+ * based on data in stripe cache. The array is read-only to upper
+ * layers, so we would skip all pending writes.
+ *
*/
static inline bool delay_towrite(struct r5conf *conf,
struct r5dev *dev,
@@ -3063,6 +3068,9 @@ static inline bool delay_towrite(struct r5conf *conf,
if (test_bit(R5C_LOG_CRITICAL, &conf->cache_state) &&
s->injournal > 0)
return true;
+ /* case 3 above */
+ if (s->log_failed && s->injournal)
+ return true;
return false;
}
@@ -4696,10 +4704,15 @@ static void handle_stripe(struct stripe_head *sh)
" to_write=%d failed=%d failed_num=%d,%d\n",
s.locked, s.uptodate, s.to_read, s.to_write, s.failed,
s.failed_num[0], s.failed_num[1]);
- /* check if the array has lost more than max_degraded devices and,
+ /*
+ * check if the array has lost more than max_degraded devices and,
* if so, some requests might need to be failed.
+ *
+ * When journal device failed (log_failed), we will only process
+ * the stripe if there is data need write to raid disks
*/
- if (s.failed > conf->max_degraded || s.log_failed) {
+ if (s.failed > conf->max_degraded ||
+ (s.log_failed && s.injournal == 0)) {
sh->check_state = 0;
sh->reconstruct_state = 0;
break_stripe_batch_list(sh, 0);
@@ -5272,8 +5285,10 @@ static struct stripe_head *__get_priority_stripe(struct r5conf *conf, int group)
struct stripe_head *sh, *tmp;
struct list_head *handle_list = NULL;
struct r5worker_group *wg;
- bool second_try = !r5c_is_writeback(conf->log);
- bool try_loprio = test_bit(R5C_LOG_TIGHT, &conf->cache_state);
+ bool second_try = !r5c_is_writeback(conf->log) &&
+ !r5l_log_disk_error(conf);
+ bool try_loprio = test_bit(R5C_LOG_TIGHT, &conf->cache_state) ||
+ r5l_log_disk_error(conf);
again:
wg = NULL;
@@ -7521,7 +7536,9 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
* neilb: there is no locking about new writes here,
* so this cannot be safe.
*/
- if (atomic_read(&conf->active_stripes)) {
+ if (atomic_read(&conf->active_stripes) ||
+ atomic_read(&conf->r5c_cached_full_stripes) ||
+ atomic_read(&conf->r5c_cached_partial_stripes)) {
return -EBUSY;
}
log_exit(conf);
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] md:raid1/10: avoid unnecessary locking
From: NeilBrown @ 2017-05-11 22:29 UTC (permalink / raw)
To: shli, linux-raid; +Cc: Shaohua Li
In-Reply-To: <1946dca4cbeb08911e4fcc155d1b2ffa8bc6c0a2.1494431492.git.shli@fb.com>
[-- Attachment #1: Type: text/plain, Size: 2293 bytes --]
On Wed, May 10 2017, shli@kernel.org wrote:
> From: Shaohua Li <shli@fb.com>
>
> If we add bios to block plugging list, locking is unnecessry, since the block
> unplug is guaranteed not to run at that time.
Yes, that make sense. Thanks
Reviewed-by: NeilBrown <neilb@suse.com>
>
> Cc: NeilBrown <neilb@suse.com>
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
> drivers/md/raid1.c | 7 +++----
> drivers/md/raid10.c | 7 +++----
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 7c1f73398800..a17ed6218d51 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1529,17 +1529,16 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
> plug = container_of(cb, struct raid1_plug_cb, cb);
> else
> plug = NULL;
> - spin_lock_irqsave(&conf->device_lock, flags);
> if (plug) {
> bio_list_add(&plug->pending, mbio);
> plug->pending_cnt++;
> } else {
> + spin_lock_irqsave(&conf->device_lock, flags);
> bio_list_add(&conf->pending_bio_list, mbio);
> conf->pending_count++;
> - }
> - spin_unlock_irqrestore(&conf->device_lock, flags);
> - if (!plug)
> + spin_unlock_irqrestore(&conf->device_lock, flags);
> md_wakeup_thread(mddev->thread);
> + }
> }
>
> r1_bio_write_done(r1_bio);
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 6b86a0032cf8..4343d7ff9916 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1282,17 +1282,16 @@ static void raid10_write_one_disk(struct mddev *mddev, struct r10bio *r10_bio,
> plug = container_of(cb, struct raid10_plug_cb, cb);
> else
> plug = NULL;
> - spin_lock_irqsave(&conf->device_lock, flags);
> if (plug) {
> bio_list_add(&plug->pending, mbio);
> plug->pending_cnt++;
> } else {
> + spin_lock_irqsave(&conf->device_lock, flags);
> bio_list_add(&conf->pending_bio_list, mbio);
> conf->pending_count++;
> - }
> - spin_unlock_irqrestore(&conf->device_lock, flags);
> - if (!plug)
> + spin_unlock_irqrestore(&conf->device_lock, flags);
> md_wakeup_thread(mddev->thread);
> + }
> }
>
> static void raid10_write_request(struct mddev *mddev, struct bio *bio,
> --
> 2.11.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: RFC - Raid error detection and auto-recovery (was Fault tolerance with badblocks)
From: Eyal Lebedinsky @ 2017-05-11 23:31 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20170510170744.GB2925@lazy.lzy>
On 11/05/17 03:07, Piergiorgio Sartor wrote:
> On Wed, May 10, 2017 at 02:26:12PM +0100, Wols Lists wrote:
[trim]
>>
>> Coders and code welcome ... :-)
>
> I just would like to stress the fact that
> there is user-space code (raid6check) which
> perform check, possibily repair, on RAID6.
Short summary: the detect/correct options suggested by the OP are valuable.
raid6check is not the same thing. As an exercise I decided to run raid6check
instead of a raid 'check'. It is *very* slow. It seems to read the disks
sequentially (not in parallel).
After running for a day iostat shows the disks are read at about 6.5MB/s each,
which is a fraction of the raw performance of the disks (above 160MB/s).
These are 4TB disks so I expect the run will last about a week?
It probably started faster and will end slower.
A raid check starts at over 140MB/s and ends at above 70MB/s. It completes
in just under 10 hours.
For reference, the smart long test time suggests around 450 minutes (7.5 hours).
I checked iostat when there was no other activity on this array.
Unfortunately the program does not offer any progress option that I can see.
cheers
> bye,
>
>>
>> Cheers,
>> Wol
--
Eyal Lebedinsky (eyal@eyal.emu.id.au)
^ permalink raw reply
* [PATCH] md/r5cache: handle sync with data in write back cache
From: Song Liu @ 2017-05-12 0:03 UTC (permalink / raw)
To: linux-raid
Cc: shli, neilb, kernel-team, dan.j.williams, hch, jes.sorensen,
Song Liu
Currently, sync of raid456 array cannot make progress when hitting
data in writeback r5cache.
This patch fixes this issue by flushing cached data of the stripe
before processing the sync request. This is achived by:
1. In handle_stripe(), do not set STRIPE_SYNCING if the stripe is
in write back cache;
2. In r5c_try_caching_write(), handle the stripe in sync with write
through;
3. In do_release_stripe(), make stripe in sync write out and send
it to the state machine.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
drivers/md/raid5-cache.c | 5 ++++-
drivers/md/raid5.c | 21 +++++++++++++++------
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index a6a62e2..f476105 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -2631,8 +2631,11 @@ int r5c_try_caching_write(struct r5conf *conf,
* When run in degraded mode, array is set to write-through mode.
* This check helps drain pending write safely in the transition to
* write-through mode.
+ *
+ * When a stripe is syncing, the write is also handled in write
+ * through mode.
*/
- if (s->failed) {
+ if (s->failed || test_bit(STRIPE_SYNCING, &sh->state)) {
r5c_make_stripe_write_out(sh);
return -EAGAIN;
}
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index f8055a7..680b1f6 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -233,11 +233,15 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh,
if (test_bit(R5_InJournal, &sh->dev[i].flags))
injournal++;
/*
- * When quiesce in r5c write back, set STRIPE_HANDLE for stripes with
- * data in journal, so they are not released to cached lists
+ * In the following cases, the stripe cannot be released to cached
+ * lists. Therefore, we make the stripe write out and set
+ * STRIPE_HANDLE:
+ * 1. when quiesce in r5c write back;
+ * 2. when resync is requested fot the stripe.
*/
- if (conf->quiesce && r5c_is_writeback(conf->log) &&
- !test_bit(STRIPE_HANDLE, &sh->state) && injournal != 0) {
+ if (test_bit(STRIPE_SYNC_REQUESTED, &sh->state) ||
+ (conf->quiesce && r5c_is_writeback(conf->log) &&
+ !test_bit(STRIPE_HANDLE, &sh->state) && injournal != 0)) {
if (test_bit(STRIPE_R5C_CACHING, &sh->state))
r5c_make_stripe_write_out(sh);
set_bit(STRIPE_HANDLE, &sh->state);
@@ -4648,8 +4652,13 @@ static void handle_stripe(struct stripe_head *sh)
if (test_bit(STRIPE_SYNC_REQUESTED, &sh->state) && !sh->batch_head) {
spin_lock(&sh->stripe_lock);
- /* Cannot process 'sync' concurrently with 'discard' */
- if (!test_bit(STRIPE_DISCARD, &sh->state) &&
+ /*
+ * Cannot process 'sync' concurrently with 'discard'.
+ * Flush data in r5cache before 'sync'.
+ */
+ if (!test_bit(STRIPE_R5C_PARTIAL_STRIPE, &sh->state) &&
+ !test_bit(STRIPE_R5C_FULL_STRIPE, &sh->state) &&
+ !test_bit(STRIPE_DISCARD, &sh->state) &&
test_and_clear_bit(STRIPE_SYNC_REQUESTED, &sh->state)) {
set_bit(STRIPE_SYNCING, &sh->state);
clear_bit(STRIPE_INSYNC, &sh->state);
--
2.9.3
^ permalink raw reply related
* [PATCH] super1: fix sb->max_dev when adding a new disk in linear array
From: Lidong Zhong @ 2017-05-12 1:51 UTC (permalink / raw)
To: linux-raid; +Cc: colyli, Jes.Sorensen
The value of sb->max_dev will always be increased by 1 when adding
a new disk in linear array. It causes an inconsistence between each
disk in the array and the "Array State" value of "mdadm --examine DISK"
is wrong. For example, when adding the first new disk into linear array
it will be:
Array State : RAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
('A' == active, '.' == missing, 'R' == replacing)
Adding the second disk into linear array it will be
Array State : .AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
('A' == active, '.' == missing, 'R' == replacing)
Signed-off-by: Lidong Zhong <lzhong@suse.com>
---
super1.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/super1.c b/super1.c
index 87a74cb..4fb655f 100644
--- a/super1.c
+++ b/super1.c
@@ -1184,8 +1184,10 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
break;
sb->dev_number = __cpu_to_le32(i);
info->disk.number = i;
- if (max >= __le32_to_cpu(sb->max_dev))
+ if (i >= __le32_to_cpu(sb->max_dev)) {
sb->max_dev = __cpu_to_le32(max+1);
+ sb->dev_roles[sb->max_dev] = __cpu_to_le16(MD_DISK_ROLE_FAULTY);
+ }
random_uuid(sb->device_uuid);
@@ -1214,6 +1216,10 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
sb->raid_disks = __cpu_to_le32(info->array.raid_disks);
sb->dev_roles[info->disk.number] =
__cpu_to_le16(info->disk.raid_disk);
+ if (sb->raid_disks+1 >= __le32_to_cpu(sb->max_dev)) {
+ sb->max_dev = __cpu_to_le32(sb->raid_disks+1);
+ sb->dev_roles[sb->max_dev] = __cpu_to_le16(MD_DISK_ROLE_FAULTY);
+ }
} else if (strcmp(update, "resync") == 0) {
/* make sure resync happens */
sb->resync_offset = 0ULL;
--
2.12.0
^ permalink raw reply related
* Re: [PATCH] super1: fix sb->max_dev when adding a new disk in linear array
From: Coly Li @ 2017-05-12 7:53 UTC (permalink / raw)
To: Lidong Zhong, linux-raid; +Cc: Jes.Sorensen
In-Reply-To: <20170512015145.18542-1-lzhong@suse.com>
On 2017/5/12 上午9:51, Lidong Zhong wrote:
> The value of sb->max_dev will always be increased by 1 when adding
> a new disk in linear array. It causes an inconsistence between each
> disk in the array and the "Array State" value of "mdadm --examine DISK"
> is wrong. For example, when adding the first new disk into linear array
> it will be:
>
> Array State : RAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
> ('A' == active, '.' == missing, 'R' == replacing)
>
> Adding the second disk into linear array it will be
>
> Array State : .AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
> ('A' == active, '.' == missing, 'R' == replacing)
>
> Signed-off-by: Lidong Zhong <lzhong@suse.com>
The above description does not very well explain why this fix is necessary.
For linear device, there was a race condition and got fixed already.
There are users encountered such bug in old kernel, when they trying to
add a hard disk into a linear device. After the kernel panic caused by
the race issue in old md linear code, they reboot from the fixed kernel,
and find 'R' or '?' marks from Array State output.
After analyze the metadata, we found value of
sb->dev_roles[sb->max_dev-1] is 0, but which should be
MD_DISK_ROLE_FAULTY. The reason for the error is, when adding a disk
into linear device, mdadm first increases sb->max_dev value, but not set
its role value. So the corresponding role value keeps as 0, which is
same as No.0 disk's role value in the linear device.
So mdadm -E will find there are 2 same role value for No.0 disk, and the
first state character of Array State is marked as 'R'. If such a panic
happened multiple times, there are more bogus zero value at tail of
dev_roles[], so the character of Array State is marked as '?'.
Now the kernel part is fixed, but mdadm part is not fixed yet. This
patch is an effort to avoid bogus dev_roles[] value if kernel panic
happens in follow ioctl system call (although the possibility is quite
low and who can tel ...).
> ---
> super1.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/super1.c b/super1.c
> index 87a74cb..4fb655f 100644
> --- a/super1.c
> +++ b/super1.c
> @@ -1184,8 +1184,10 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
> break;
> sb->dev_number = __cpu_to_le32(i);
> info->disk.number = i;
> - if (max >= __le32_to_cpu(sb->max_dev))
> + if (i >= __le32_to_cpu(sb->max_dev)) {
> sb->max_dev = __cpu_to_le32(max+1);
> + sb->dev_roles[sb->max_dev] = __cpu_to_le16(MD_DISK_ROLE_FAULTY);
> + }
>
> random_uuid(sb->device_uuid);
The above part makes sense to me.
>
> @@ -1214,6 +1216,10 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
> sb->raid_disks = __cpu_to_le32(info->array.raid_disks);
> sb->dev_roles[info->disk.number] =
> __cpu_to_le16(info->disk.raid_disk);
> + if (sb->raid_disks+1 >= __le32_to_cpu(sb->max_dev)) {
> + sb->max_dev = __cpu_to_le32(sb->raid_disks+1);
> + sb->dev_roles[sb->max_dev] = __cpu_to_le16(MD_DISK_ROLE_FAULTY);
> + }
> } else if (strcmp(update, "resync") == 0) {
> /* make sure resync happens */
> sb->resync_offset = 0ULL;
>
Just a lazy question, could you please explain a little bit why you also
modify here ? Thanks.
Coly
^ permalink raw reply
* APPLY FOR YOUR URGENT LOAN AT 2%
From: Thomas Credit Firm @ 2017-05-12 10:53 UTC (permalink / raw)
To: receipt
* Do you need an urgent loan? We offer all kinds of loan
* Very fast and urgent transfer to your bank account.
* Payment begins (6) months after you get the money in your bank account
* Low interest rates 2%
* Payment of long-term (1-30 years) duration
For more information and loan application form through the Contact now
E-mail: Thomascreditfirm@gmail.com
Thomas B. Dawson
THOMAS CREDIT FIRM
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
^ permalink raw reply
* Re: A sector-of-mismatch warning patch (was Re: Fault tolerance with badblocks)
From: Nix @ 2017-05-12 11:14 UTC (permalink / raw)
To: Chris Murphy
Cc: David Brown, Anthony Youngman, Phil Turmel, Ravi (Tom) Hale,
Linux-RAID
In-Reply-To: <87bmr14u5f.fsf_-_@esperi.org.uk>
On 9 May 2017, nix@esperi.org.uk outgrape:
> On 9 May 2017, Chris Murphy verbalised:
>
>> 1. md reports all data drives and the LBAs for the affected stripe
>
> Enough rambling from me. Here's a hilariously untested patch against
> 4.11 (as in I haven't even booted with it: my systems are kind of in
> flux right now as I migrate to the md-based server that got me all
> concerned about this). It compiles! And it's definitely safer than
> trying a repair, and makes it possible to recover from a real mismatch
> without losing all your hair in the process, or determine that a
> mismatch is spurious or irrelevant. And that's enough for me, frankly.
> This is a very rare problem, one hopes.
>
> (It's probably not ideal, because the error is just known to be
> somewhere in that stripe, not on that sector, which makes determining
> the affected data somewhat harder. But at least you can figure out what
> filesystem it's on. :) )
Aside: this foolish optimist hopes that it might be fairly easy to tie
the new GETFSMAP ioctl() into mismatch reports if the filesystem(s)
overlying a mismatched stripe support it: it looks like we could get the
necessary info for a whole stripe in a single call. Being automatically
told "these files may be corrupted, restore them" or "oops you lost some
metadata on fses A and B, run fsck" would be wonderful. (Though the
actual corruption would be less wonderful.)
This feels like something mdadm's monitor mode should be able to do, to
me. I'll have a look in a bit, but I know nothing about the
implementation of monitor mode at all so I have some learning to do
first...
--
NULL && (void)
^ 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