* Re: RAID6 write I/O amplification?
From: Alireza Haghdoost @ 2015-02-26 0:40 UTC (permalink / raw)
To: Markus Stockhausen; +Cc: Roman Mamedov, linux-raid@vger.kernel.org
In-Reply-To: <12EF8D94C6F8734FB2FF37B9FBEDD1735F9E168E@EXCHANGE.collogia.de>
On Tue, Feb 24, 2015 at 12:29 AM, Markus Stockhausen
<stockhausen@collogia.de> wrote:
>> Von: linux-raid-owner@vger.kernel.org [linux-raid-owner@vger.kernel.org]" im Auftrag von "Roman Mamedov [rm@romanrm.net]
>> Gesendet: Dienstag, 24. Februar 2015 00:58
>> An: linux-raid@vger.kernel.org
>> Betreff: RAID6 write I/O amplification?
>>
>> Hello,
>>
>> Got a bit of a "how does it actually work" question...
>>
>> Suppose I have an MD RAID6 of 8 drives, with 64KB chunk size.
>>
>> I am rewriting a 4KB filesystem sector somewhere on that RAID (not crossing
>> the stripe boundary).
>>
>> What's the amount of disk I/O in total this will result in?
>>
>> I assume the RAID will need to read data from all drives, recompute parity,
>> then write to the data stripe where the updated piece happened to be, and also
>> write to two parity stripes.
>>
>> Is this done at a stripe granularity, so 6x64KB reads, 3x64KB writes?
>> Or down to individual sectors (pages), i.e. 6x4KB reads, 3x4KB writes?
>> Or am I describing this algorithm correctly at all?
>
> Implementation will work on "internal" stripe granularity and that is 4K
> So your case will be 6x4KB read + 3x4KB write.
Having said that, does it mean that following description of "chunk
size" is wrong:
'[chunk size] is the smallest "atomic" mass of data that can be
written to the devices'
since in this case chunk size is 64KB but 4KB is written atomically (?).
I have find it in the kernel.org wiki page [1]
---
Alireza
1. https://raid.wiki.kernel.org/index.php/RAID_setup
^ permalink raw reply
* Re: RAID6 write I/O amplification?
From: NeilBrown @ 2015-02-26 0:55 UTC (permalink / raw)
To: Alireza Haghdoost
Cc: Markus Stockhausen, Roman Mamedov, linux-raid@vger.kernel.org
In-Reply-To: <CAB-428mPydqGoku-RnhZUDVyHVnzb73Yz=5bL7DOd+G88siDtg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2226 bytes --]
On Wed, 25 Feb 2015 18:40:46 -0600 Alireza Haghdoost <alireza@cs.umn.edu>
wrote:
> On Tue, Feb 24, 2015 at 12:29 AM, Markus Stockhausen
> <stockhausen@collogia.de> wrote:
> >> Von: linux-raid-owner@vger.kernel.org [linux-raid-owner@vger.kernel.org]" im Auftrag von "Roman Mamedov [rm@romanrm.net]
> >> Gesendet: Dienstag, 24. Februar 2015 00:58
> >> An: linux-raid@vger.kernel.org
> >> Betreff: RAID6 write I/O amplification?
> >>
> >> Hello,
> >>
> >> Got a bit of a "how does it actually work" question...
> >>
> >> Suppose I have an MD RAID6 of 8 drives, with 64KB chunk size.
> >>
> >> I am rewriting a 4KB filesystem sector somewhere on that RAID (not crossing
> >> the stripe boundary).
> >>
> >> What's the amount of disk I/O in total this will result in?
> >>
> >> I assume the RAID will need to read data from all drives, recompute parity,
> >> then write to the data stripe where the updated piece happened to be, and also
> >> write to two parity stripes.
> >>
> >> Is this done at a stripe granularity, so 6x64KB reads, 3x64KB writes?
> >> Or down to individual sectors (pages), i.e. 6x4KB reads, 3x4KB writes?
> >> Or am I describing this algorithm correctly at all?
> >
> > Implementation will work on "internal" stripe granularity and that is 4K
> > So your case will be 6x4KB read + 3x4KB write.
>
> Having said that, does it mean that following description of "chunk
> size" is wrong:
> '[chunk size] is the smallest "atomic" mass of data that can be
> written to the devices'
> since in this case chunk size is 64KB but 4KB is written atomically (?).
> I have find it in the kernel.org wiki page [1]
I think that when it says "atomic" it means in space, not time.
i.e. one (properly aligned) chunk of data will not be split up and
written to different devices, it will all be written to one device.
If you write more than a chunk, it will be split up and parts of if written
to different devices.
You can still write less than a chunk.
So the intent is correct I think, but the word "atomic" doesn't really convey
the right meaning. Probably it should be re-written to avoid that term and
just spell out what is happening.
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* smartctl-timeouts_v1.0 (erc timeout fix)
From: Chris @ 2015-02-27 12:31 UTC (permalink / raw)
To: linux-raid
[-- Attachment #1: Type: text/plain, Size: 381 bytes --]
Hi,
here they are now. The smartctl-timeouts scripts fix commonly
mismatching defaults with drives that have no error recovery timeout
configured, which has often lead to data loss.
To test, extract the files to /etc/udev/rules.d/ and
*reboot*. For me, the rules somehow had no effect without
rebooting.
Please test, report, and then ship it with smartctl/mdadm.
Cheers,
Chris
[-- Attachment #2: smartctl-timeouts_v1.0.zip --]
[-- Type: application/zip, Size: 10355 bytes --]
^ permalink raw reply
* Re: [PATCH 3/5] add_orom(): Compare content of struct imsm_orom rather than pointers to it
From: Artur Paszkiewicz @ 2015-02-27 13:39 UTC (permalink / raw)
To: Jes Sorensen; +Cc: neilb, linux-raid
In-Reply-To: <wrfj1tld3oa9.fsf@redhat.com>
On 02/25/2015 06:15 PM, Jes Sorensen wrote:
> Artur Paszkiewicz <artur.paszkiewicz@intel.com> writes:
>> On 02/25/2015 01:29 PM, Jes Sorensen wrote:
>>> Artur Paszkiewicz <artur.paszkiewicz@intel.com> writes:
>>>> On 02/24/2015 10:00 PM, Jes.Sorensen@redhat.com wrote:
>>>>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>>>>
>>>>> This avoids adding the same orom entry to the oroms list multiple
>>>>> times, as the comparison of pointers is never going to succeed, in
>>>>> particular when '*orom' points to a local stack variable in the
>>>>> calling function.
>>>>>
>>>>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>>>>> ---
>>>>> platform-intel.c | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/platform-intel.c b/platform-intel.c
>>>>> index 37274da..a4ffa9f 100644
>>>>> --- a/platform-intel.c
>>>>> +++ b/platform-intel.c
>>>>> @@ -255,8 +255,8 @@ static const struct imsm_orom *add_orom(const struct imsm_orom *orom)
>>>>> int i;
>>>>>
>>>>> for (i = 0; i < SYS_DEV_MAX; i++) {
>>>>> - if (&oroms[i].orom == orom)
>>>>> - return orom;
>>>>> + if (!memcmp(&oroms[i].orom, orom, sizeof(struct imsm_orom)))
>>>>> + return &oroms[i].orom;
>>>>> if (oroms[i].orom.signature[0] == 0) {
>>>>> oroms[i].orom = *orom;
>>>>> return &oroms[i].orom;
>>>>>
>>>>
>>>> Hi Jes,
>>>>
>>>> You are right that this can add the same entry multiple times, but this
>>>> is how it is supposed to work. The oroms list should contain all the
>>>> platform's oroms and they can be the same, this is why memcmp() should
>>>> not be used here. We don't want to compare the contents of the
>>>> structure, just its address. Sorry if it's not clear.
>>>
>>> Artur,
>>>
>>> Then the code is fundamentally broken, since you end up comparing a
>>> stack variable against the oroms array when you call it from
>>> find_imsm_efi(). Worse you can end up returning the local stack variable
>>> declared in find_imsm_efi() to the calling function - there is no way
>>> that can be correct.
>>>
>>> Look at this:
>>>
>>> static const struct imsm_orom *add_orom(const struct imsm_orom *orom)
>>> {
>>> int i;
>>>
>>> for (i = 0; i < SYS_DEV_MAX; i++) {
>>> if (&oroms[i].orom == orom)
>>> return orom;
>>> if (oroms[i].orom.signature[0] == 0) {
>>> oroms[i].orom = *orom;
>>> return &oroms[i].orom;
>>> }
>>> }
>>> return NULL;
>>> }
>>>
>>> const struct imsm_orom *find_imsm_efi(struct sys_dev *hba)
>>> {
>>> struct imsm_orom orom;
>>> const struct imsm_orom *ret;
>>> int err;
>>>
>>> ....
>>>
>>> ret = add_orom(&orom);
>>> add_orom_device_id(ret, hba->dev_id);
>>>
>>> return ret;
>>> }
>>
>> I can't see how this can lead to returning a stack variable. The oroms
>> array is global and add_orom() will always return a pointer to a struct
>> in this array. This comparison will always fail when we pass a pointer
>> to a stack variable to add_orom():
>>
>> if (&oroms[i].orom == orom)
>> return orom;
>>
>> This was meant to prevent adding an orom again like this:
>>
>> ret = add_orom(&orom);
>> add_orom(ret);
>>
>> Maybe it would be more appropriate to return NULL to indicate that
>> nothing was added instead of returning back the same pointer. I can do a
>> patch for this. What do you think?
>
> It will fail because we know we're comparing a stack pointer, but it
> raises red flags with tools like coverity and it is really bad coding
> practice to rely on hacks like this.
>
> I also don't understand why you want to keep a table of identical
> entries in the orom structure if multiple identical entries are found.
> Each entry ought to match onto a specific physical controller, unless I
> get something wrong?
>
OK, you're right, it is a hack. I thought it over and redesigned those
orom functions. This should make it simpler and more consistent.
Thanks,
Artur
From 673ecf1c0539f0050cc5934203af6d79cd68234d Mon Sep 17 00:00:00 2001
From: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
Date: Fri, 27 Feb 2015 10:34:20 +0100
Subject: [PATCH] imsm: simplified multiple OROMs support
Replaced oroms array with list, add_orom() now only appends to this list
and add_orom_device_id() only appends devid_list node to an orom_entry.
Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
---
platform-intel.c | 96 +++++++++++++++++++++++++++-----------------------------
platform-intel.h | 4 ++-
super-intel.c | 18 +++++------
3 files changed, 57 insertions(+), 61 deletions(-)
diff --git a/platform-intel.c b/platform-intel.c
index 37274da..9c89c20 100644
--- a/platform-intel.c
+++ b/platform-intel.c
@@ -229,65 +229,61 @@ struct pciExpDataStructFormat {
__u16 devListOffset;
} __attribute__ ((packed));
-static struct orom_entry oroms[SYS_DEV_MAX];
-
-const struct orom_entry *get_oroms(void)
-{
- return (const struct orom_entry *)&oroms;
-}
+struct orom_entry *orom_entries;
const struct imsm_orom *get_orom_by_device_id(__u16 dev_id)
{
- int i;
- struct devid_list *list;
+ struct orom_entry *entry;
+ struct devid_list *devid;
- for (i = 0; i < SYS_DEV_MAX; i++) {
- for (list = oroms[i].devid_list; list; list = list->next) {
- if (list->devid == dev_id)
- return &oroms[i].orom;
+ for (entry = orom_entries; entry; entry = entry->next) {
+ for (devid = entry->devid_list; devid; devid = devid->next) {
+ if (devid->devid == dev_id)
+ return &entry->orom;
}
}
+
return NULL;
}
-static const struct imsm_orom *add_orom(const struct imsm_orom *orom)
+static struct orom_entry *add_orom(const struct imsm_orom *orom)
{
- int i;
-
- for (i = 0; i < SYS_DEV_MAX; i++) {
- if (&oroms[i].orom == orom)
- return orom;
- if (oroms[i].orom.signature[0] == 0) {
- oroms[i].orom = *orom;
- return &oroms[i].orom;
- }
- }
- return NULL;
+ struct orom_entry *list;
+ struct orom_entry *prev = NULL;
+
+ for (list = orom_entries; list; prev = list, list = list->next)
+ ;
+
+ list = xmalloc(sizeof(struct orom_entry));
+ list->orom = *orom;
+ list->devid_list = NULL;
+ list->next = NULL;
+
+ if (prev == NULL)
+ orom_entries = list;
+ else
+ prev->next = list;
+
+ return list;
}
-static void add_orom_device_id(const struct imsm_orom *orom, __u16 dev_id)
+static void add_orom_device_id(struct orom_entry *entry, __u16 dev_id)
{
- int i;
struct devid_list *list;
struct devid_list *prev = NULL;
- for (i = 0; i < SYS_DEV_MAX; i++) {
- if (&oroms[i].orom == orom) {
- for (list = oroms[i].devid_list; list; prev = list, list = list->next) {
- if (list->devid == dev_id)
- return;
- }
- list = xmalloc(sizeof(struct devid_list));
- list->devid = dev_id;
- list->next = NULL;
-
- if (prev == NULL)
- oroms[i].devid_list = list;
- else
- prev->next = list;
+ for (list = entry->devid_list; list; prev = list, list = list->next) {
+ if (list->devid == dev_id)
return;
- }
}
+ list = xmalloc(sizeof(struct devid_list));
+ list->devid = dev_id;
+ list->next = NULL;
+
+ if (prev == NULL)
+ entry->devid_list = list;
+ else
+ prev->next = list;
}
static int scan(const void *start, const void *end, const void *data)
@@ -321,7 +317,7 @@ static int scan(const void *start, const void *end, const void *data)
if (!imsm_mem)
return 0;
- const struct imsm_orom *orom = add_orom(imsm_mem);
+ struct orom_entry *orom = add_orom(imsm_mem);
if (ptr->devListOffset) {
const __u16 *dev_list = (void *)ptr + ptr->devListOffset;
@@ -367,11 +363,11 @@ const struct imsm_orom *imsm_platform_test(struct sys_dev *hba)
IMSM_OROM_RLC_RAID10;
}
- const struct imsm_orom *ret = add_orom(&orom);
+ struct orom_entry *ret = add_orom(&orom);
add_orom_device_id(ret, hba->dev_id);
- return ret;
+ return &ret->orom;
}
static const struct imsm_orom *find_imsm_hba_orom(struct sys_dev *hba)
@@ -508,7 +504,7 @@ static int read_efi_variable(void *buffer, ssize_t buf_size, char *variable_name
const struct imsm_orom *find_imsm_efi(struct sys_dev *hba)
{
struct imsm_orom orom;
- const struct imsm_orom *ret;
+ struct orom_entry *ret;
int err;
if (check_env("IMSM_TEST_AHCI_EFI") || check_env("IMSM_TEST_SCU_EFI"))
@@ -529,14 +525,14 @@ const struct imsm_orom *find_imsm_efi(struct sys_dev *hba)
/* try to read variable for combined AHCI controllers */
if (err && hba->type == SYS_DEV_SATA) {
- static const struct imsm_orom *csata;
+ static struct orom_entry *csata;
err = read_efi_variable(&orom, sizeof(orom), AHCI_CSATA_PROP, VENDOR_GUID);
if (!err) {
if (!csata)
csata = add_orom(&orom);
add_orom_device_id(csata, hba->dev_id);
- return csata;
+ return &csata->orom;
}
}
@@ -546,12 +542,12 @@ const struct imsm_orom *find_imsm_efi(struct sys_dev *hba)
ret = add_orom(&orom);
add_orom_device_id(ret, hba->dev_id);
- return ret;
+ return &ret->orom;
}
const struct imsm_orom *find_imsm_nvme(struct sys_dev *hba)
{
- static const struct imsm_orom *nvme_orom;
+ static struct orom_entry *nvme_orom;
if (hba->type != SYS_DEV_NVME)
return NULL;
@@ -574,7 +570,7 @@ const struct imsm_orom *find_imsm_nvme(struct sys_dev *hba)
nvme_orom = add_orom(&nvme_orom_compat);
}
add_orom_device_id(nvme_orom, hba->dev_id);
- return nvme_orom;
+ return &nvme_orom->orom;
}
const struct imsm_orom *find_imsm_capability(struct sys_dev *hba)
diff --git a/platform-intel.h b/platform-intel.h
index 2ead431..631fa76 100644
--- a/platform-intel.h
+++ b/platform-intel.h
@@ -213,8 +213,11 @@ struct devid_list {
struct orom_entry {
struct imsm_orom orom;
struct devid_list *devid_list;
+ struct orom_entry *next;
};
+extern struct orom_entry *orom_entries;
+
static inline char *guid_str(char *buf, struct efi_guid guid)
{
sprintf(buf, "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
@@ -235,6 +238,5 @@ int devt_attached_to_hba(dev_t dev, const char *hba_path);
char *devt_to_devpath(dev_t dev);
int path_attached_to_hba(const char *disk_path, const char *hba_path);
const char *get_sys_dev_type(enum sys_dev_type);
-const struct orom_entry * get_oroms(void);
const struct imsm_orom *get_orom_by_device_id(__u16 device_id);
struct sys_dev *device_by_id(__u16 device_id);
diff --git a/super-intel.c b/super-intel.c
index 819e0da..53269fd 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1948,13 +1948,12 @@ static int detail_platform_imsm(int verbose, int enumerate_only, char *controlle
return result;
}
- const struct orom_entry *oroms = get_oroms();
- int i;
+ const struct orom_entry *entry;
- for (i = 0; i < SYS_DEV_MAX && oroms[i].devid_list; i++) {
- print_imsm_capability(&oroms[i].orom);
+ for (entry = orom_entries; entry; entry = entry->next) {
+ print_imsm_capability(&entry->orom);
- if (imsm_orom_is_nvme(&oroms[i].orom)) {
+ if (imsm_orom_is_nvme(&entry->orom)) {
for (hba = list; hba; hba = hba->next) {
if (hba->type == SYS_DEV_NVME)
printf(" NVMe Device : %s\n", hba->path);
@@ -1963,7 +1962,7 @@ static int detail_platform_imsm(int verbose, int enumerate_only, char *controlle
}
struct devid_list *devid;
- for (devid = oroms[i].devid_list; devid; devid = devid->next) {
+ for (devid = entry->devid_list; devid; devid = devid->next) {
hba = device_by_id(devid->devid);
if (!hba)
continue;
@@ -2007,11 +2006,10 @@ static int export_detail_platform_imsm(int verbose, char *controller_path)
result = 0;
}
- const struct orom_entry *oroms = get_oroms();
- int i;
+ const struct orom_entry *entry;
- for (i = 0; i < SYS_DEV_MAX && oroms[i].devid_list; i++)
- print_imsm_capability_export(&oroms[i].orom);
+ for (entry = orom_entries; entry; entry = entry->next)
+ print_imsm_capability_export(&entry->orom);
return result;
}
--
2.1.4
^ permalink raw reply related
* [PATCH] IMSM-orom: make sure, that device list is supported
From: Pawel Baldysiak @ 2015-02-27 14:45 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, pawel.baldysiak, artur.paszkiewicz
Devices list in PCI Data Structure is supported only in
3 and above revision. Make sure that this is checked.
Signed-off-by: Pawel Baldysiak <pawel.baldysiak@intel.com>
---
platform-intel.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/platform-intel.c b/platform-intel.c
index 37274da..c6a28e8 100644
--- a/platform-intel.c
+++ b/platform-intel.c
@@ -227,6 +227,8 @@ struct pciExpDataStructFormat {
__u16 vendorID;
__u16 deviceID;
__u16 devListOffset;
+ __u16 pciDataStructLen;
+ __u8 pciDataStructRev;
} __attribute__ ((packed));
static struct orom_entry oroms[SYS_DEV_MAX];
@@ -323,7 +325,8 @@ static int scan(const void *start, const void *end, const void *data)
const struct imsm_orom *orom = add_orom(imsm_mem);
- if (ptr->devListOffset) {
+ /* only PciDataStructure with revision 3 and above supports devices list. */
+ if (ptr->pciDataStructRev >= 3 && ptr->devListOffset) {
const __u16 *dev_list = (void *)ptr + ptr->devListOffset;
int i;
^ permalink raw reply related
* [PATCH] IncRemove: Set "auto-read" only after successful excl open.
From: Pawel Baldysiak @ 2015-02-27 14:47 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, pawel.baldysiak, artur.paszkiewicz
"mdadm -If" - triggered from udev rules when disk is removed from OS -
tries to set array in auto-read-only mode. This can interrupt rebuild
process which is started automatically, e.g. if array is mounted and
spare disk is available (I/O error is detected faster than removing
failed disk by mdadm).
This patch prevents "mdadm -If" from setting array into "auto-read-only",
by requiring exclusive open to succeed.
Signed-off-by: Pawel Baldysiak <pawel.baldysiak@intel.com>
---
Incremental.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/Incremental.c b/Incremental.c
index 87d9114..b12f144 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -1712,12 +1712,16 @@ int IncrementalRemove(char *devname, char *id_path, int verbose)
return 1;
}
sysfs_init(&mdi, -1, ent->devnm);
- if (sysfs_get_str(&mdi, NULL, "array_state",
- buf, sizeof(buf)) > 0) {
- if (strncmp(buf, "active", 6) == 0 ||
- strncmp(buf, "clean", 5) == 0)
- sysfs_set_str(&mdi, NULL,
- "array_state", "read-auto");
+ mdfd = open_dev_excl(ent->devnm);
+ if (mdfd > 0) {
+ close(mdfd);
+ if (sysfs_get_str(&mdi, NULL, "array_state",
+ buf, sizeof(buf)) > 0) {
+ if (strncmp(buf, "active", 6) == 0 ||
+ strncmp(buf, "clean", 5) == 0)
+ sysfs_set_str(&mdi, NULL,
+ "array_state", "read-auto");
+ }
}
mdfd = open_dev(ent->devnm);
if (mdfd < 0) {
^ permalink raw reply related
* Re: [PATCH 3/5] add_orom(): Compare content of struct imsm_orom rather than pointers to it
From: Jes Sorensen @ 2015-02-27 20:51 UTC (permalink / raw)
To: Artur Paszkiewicz; +Cc: neilb, linux-raid
In-Reply-To: <54F0739E.50207@intel.com>
Artur Paszkiewicz <artur.paszkiewicz@intel.com> writes:
> On 02/25/2015 06:15 PM, Jes Sorensen wrote:
>> Artur Paszkiewicz <artur.paszkiewicz@intel.com> writes:
>>> On 02/25/2015 01:29 PM, Jes Sorensen wrote:
>>>> Artur Paszkiewicz <artur.paszkiewicz@intel.com> writes:
>>>>> On 02/24/2015 10:00 PM, Jes.Sorensen@redhat.com wrote:
>>>>>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>>>>>
>>>>>> This avoids adding the same orom entry to the oroms list multiple
>>>>>> times, as the comparison of pointers is never going to succeed, in
>>>>>> particular when '*orom' points to a local stack variable in the
>>>>>> calling function.
>>>>>>
>>>>>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>>>>>> ---
>>>>>> platform-intel.c | 4 ++--
>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/platform-intel.c b/platform-intel.c
>>>>>> index 37274da..a4ffa9f 100644
>>>>>> --- a/platform-intel.c
>>>>>> +++ b/platform-intel.c
>>>>>> @@ -255,8 +255,8 @@ static const struct imsm_orom *add_orom(const struct imsm_orom *orom)
>>>>>> int i;
>>>>>>
>>>>>> for (i = 0; i < SYS_DEV_MAX; i++) {
>>>>>> - if (&oroms[i].orom == orom)
>>>>>> - return orom;
>>>>>> + if (!memcmp(&oroms[i].orom, orom, sizeof(struct imsm_orom)))
>>>>>> + return &oroms[i].orom;
>>>>>> if (oroms[i].orom.signature[0] == 0) {
>>>>>> oroms[i].orom = *orom;
>>>>>> return &oroms[i].orom;
>>>>>>
>>>>>
>>>>> Hi Jes,
>>>>>
>>>>> You are right that this can add the same entry multiple times, but this
>>>>> is how it is supposed to work. The oroms list should contain all the
>>>>> platform's oroms and they can be the same, this is why memcmp() should
>>>>> not be used here. We don't want to compare the contents of the
>>>>> structure, just its address. Sorry if it's not clear.
>>>>
>>>> Artur,
>>>>
>>>> Then the code is fundamentally broken, since you end up comparing a
>>>> stack variable against the oroms array when you call it from
>>>> find_imsm_efi(). Worse you can end up returning the local stack variable
>>>> declared in find_imsm_efi() to the calling function - there is no way
>>>> that can be correct.
>>>>
>>>> Look at this:
>>>>
>>>> static const struct imsm_orom *add_orom(const struct imsm_orom *orom)
>>>> {
>>>> int i;
>>>>
>>>> for (i = 0; i < SYS_DEV_MAX; i++) {
>>>> if (&oroms[i].orom == orom)
>>>> return orom;
>>>> if (oroms[i].orom.signature[0] == 0) {
>>>> oroms[i].orom = *orom;
>>>> return &oroms[i].orom;
>>>> }
>>>> }
>>>> return NULL;
>>>> }
>>>>
>>>> const struct imsm_orom *find_imsm_efi(struct sys_dev *hba)
>>>> {
>>>> struct imsm_orom orom;
>>>> const struct imsm_orom *ret;
>>>> int err;
>>>>
>>>> ....
>>>>
>>>> ret = add_orom(&orom);
>>>> add_orom_device_id(ret, hba->dev_id);
>>>>
>>>> return ret;
>>>> }
>>>
>>> I can't see how this can lead to returning a stack variable. The oroms
>>> array is global and add_orom() will always return a pointer to a struct
>>> in this array. This comparison will always fail when we pass a pointer
>>> to a stack variable to add_orom():
>>>
>>> if (&oroms[i].orom == orom)
>>> return orom;
>>>
>>> This was meant to prevent adding an orom again like this:
>>>
>>> ret = add_orom(&orom);
>>> add_orom(ret);
>>>
>>> Maybe it would be more appropriate to return NULL to indicate that
>>> nothing was added instead of returning back the same pointer. I can do a
>>> patch for this. What do you think?
>>
>> It will fail because we know we're comparing a stack pointer, but it
>> raises red flags with tools like coverity and it is really bad coding
>> practice to rely on hacks like this.
>>
>> I also don't understand why you want to keep a table of identical
>> entries in the orom structure if multiple identical entries are found.
>> Each entry ought to match onto a specific physical controller, unless I
>> get something wrong?
>>
>
> OK, you're right, it is a hack. I thought it over and redesigned those
> orom functions. This should make it simpler and more consistent.
Looks a lot nicer to me :)
Jes
> Thanks,
> Artur
>
> From 673ecf1c0539f0050cc5934203af6d79cd68234d Mon Sep 17 00:00:00 2001
> From: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
> Date: Fri, 27 Feb 2015 10:34:20 +0100
> Subject: [PATCH] imsm: simplified multiple OROMs support
>
> Replaced oroms array with list, add_orom() now only appends to this list
> and add_orom_device_id() only appends devid_list node to an orom_entry.
>
> Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
> ---
> platform-intel.c | 96 +++++++++++++++++++++++++++-----------------------------
> platform-intel.h | 4 ++-
> super-intel.c | 18 +++++------
> 3 files changed, 57 insertions(+), 61 deletions(-)
>
> diff --git a/platform-intel.c b/platform-intel.c
> index 37274da..9c89c20 100644
> --- a/platform-intel.c
> +++ b/platform-intel.c
> @@ -229,65 +229,61 @@ struct pciExpDataStructFormat {
> __u16 devListOffset;
> } __attribute__ ((packed));
>
> -static struct orom_entry oroms[SYS_DEV_MAX];
> -
> -const struct orom_entry *get_oroms(void)
> -{
> - return (const struct orom_entry *)&oroms;
> -}
> +struct orom_entry *orom_entries;
>
> const struct imsm_orom *get_orom_by_device_id(__u16 dev_id)
> {
> - int i;
> - struct devid_list *list;
> + struct orom_entry *entry;
> + struct devid_list *devid;
>
> - for (i = 0; i < SYS_DEV_MAX; i++) {
> - for (list = oroms[i].devid_list; list; list = list->next) {
> - if (list->devid == dev_id)
> - return &oroms[i].orom;
> + for (entry = orom_entries; entry; entry = entry->next) {
> + for (devid = entry->devid_list; devid; devid = devid->next) {
> + if (devid->devid == dev_id)
> + return &entry->orom;
> }
> }
> +
> return NULL;
> }
>
> -static const struct imsm_orom *add_orom(const struct imsm_orom *orom)
> +static struct orom_entry *add_orom(const struct imsm_orom *orom)
> {
> - int i;
> -
> - for (i = 0; i < SYS_DEV_MAX; i++) {
> - if (&oroms[i].orom == orom)
> - return orom;
> - if (oroms[i].orom.signature[0] == 0) {
> - oroms[i].orom = *orom;
> - return &oroms[i].orom;
> - }
> - }
> - return NULL;
> + struct orom_entry *list;
> + struct orom_entry *prev = NULL;
> +
> + for (list = orom_entries; list; prev = list, list = list->next)
> + ;
> +
> + list = xmalloc(sizeof(struct orom_entry));
> + list->orom = *orom;
> + list->devid_list = NULL;
> + list->next = NULL;
> +
> + if (prev == NULL)
> + orom_entries = list;
> + else
> + prev->next = list;
> +
> + return list;
> }
>
> -static void add_orom_device_id(const struct imsm_orom *orom, __u16 dev_id)
> +static void add_orom_device_id(struct orom_entry *entry, __u16 dev_id)
> {
> - int i;
> struct devid_list *list;
> struct devid_list *prev = NULL;
>
> - for (i = 0; i < SYS_DEV_MAX; i++) {
> - if (&oroms[i].orom == orom) {
> - for (list = oroms[i].devid_list; list; prev = list, list = list->next) {
> - if (list->devid == dev_id)
> - return;
> - }
> - list = xmalloc(sizeof(struct devid_list));
> - list->devid = dev_id;
> - list->next = NULL;
> -
> - if (prev == NULL)
> - oroms[i].devid_list = list;
> - else
> - prev->next = list;
> + for (list = entry->devid_list; list; prev = list, list = list->next) {
> + if (list->devid == dev_id)
> return;
> - }
> }
> + list = xmalloc(sizeof(struct devid_list));
> + list->devid = dev_id;
> + list->next = NULL;
> +
> + if (prev == NULL)
> + entry->devid_list = list;
> + else
> + prev->next = list;
> }
>
> static int scan(const void *start, const void *end, const void *data)
> @@ -321,7 +317,7 @@ static int scan(const void *start, const void *end, const void *data)
> if (!imsm_mem)
> return 0;
>
> - const struct imsm_orom *orom = add_orom(imsm_mem);
> + struct orom_entry *orom = add_orom(imsm_mem);
>
> if (ptr->devListOffset) {
> const __u16 *dev_list = (void *)ptr + ptr->devListOffset;
> @@ -367,11 +363,11 @@ const struct imsm_orom *imsm_platform_test(struct sys_dev *hba)
> IMSM_OROM_RLC_RAID10;
> }
>
> - const struct imsm_orom *ret = add_orom(&orom);
> + struct orom_entry *ret = add_orom(&orom);
>
> add_orom_device_id(ret, hba->dev_id);
>
> - return ret;
> + return &ret->orom;
> }
>
> static const struct imsm_orom *find_imsm_hba_orom(struct sys_dev *hba)
> @@ -508,7 +504,7 @@ static int read_efi_variable(void *buffer, ssize_t buf_size, char *variable_name
> const struct imsm_orom *find_imsm_efi(struct sys_dev *hba)
> {
> struct imsm_orom orom;
> - const struct imsm_orom *ret;
> + struct orom_entry *ret;
> int err;
>
> if (check_env("IMSM_TEST_AHCI_EFI") || check_env("IMSM_TEST_SCU_EFI"))
> @@ -529,14 +525,14 @@ const struct imsm_orom *find_imsm_efi(struct sys_dev *hba)
>
> /* try to read variable for combined AHCI controllers */
> if (err && hba->type == SYS_DEV_SATA) {
> - static const struct imsm_orom *csata;
> + static struct orom_entry *csata;
>
> err = read_efi_variable(&orom, sizeof(orom), AHCI_CSATA_PROP, VENDOR_GUID);
> if (!err) {
> if (!csata)
> csata = add_orom(&orom);
> add_orom_device_id(csata, hba->dev_id);
> - return csata;
> + return &csata->orom;
> }
> }
>
> @@ -546,12 +542,12 @@ const struct imsm_orom *find_imsm_efi(struct sys_dev *hba)
> ret = add_orom(&orom);
> add_orom_device_id(ret, hba->dev_id);
>
> - return ret;
> + return &ret->orom;
> }
>
> const struct imsm_orom *find_imsm_nvme(struct sys_dev *hba)
> {
> - static const struct imsm_orom *nvme_orom;
> + static struct orom_entry *nvme_orom;
>
> if (hba->type != SYS_DEV_NVME)
> return NULL;
> @@ -574,7 +570,7 @@ const struct imsm_orom *find_imsm_nvme(struct sys_dev *hba)
> nvme_orom = add_orom(&nvme_orom_compat);
> }
> add_orom_device_id(nvme_orom, hba->dev_id);
> - return nvme_orom;
> + return &nvme_orom->orom;
> }
>
> const struct imsm_orom *find_imsm_capability(struct sys_dev *hba)
> diff --git a/platform-intel.h b/platform-intel.h
> index 2ead431..631fa76 100644
> --- a/platform-intel.h
> +++ b/platform-intel.h
> @@ -213,8 +213,11 @@ struct devid_list {
> struct orom_entry {
> struct imsm_orom orom;
> struct devid_list *devid_list;
> + struct orom_entry *next;
> };
>
> +extern struct orom_entry *orom_entries;
> +
> static inline char *guid_str(char *buf, struct efi_guid guid)
> {
> sprintf(buf, "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
> @@ -235,6 +238,5 @@ int devt_attached_to_hba(dev_t dev, const char *hba_path);
> char *devt_to_devpath(dev_t dev);
> int path_attached_to_hba(const char *disk_path, const char *hba_path);
> const char *get_sys_dev_type(enum sys_dev_type);
> -const struct orom_entry * get_oroms(void);
> const struct imsm_orom *get_orom_by_device_id(__u16 device_id);
> struct sys_dev *device_by_id(__u16 device_id);
> diff --git a/super-intel.c b/super-intel.c
> index 819e0da..53269fd 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -1948,13 +1948,12 @@ static int detail_platform_imsm(int verbose, int enumerate_only, char *controlle
> return result;
> }
>
> - const struct orom_entry *oroms = get_oroms();
> - int i;
> + const struct orom_entry *entry;
>
> - for (i = 0; i < SYS_DEV_MAX && oroms[i].devid_list; i++) {
> - print_imsm_capability(&oroms[i].orom);
> + for (entry = orom_entries; entry; entry = entry->next) {
> + print_imsm_capability(&entry->orom);
>
> - if (imsm_orom_is_nvme(&oroms[i].orom)) {
> + if (imsm_orom_is_nvme(&entry->orom)) {
> for (hba = list; hba; hba = hba->next) {
> if (hba->type == SYS_DEV_NVME)
> printf(" NVMe Device : %s\n", hba->path);
> @@ -1963,7 +1962,7 @@ static int detail_platform_imsm(int verbose, int enumerate_only, char *controlle
> }
>
> struct devid_list *devid;
> - for (devid = oroms[i].devid_list; devid; devid = devid->next) {
> + for (devid = entry->devid_list; devid; devid = devid->next) {
> hba = device_by_id(devid->devid);
> if (!hba)
> continue;
> @@ -2007,11 +2006,10 @@ static int export_detail_platform_imsm(int verbose, char *controller_path)
> result = 0;
> }
>
> - const struct orom_entry *oroms = get_oroms();
> - int i;
> + const struct orom_entry *entry;
>
> - for (i = 0; i < SYS_DEV_MAX && oroms[i].devid_list; i++)
> - print_imsm_capability_export(&oroms[i].orom);
> + for (entry = orom_entries; entry; entry = entry->next)
> + print_imsm_capability_export(&entry->orom);
>
> return result;
> }
^ permalink raw reply
* raid0: "clean" state on drive failure/removal
From: Sushma Gurram @ 2015-02-27 20:54 UTC (permalink / raw)
To: linux-raid@vger.kernel.org
Hi,
A basic question on raid0 behavior.
When a drive fails/removed in a raid0 array, “mdadm –detail” reports the array
state as clean instead of failed.
It appears that the drive is whacked out of the array, but that array slot
continues to show “active sync” state.
I/O to the failed drive errors out. However, it appears that I/O to other drives
in the raid0 array would succeed. How would the user know about the array state
and potential data loss? Should the array information be cached, drive
failure/removals be monitored and checked against the cached information?
Could anyone please explain the reason for this behavior of not reporting a
raid0 array as failed?
Thanks,
Sushma
________________________________
PLEASE NOTE: The information contained in this electronic mail message is intended only for the use of the designated recipient(s) named above. If the reader of this message is not the intended recipient, you are hereby notified that you have received this message in error and that any review, dissemination, distribution, or copying of this message is strictly prohibited. If you have received this communication in error, please notify the sender by telephone or e-mail (as shown above) immediately and destroy any and all copies of this message in your possession (whether hard copies or electronically stored copies).
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: raid0: "clean" state on drive failure/removal
From: NeilBrown @ 2015-02-27 21:56 UTC (permalink / raw)
To: Sushma Gurram; +Cc: linux-raid@vger.kernel.org
In-Reply-To: <B9FF37F8E2D7894A9493A5314D0816FC242755A9@SACMBXIP01.sdcorp.global.sandisk.com>
[-- Attachment #1: Type: text/plain, Size: 2351 bytes --]
On Fri, 27 Feb 2015 20:54:32 +0000 Sushma Gurram <Sushma.Gurram@sandisk.com>
wrote:
> Hi,
>
> A basic question on raid0 behavior.
>
> When a drive fails/removed in a raid0 array, “mdadm –detail” reports the array
> state as clean instead of failed.
>
> It appears that the drive is whacked out of the array, but that array slot
> continues to show “active sync” state.
>
> I/O to the failed drive errors out. However, it appears that I/O to other drives
> in the raid0 array would succeed. How would the user know about the array state
> and potential data loss? Should the array information be cached, drive
> failure/removals be monitored and checked against the cached information?
The user would know about potential data loss in *exactly* they same way as
if half the sectors on a single hard-drive stopped working.
i.e. the filesystem would complain.
>
> Could anyone please explain the reason for this behavior of not reporting a
> raid0 array as failed?
RAID0 is not really RAID - there is no redundancy.
So it isn't treated like RAID.
RAID0 is simply a mapping off linear addresses onto multiple devices.
There is no sense in which the array as a whole "fails", or any value in
marking a device as "failed".
If there is something wrong, then IO requests will fail. Not necessarily all
IO requests, but some. Just like a normal drive that has partially failed.
NeilBrown
>
> Thanks,
> Sushma
>
> ________________________________
>
> PLEASE NOTE: The information contained in this electronic mail message is intended only for the use of the designated recipient(s) named above. If the reader of this message is not the intended recipient, you are hereby notified that you have received this message in error and that any review, dissemination, distribution, or copying of this message is strictly prohibited. If you have received this communication in error, please notify the sender by telephone or e-mail (as shown above) immediately and destroy any and all copies of this message in your possession (whether hard copies or electronically stored copies).
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* [PATCH md] recover_bitmaps() can be static
From: kbuild test robot @ 2015-02-27 23:04 UTC (permalink / raw)
To: Goldwyn Rodrigues; +Cc: kbuild-all, Neil Brown, linux-raid, linux-kernel
In-Reply-To: <201502280727.kOhvF4kI%fengguang.wu@intel.com>
drivers/md/md-cluster.c:190:6: sparse: symbol 'recover_bitmaps' was not declared. Should it be static?
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
md-cluster.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 1f82d0d..c71217a 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -187,7 +187,7 @@ out:
return s;
}
-void recover_bitmaps(struct md_thread *thread)
+static void recover_bitmaps(struct md_thread *thread)
{
struct mddev *mddev = thread->mddev;
struct md_cluster_info *cinfo = mddev->cluster_info;
^ permalink raw reply related
* [md:for-next 14/48] drivers/md/md-cluster.c:190:6: sparse: symbol 'recover_bitmaps' was not declared. Should it be static?
From: kbuild test robot @ 2015-02-27 23:04 UTC (permalink / raw)
To: Goldwyn Rodrigues; +Cc: kbuild-all, Neil Brown, linux-raid, linux-kernel
tree: git://neil.brown.name/md for-next
head: 5d98513a2e546337c0deb93d772eb0c988d47c1d
commit: e94987db2ed983aea4e45d22db9e17c6bbf2a623 [14/48] Initiate recovery on node failure
reproduce:
# apt-get install sparse
git checkout e94987db2ed983aea4e45d22db9e17c6bbf2a623
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
drivers/md/md-cluster.c:178:15: sparse: restricted __le64 degrades to integer
>> drivers/md/md-cluster.c:190:6: sparse: symbol 'recover_bitmaps' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
^ permalink raw reply
* [PATCH] Communication Framework: fix semicolon.cocci warnings
From: kbuild test robot @ 2015-02-28 1:16 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: kbuild-all, Lidong Zhong, Neil Brown, linux-raid, linux-kernel
In-Reply-To: <201502280906.22NYH7vD%fengguang.wu@intel.com>
drivers/md/md-cluster.c:328:2-3: Unneeded semicolon
Removes unneeded semicolon.
Generated by: scripts/coccinelle/misc/semicolon.cocci
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
md-cluster.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -325,7 +325,7 @@ static void process_recvd_msg(struct mdd
pr_info("%s: %d Received message: RESYNCING from %d\n",
__func__, __LINE__, msg->slot);
break;
- };
+ }
}
/*
^ permalink raw reply
* block devices loosing state after resume: trigger udev rules to re-apply settings
From: Chris @ 2015-02-28 8:30 UTC (permalink / raw)
To: 779412; +Cc: 725284, linux-raid, smartmontools-support
(http://bugs.debian.org/779412 explanation)
There is a general problem with non-permanent block devices settings
(hard disks, optical disks, usb storage, ...), that are not restored
when resuming from suspend (instead using factory defaults and
loosing all pre-suspend settings).
And as long as the ata/scsi command set drivers can not save and
restore every state register a device may have (impossible?),
systemd may ship a viable workaround for this:
A systemd unit file could trigger an udev change action upon resume for
block devices. This way the same udev rules that set up the devices when
they are first plugged, will re-apply their settings after resume.
Providing this centrally with the systemd package could avoid that
multiple packages ship their own files, resulting in multiple change
events triggerd on each resume.
Examples for very important (non-permanent) settings are with
hdparm (i.e. the important -B hard disk wear settings)
https://bugs.debian.org/725284
smartctl/mdadm/lvm/btrfs/zfs/... (i.e. set error recovery timeouts to
prevent controller resets and data loss)
http://sourceforge.net/p/smartmontools/mailman/message/33501936/
A draft for such a central systemd unit file:
[Unit]
Description=Trigger all block device udev rules on resume, to re-apply all non-permanent device settings (e.g. smartctl and hdparm rules).
After=suspend.target After=hibernate.target
After=hybrid-sleep.target
[Service]
Type=oneshot
ExecStart=/sbin/udevadm trigger --action=change --subsystem-match=block
[Install]
WantedBy=suspend.target
WantedBy=hibernate.target
WantedBy=hybrid-sleep.target
^ permalink raw reply
* Bug#725284: Bug#779412: block devices loosing state after resume: trigger udev rules to re-apply settings
From: Michael Biebl @ 2015-02-28 8:38 UTC (permalink / raw)
To: Chris, 779412; +Cc: 725284, linux-raid, smartmontools-support
In-Reply-To: <20150228093023.14e30d9a@smtp.arcor.de>
[-- Attachment #1: Type: text/plain, Size: 1693 bytes --]
Am 28.02.2015 um 09:30 schrieb Chris:
> (http://bugs.debian.org/779412 explanation)
>
> There is a general problem with non-permanent block devices settings
> (hard disks, optical disks, usb storage, ...), that are not restored
> when resuming from suspend (instead using factory defaults and
> loosing all pre-suspend settings).
>
> And as long as the ata/scsi command set drivers can not save and
> restore every state register a device may have (impossible?),
> systemd may ship a viable workaround for this:
>
> A systemd unit file could trigger an udev change action upon resume for
> block devices. This way the same udev rules that set up the devices when
> they are first plugged, will re-apply their settings after resume.
>
> Providing this centrally with the systemd package could avoid that
> multiple packages ship their own files, resulting in multiple change
> events triggerd on each resume.
>
> Examples for very important (non-permanent) settings are with
> hdparm (i.e. the important -B hard disk wear settings)
> https://bugs.debian.org/725284
> smartctl/mdadm/lvm/btrfs/zfs/... (i.e. set error recovery timeouts to
> prevent controller resets and data loss)
> http://sourceforge.net/p/smartmontools/mailman/message/33501936/
>
I don't think working around this in udev/systemd is a good idea.
After all, most of those custom settings aren't applied via udev rules
anyway. This should be fixed in the kernel properly (or the individual
services) and not be papered over in systemd.
Marco, what do you think?
--
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: Bug#779412: block devices loosing state after resume: trigger udev rules to re-apply settings
From: Chris @ 2015-02-28 9:47 UTC (permalink / raw)
To: Michael Biebl; +Cc: linux-raid, 725284, smartmontools-support, 779412
In-Reply-To: <54F17E83.5070400@debian.org>
Am Sat, 28 Feb 2015 09:38:27 +0100
schrieb Michael Biebl <biebl@debian.org>:
> I don't think working around this in udev/systemd is a good idea.
Idealy and in the long run, the kernel drivers should keep state, yes.
But until then, better not to make releases with default configurations
that deliver serious problems (excessive hardware wear, data loss) to
the users.
I believe before things stadardized around systemd and udev, packages
like hdparm, laptop-mode-tools, pm-utils, acpi-support,
gnome-power-manager, and more, all tried to work around problems with
block devices loosing state. Unfortunately, accumulating a large mess
and interferences resuling in releases with many bugs in this regard.
Now the situation can improve a lot, if we can say packages are safe if
they use udev rules to initialize devices. (As the kernel keeps
state, or systemd centrally triggers a udev change event where this is
not (yet) the case.)
> most of those custom settings aren't applied via udev rules
> anyway.
Which settings were you refering to?
With current versions hdparm, mdadm, etc. all seem to ship udev rules.
And that seems to be the proper way to configure the standard
hot-pluggable systems of today. (leaving aside embedded, non-systemd,
non udev systems)
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
^ permalink raw reply
* [GIT PULL REQUEST] md fixes for 4.0
From: NeilBrown @ 2015-03-02 0:03 UTC (permalink / raw)
To: Linus Torvalds
Cc: Eric Mei, Tomáš Hodek, Dark Penguin, linux RAID, lkml
[-- Attachment #1: Type: text/plain, Size: 1172 bytes --]
The following changes since commit c517d838eb7d07bbe9507871fab3931deccff539:
Linux 4.0-rc1 (2015-02-22 18:21:14 -0800)
are available in the git repository at:
git://neil.brown.name/md/ tags/md/4.0-fixes
for you to fetch changes up to 750f199ee8b578062341e6ddfe36c59ac8ff2dcb:
md: mark some attributes as pre-alloc (2015-02-25 11:38:46 +1100)
----------------------------------------------------------------
3 md fixes for 4.0
- fix a read-balance problem that was reported 2 years ago, but
that I never noticed the report :-(
- fix for rare RAID6 problem causing incorrect bitmap updates when
two devices fail.
- add __ATTR_PREALLOC annotation now that it is possible.
----------------------------------------------------------------
Eric Mei (1):
raid5: check faulty flag for array status during recovery.
NeilBrown (1):
md: mark some attributes as pre-alloc
Tomáš Hodek (1):
md/raid1: fix read balance when a drive is write-mostly.
drivers/md/md.c | 14 ++++++++------
drivers/md/raid1.c | 5 +++--
drivers/md/raid5.c | 13 +++++++++----
3 files changed, 20 insertions(+), 12 deletions(-)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Riad scrub generated errors, should I worry?
From: Wilson, Jonathan @ 2015-03-02 14:36 UTC (permalink / raw)
To: linux-raid
While the monthly scrub was running the following errors (at the bottom
of the post, copied from syslog) were issued.
I ran a full test smartctl -l selftest /dev/sdf and no errors were
found.
I did notice that the drive also upped its "raw read error rate" by a
value of 4 from zero, but has since issued no further errors or
problems.
> === START OF INFORMATION SECTION ===
> Model Family: Western Digital Red (AF)
> Device Model: WDC WD30EFRX-68EUZN0
> Serial Number: WD-WMC4N0990925
> LU WWN Device Id: 5 0014ee 603e8733c
> Firmware Version: 80.00A80
> User Capacity: 3,000,592,982,016 bytes [3.00 TB]
> Sector Sizes: 512 bytes logical, 4096 bytes physical
> Rotation Rate: 5400 rpm
> Device is: In smartctl database [for details use: -P show]
> ATA Version is: ACS-2 (minor revision not indicated)
> SATA Version is: SATA 3.0, 6.0 Gb/s (current: 6.0 Gb/s)
> Local Time is: Mon Mar 2 14:26:46 2015 GMT
> SMART support is: Available - device has SMART capability.
> SMART support is: Enabled
>
> === START OF READ SMART DATA SECTION ===
> SMART overall-health self-assessment test result: PASSED
>
> General SMART Values:
> Offline data collection status: (0x00) Offline data collection activity
> was never started.
> Auto Offline Data Collection: Disabled.
> Self-test execution status: ( 0) The previous self-test routine completed
> without error or no self-test has ever
> been run.
> Total time to complete Offline
> data collection: (40860) seconds.
> Offline data collection
> capabilities: (0x7b) SMART execute Offline immediate.
> Auto Offline data collection on/off support.
> Suspend Offline collection upon new
> command.
> Offline surface scan supported.
> Self-test supported.
> Conveyance Self-test supported.
> Selective Self-test supported.
> SMART capabilities: (0x0003) Saves SMART data before entering
> power-saving mode.
> Supports SMART auto save timer.
> Error logging capability: (0x01) Error logging supported.
> General Purpose Logging supported.
> Short self-test routine
> recommended polling time: ( 2) minutes.
> Extended self-test routine
> recommended polling time: ( 410) minutes.
> Conveyance self-test routine
> recommended polling time: ( 5) minutes.
> SCT capabilities: (0x703d) SCT Status supported.
> SCT Error Recovery Control supported.
> SCT Feature Control supported.
> SCT Data Table supported.
>
> SMART Attributes Data Structure revision number: 16
> Vendor Specific SMART Attributes with Thresholds:
> ID# ATTRIBUTE_NAME FLAG VALUE WORST THRESH TYPE UPDATED WHEN_FAILED RAW_VALUE
> 1 Raw_Read_Error_Rate 0x002f 200 200 051 Pre-fail Always - 4
> 3 Spin_Up_Time 0x0027 186 179 021 Pre-fail Always - 5683
> 4 Start_Stop_Count 0x0032 100 100 000 Old_age Always - 156
> 5 Reallocated_Sector_Ct 0x0033 200 200 140 Pre-fail Always - 0
> 7 Seek_Error_Rate 0x002e 200 200 000 Old_age Always - 0
> 9 Power_On_Hours 0x0032 085 085 000 Old_age Always - 10965
> 10 Spin_Retry_Count 0x0032 100 100 000 Old_age Always - 0
> 11 Calibration_Retry_Count 0x0032 100 100 000 Old_age Always - 0
> 12 Power_Cycle_Count 0x0032 100 100 000 Old_age Always - 156
> 192 Power-Off_Retract_Count 0x0032 200 200 000 Old_age Always - 91
> 193 Load_Cycle_Count 0x0032 199 199 000 Old_age Always - 4453
> 194 Temperature_Celsius 0x0022 118 111 000 Old_age Always - 32
> 196 Reallocated_Event_Count 0x0032 200 200 000 Old_age Always - 0
> 197 Current_Pending_Sector 0x0032 200 200 000 Old_age Always - 0
> 198 Offline_Uncorrectable 0x0030 100 253 000 Old_age Offline - 0
> 199 UDMA_CRC_Error_Count 0x0032 200 200 000 Old_age Always - 0
> 200 Multi_Zone_Error_Rate 0x0008 200 200 000 Old_age Offline - 0
>
> SMART Error Log Version: 1
> No Errors Logged
>
> SMART Self-test log structure revision number 1
> Num Test_Description Status Remaining LifeTime(hours) LBA_of_first_error
> # 1 Extended offline Completed without error 00% 10949 -
> # 2 Extended offline Completed without error 00% 5486 -
> # 3 Short offline Completed without error 00% 5478 -
>
> SMART Selective self-test log data structure revision number 1
> SPAN MIN_LBA MAX_LBA CURRENT_TEST_STATUS
> 1 0 0 Not_testing
> 2 0 0 Not_testing
> 3 0 0 Not_testing
> 4 0 0 Not_testing
> 5 0 0 Not_testing
> Selective self-test flags (0x0):
> After scanning selected spans, do NOT read-scan remainder of disk.
> If Selective self-test is pending on power-up, resume after 0 minute delay.
As you can see above the extended off line completed without error.
I'm guessing it might be a case of "watch and see" if more errors start
to happen then look at RMA'ing it? Or is the RRER a non
issue/manufacture specific doesn't mean much in its own rights value?
I also read that it can sometimes be caused by bad cables or noise bleed
from unshielded cables, but would that hold true when the device it self
knows a problem of some description related to it self has happened as
opposed to the kernel throwing a wobbler but the drive not reporting
anything. (I have had old/cheap sata cables cause intermittent kernel
floods along the line of "device ata, resetting, link down" but none of
these caused the drive it self to issue/know of any problems.
> 12:50:44 borgCube kernel: [253970.367307] ata6.00: exception Emask 0x0 SAct 0x1e00 SErr 0x0 action 0x0
> Mar 1 12:50:44 borgCube kernel: [253970.367309] ata6.00: irq_stat 0x40000008
> Mar 1 12:50:44 borgCube kernel: [253970.367311] ata6.00: failed command: READ FPDMA QUEUED
> Mar 1 12:50:44 borgCube kernel: [253970.367313] ata6.00: cmd 60/00:48:d0:b9:96/04:00:52:01:00/40 tag 9 ncq 524288 in
> Mar 1 12:50:44 borgCube kernel: [253970.367313] res 41/40:00:d8:bc:96/00:00:52:01:00/40 Emask 0x409 (media error) <F>
> Mar 1 12:50:44 borgCube kernel: [253970.367314] ata6.00: status: { DRDY ERR }
> Mar 1 12:50:44 borgCube kernel: [253970.367314] ata6.00: error: { UNC }
> Mar 1 12:50:44 borgCube kernel: [253970.368337] ata6.00: configured for UDMA/133
> Mar 1 12:50:44 borgCube kernel: [253970.368365] sd 5:0:0:0: [sdf] Unhandled sense code
> Mar 1 12:50:44 borgCube kernel: [253970.368366] sd 5:0:0:0: [sdf]
> Mar 1 12:50:44 borgCube kernel: [253970.368366] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
> Mar 1 12:50:44 borgCube kernel: [253970.368367] sd 5:0:0:0: [sdf]
> Mar 1 12:50:44 borgCube kernel: [253970.368368] Sense Key : Medium Error [current] [descriptor]
> Mar 1 12:50:44 borgCube kernel: [253970.368369] Descriptor sense data with sense descriptors (in hex):
> Mar 1 12:50:44 borgCube kernel: [253970.368369] 72 03 11 04 00 00 00 0c 00 0a 80 00 00 00 00 01
> Mar 1 12:50:44 borgCube kernel: [253970.368372] 52 96 bc d8
> Mar 1 12:50:44 borgCube kernel: [253970.368374] sd 5:0:0:0: [sdf]
> Mar 1 12:50:44 borgCube kernel: [253970.368375] Add. Sense: Unrecovered read error - auto reallocate failed
> Mar 1 12:50:44 borgCube kernel: [253970.368375] sd 5:0:0:0: [sdf] CDB:
> Mar 1 12:50:44 borgCube kernel: [253970.368376] Read(16): 88 00 00 00 00 01 52 96 b9 d0 00 00 04 00 00 00
> Mar 1 12:50:44 borgCube kernel: [253970.368380] end_request: I/O error, dev sdf, sector 5680577752
> Mar 1 12:50:44 borgCube kernel: [253970.368391] ata6: EH complete
> Mar 1 12:50:48 borgCube kernel: [253974.067305] ata6.00: exception Emask 0x0 SAct 0x10 SErr 0x0 action 0x0
> Mar 1 12:50:48 borgCube kernel: [253974.067307] ata6.00: irq_stat 0x40000008
> Mar 1 12:50:48 borgCube kernel: [253974.067309] ata6.00: failed command: READ FPDMA QUEUED
> Mar 1 12:50:48 borgCube kernel: [253974.067311] ata6.00: cmd 60/80:20:d8:bc:96/00:00:52:01:00/40 tag 4 ncq 65536 in
> Mar 1 12:50:48 borgCube kernel: [253974.067311] res 41/40:00:d8:bc:96/00:00:52:01:00/40 Emask 0x409 (media error) <F>
> Mar 1 12:50:48 borgCube kernel: [253974.067312] ata6.00: status: { DRDY ERR }
> Mar 1 12:50:48 borgCube kernel: [253974.067312] ata6.00: error: { UNC }
> Mar 1 12:50:48 borgCube kernel: [253974.068360] ata6.00: configured for UDMA/133
> Mar 1 12:50:48 borgCube kernel: [253974.068368] sd 5:0:0:0: [sdf] Unhandled sense code
> Mar 1 12:50:48 borgCube kernel: [253974.068369] sd 5:0:0:0: [sdf]
> Mar 1 12:50:48 borgCube kernel: [253974.068369] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
> Mar 1 12:50:48 borgCube kernel: [253974.068370] sd 5:0:0:0: [sdf]
> Mar 1 12:50:48 borgCube kernel: [253974.068371] Sense Key : Medium Error [current] [descriptor]
> Mar 1 12:50:48 borgCube kernel: [253974.068372] Descriptor sense data with sense descriptors (in hex):
> Mar 1 12:50:48 borgCube kernel: [253974.068373] 72 03 11 04 00 00 00 0c 00 0a 80 00 00 00 00 01
> Mar 1 12:50:48 borgCube kernel: [253974.068375] 52 96 bc d8
> Mar 1 12:50:48 borgCube kernel: [253974.068377] sd 5:0:0:0: [sdf]
> Mar 1 12:50:48 borgCube kernel: [253974.068378] Add. Sense: Unrecovered read error - auto reallocate failed
> Mar 1 12:50:48 borgCube kernel: [253974.068378] sd 5:0:0:0: [sdf] CDB:
> Mar 1 12:50:48 borgCube kernel: [253974.068379] Read(16): 88 00 00 00 00 01 52 96 bc d8 00 00 00 80 00 00
> Mar 1 12:50:48 borgCube kernel: [253974.068383] end_request: I/O error, dev sdf, sector 5680577752
> Mar 1 12:50:48 borgCube kernel: [253974.068392] ata6: EH complete
> Mar 1 12:50:48 borgCube kernel: [253974.238221] md/raid:md51: read error corrected (8 sectors at 5478837464 on sdf5)
> Mar 1 12:50:48 borgCube kernel: [253974.238226] md/raid:md51: read error corrected (8 sectors at 5478837472 on sdf5)
> Mar 1 12:50:48 borgCube kernel: [253974.238227] md/raid:md51: read error corrected (8 sectors at 5478837480 on sdf5)
> Mar 1 12:50:48 borgCube kernel: [253974.238228] md/raid:md51: read error corrected (8 sectors at 5478837488 on sdf5)
> Mar 1 12:50:48 borgCube kernel: [253974.238229] md/raid:md51: read error corrected (8 sectors at 5478837496 on sdf5)
> Mar 1 12:50:48 borgCube kernel: [253974.238231] md/raid:md51: read error corrected (8 sectors at 5478837504 on sdf5)
> Mar 1 12:50:48 borgCube kernel: [253974.238232] md/raid:md51: read error corrected (8 sectors at 5478837512 on sdf5)
> Mar 1 12:50:48 borgCube kernel: [253974.238233] md/raid:md51: read error corrected (8 sectors at 5478837520 on sdf5)
> Mar 1 12:50:48 borgCube kernel: [253974.238234] md/raid:md51: read error corrected (8 sectors at 5478837528 on sdf5)
> Mar 1 12:50:48 borgCube kernel: [253974.238235] md/raid:md51: read error corrected (8 sectors at 5478837536 on sdf5)
> Mar 1 12:50:52 borgCube kernel: [253977.979357] ata6.00: exception Emask 0x0 SAct 0x60 SErr 0x0 action 0x0
> Mar 1 12:50:52 borgCube kernel: [253977.979359] ata6.00: irq_stat 0x40000008
> Mar 1 12:50:52 borgCube kernel: [253977.979361] ata6.00: failed command: READ FPDMA QUEUED
> Mar 1 12:50:52 borgCube kernel: [253977.979364] ata6.00: cmd 60/00:28:d8:c1:96/04:00:52:01:00/40 tag 5 ncq 524288 in
> Mar 1 12:50:52 borgCube kernel: [253977.979364] res 41/40:00:18:c3:96/00:00:52:01:00/40 Emask 0x409 (media error) <F>
> Mar 1 12:50:52 borgCube kernel: [253977.979366] ata6.00: status: { DRDY ERR }
> Mar 1 12:50:52 borgCube kernel: [253977.979366] ata6.00: error: { UNC }
> Mar 1 12:50:52 borgCube kernel: [253977.980565] ata6.00: configured for UDMA/133
> Mar 1 12:50:52 borgCube kernel: [253977.980591] sd 5:0:0:0: [sdf] Unhandled sense code
> Mar 1 12:50:52 borgCube kernel: [253977.980592] sd 5:0:0:0: [sdf]
> Mar 1 12:50:52 borgCube kernel: [253977.980593] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
> Mar 1 12:50:52 borgCube kernel: [253977.980594] sd 5:0:0:0: [sdf]
> Mar 1 12:50:52 borgCube kernel: [253977.980595] Sense Key : Medium Error [current] [descriptor]
> Mar 1 12:50:52 borgCube kernel: [253977.980597] Descriptor sense data with sense descriptors (in hex):
> Mar 1 12:50:52 borgCube kernel: [253977.980598] 72 03 11 04 00 00 00 0c 00 0a 80 00 00 00 00 01
> Mar 1 12:50:52 borgCube kernel: [253977.980602] 52 96 c3 18
> Mar 1 12:50:52 borgCube kernel: [253977.980604] sd 5:0:0:0: [sdf]
> Mar 1 12:50:52 borgCube kernel: [253977.980605] Add. Sense: Unrecovered read error - auto reallocate failed
> Mar 1 12:50:52 borgCube kernel: [253977.980606] sd 5:0:0:0: [sdf] CDB:
> Mar 1 12:50:52 borgCube kernel: [253977.980607] Read(16): 88 00 00 00 00 01 52 96 c1 d8 00 00 04 00 00 00
> Mar 1 12:50:52 borgCube kernel: [253977.980612] end_request: I/O error, dev sdf, sector 5680579352
> Mar 1 12:50:52 borgCube kernel: [253977.980636] ata6: EH complete
> Mar 1 12:50:55 borgCube kernel: [253981.511344] ata6.00: exception Emask 0x0 SAct 0x3ff00 SErr 0x0 action 0x0
> Mar 1 12:50:55 borgCube kernel: [253981.511346] ata6.00: irq_stat 0x40000008
> Mar 1 12:50:55 borgCube kernel: [253981.511348] ata6.00: failed command: READ FPDMA QUEUED
> Mar 1 12:50:55 borgCube kernel: [253981.511350] ata6.00: cmd 60/80:40:18:c3:96/00:00:52:01:00/40 tag 8 ncq 65536 in
> Mar 1 12:50:55 borgCube kernel: [253981.511350] res 41/40:00:18:c3:96/00:00:52:01:00/40 Emask 0x409 (media error) <F>
> Mar 1 12:50:55 borgCube kernel: [253981.511351] ata6.00: status: { DRDY ERR }
> Mar 1 12:50:55 borgCube kernel: [253981.511351] ata6.00: error: { UNC }
> Mar 1 12:50:55 borgCube kernel: [253981.512557] ata6.00: configured for UDMA/133
> Mar 1 12:50:55 borgCube kernel: [253981.512567] sd 5:0:0:0: [sdf] Unhandled sense code
> Mar 1 12:50:55 borgCube kernel: [253981.512568] sd 5:0:0:0: [sdf]
> Mar 1 12:50:55 borgCube kernel: [253981.512569] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
> Mar 1 12:50:55 borgCube kernel: [253981.512570] sd 5:0:0:0: [sdf]
> Mar 1 12:50:55 borgCube kernel: [253981.512570] Sense Key : Medium Error [current] [descriptor]
> Mar 1 12:50:55 borgCube kernel: [253981.512572] Descriptor sense data with sense descriptors (in hex):
> Mar 1 12:50:55 borgCube kernel: [253981.512572] 72 03 11 04 00 00 00 0c 00 0a 80 00 00 00 00 01
> Mar 1 12:50:55 borgCube kernel: [253981.512575] 52 96 c3 18
> Mar 1 12:50:55 borgCube kernel: [253981.512576] sd 5:0:0:0: [sdf]
> Mar 1 12:50:55 borgCube kernel: [253981.512577] Add. Sense: Unrecovered read error - auto reallocate failed
> Mar 1 12:50:55 borgCube kernel: [253981.512578] sd 5:0:0:0: [sdf] CDB:
> Mar 1 12:50:55 borgCube kernel: [253981.512579] Read(16): 88 00 00 00 00 01 52 96 c3 18 00 00 00 80 00 00
> Mar 1 12:50:55 borgCube kernel: [253981.512582] end_request: I/O error, dev sdf, sector 5680579352
> Mar 1 12:50:55 borgCube kernel: [253981.512596] ata6: EH complete
> Mar 1 12:50:55 borgCube kernel: [253981.635156] raid5_end_read_request: 6 callbacks suppressed
> Mar 1 12:50:55 borgCube kernel: [253981.635163] md/raid:md51: read error corrected (8 sectors at 5478839064 on sdf5)
> Mar 1 12:50:55 borgCube kernel: [253981.635165] md/raid:md51: read error corrected (8 sectors at 5478839072 on sdf5)
> Mar 1 12:50:55 borgCube kernel: [253981.635167] md/raid:md51: read error corrected (8 sectors at 5478839080 on sdf5)
> Mar 1 12:50:55 borgCube kernel: [253981.635168] md/raid:md51: read error corrected (8 sectors at 5478839088 on sdf5)
> Mar 1 12:50:55 borgCube kernel: [253981.635170] md/raid:md51: read error corrected (8 sectors at 5478839096 on sdf5)
> Mar 1 12:50:55 borgCube kernel: [253981.635171] md/raid:md51: read error corrected (8 sectors at 5478839104 on sdf5)
> Mar 1 12:50:55 borgCube kernel: [253981.635173] md/raid:md51: read error corrected (8 sectors at 5478839112 on sdf5)
> Mar 1 12:50:55 borgCube kernel: [253981.635174] md/raid:md51: read error corrected (8 sectors at 5478839120 on sdf5)
> Mar 1 12:50:55 borgCube kernel: [253981.635176] md/raid:md51: read error corrected (8 sectors at 5478839128 on sdf5)
> Mar 1 12:50:55 borgCube kernel: [253981.635178] md/raid:md51: read error corrected (8 sectors at 5478839136 on sdf5)
> Mar 1 13:16:42 borgCube kernel: [255528.361600] md: md51: data-check done.
^ permalink raw reply
* Re: Riad scrub generated errors, should I worry?
From: Mikael Abrahamsson @ 2015-03-02 15:22 UTC (permalink / raw)
To: Wilson, Jonathan; +Cc: linux-raid
In-Reply-To: <BLU436-SMTP203777F3704141DF5C096B898100@phx.gbl>
On Mon, 2 Mar 2015, Wilson, Jonathan wrote:
> While the monthly scrub was running the following errors (at the bottom
> of the post, copied from syslog) were issued.
As soon as you get UNC, it's the drive reporting that it can't
successfully read a sector. Usually this sector is then reported as
"pending" in your SMART output.
Since the log you provided shows a lot of sectors being corrected and you
after that have 0 pending sectors on the drive, I'd say you are now fine.
I would run a new scrub manually in a few days just to check, but you
might be fine going forward. There is no really good way to know, but
generally, a drive that throws a bunch of UNC should be monitored so this
isn't becoming a common problem. I tend to replace drives that have thrown
these kinds of errors if it happens on any kind of regular basis.
--
Mikael Abrahamsson email: swmike@swm.pp.se
^ permalink raw reply
* Re: [PATCH 0/1] RFC: Use /dev/md/X as default name
From: Jes Sorensen @ 2015-03-02 16:32 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <20150219093236.25660678@notabene.brown>
NeilBrown <neilb@suse.de> writes:
> On Wed, 18 Feb 2015 17:11:33 -0500 Jes Sorensen <Jes.Sorensen@redhat.com>
> wrote:
>
>> NeilBrown <neilb@suse.de> writes:
>> > On Wed, 18 Feb 2015 13:12:09 -0500 Jes Sorensen <Jes.Sorensen@redhat.com>
>> > wrote:
>> >
>> >> Jes.Sorensen@redhat.com writes:
>> >> > From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> >> >
>> >> > I have received some issues for when creating an array using a
>> >> > /dev/mdX name, the matching symlink in /dev/md/X isn't
>> >> > created. Whereas if you create /dev/md/X, /dev/mdX is created
>> >> > automatically.
>> >> >
>> >> > I was trying to see if there was a better way of dealing with this,
>> >> > but I couldn't find one. If you have suggestions for a better solution
>> >> > I am all ears.
>> >> >
>> >> > Thoughts?
>> >>
>> >> Hi Neil,
>> >>
>> >> Any thoughts on this one?
>> >
>> > Thanks for the reminder....
>> >
>> > I'm not sure that I really see the problem.
>> >
>> > "I ask it to create /dev/mdX and it doesn't create /dev/md/X".
>> >
>> > Well ... no. You didn't ask it to. If you want it to create /dev/md/X,
>> > then ... ask it to.
>> >
>> > /dev/mdX is the canonical name. It always gets created.
>> > /dev/md/X is a convenient alias. It gets created if requested.
>> >
>> > Is there really a problem here worth solving?
>> >
>> > Maybe I missed something.
>>
>> I have had complaints in Fedora from the installer people that they rely
>> on the /dev/md/ name being created when they create a new device. It is
>> also inconsistent because /dev/md/<X> will be created if you run
>> 'mdadm -As' later on.
>
> If they rely on the /dev/md/ name being created, then surely they should ask
> for it to be created.
> Is it really harder to run "mdadm -C /dev/md/0" than "mdadm -C /dev/md0" ??
>
> If you create an array as "/dev/md0", then after subsequent "mdadm
> -As" /dev/md0 will exist.
> If you create an array as "/dev/md/0", then after subsequent mdadm
> -As", /dev/md/0 will exist.
>
> The fact that something unasked for also exists is a bonus.
>
>>
>> I don't see it as a major issue, but I can see why it is frustrating for
>> some and I think there is something to be said for being consistent in
>> behavior.
>
> I think the behaviour is perfectly consistent. It just follows rules that
> are slightly less trivial that some people appear to want.
>
> However....
> If you changed
>
> mp = map_by_uuid(&map, info->uuid);
> if (mp && mp->path &&
> strncmp(mp->path, "/dev/md/", 8) == 0) {
> printf("MD_DEVNAME=");
> print_escape(mp->path+8);
> putchar('\n');
> }
>
>
> in Detail.c so that when mp->path were "/dev/md0", MD_DEVNAME became "0",
> then you should get the result that you are after, and I probably wouldn't
> object to the patch.
Neil,
Finally had time to go back and look at this - I don't quite understand
your request here.
As far as I can see, the above code isn't run at all during device
creation, so I don't get how modifying it as you suggest will make
/dev/md/111 appear if I create /dev/md111?
Cheers,
Jes
^ permalink raw reply
* [PATCH] Fix stray --cluster-confrim crash
From: Goldwyn Rodrigues @ 2015-03-02 16:55 UTC (permalink / raw)
To: neilb; +Cc: linux-raid
Hi Neil,
This fix is againt the md/for-next.
A --cluster-confirm without an --add (by another node) can
crash the kernel.
Fix it by guarding it using a state.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 03e521a..96679b2 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -42,6 +42,10 @@ struct resync_info {
__le64 hi;
};
+/* md_cluster_info flags */
+#define MD_CLUSTER_WAITING_FOR_NEWDISK 1
+
+
struct md_cluster_info {
/* dlm lock space and resources for clustered raid. */
dlm_lockspace_t *lockspace;
@@ -61,6 +65,7 @@ struct md_cluster_info {
struct dlm_lock_resource *no_new_dev_lockres;
struct md_thread *recv_thread;
struct completion newdisk_completion;
+ unsigned long state;
};
enum msg_type {
@@ -380,9 +385,11 @@ static void process_add_new_disk(struct mddev *mddev, struct cluster_msg *cmsg)
snprintf(raid_slot, 16, "RAID_DISK=%d", cmsg->raid_slot);
pr_info("%s:%d Sending kobject change with %s and %s\n", __func__, __LINE__, disk_uuid, raid_slot);
init_completion(&cinfo->newdisk_completion);
+ set_bit(MD_CLUSTER_WAITING_FOR_NEWDISK, &cinfo->state);
kobject_uevent_env(&disk_to_dev(mddev->gendisk)->kobj, KOBJ_CHANGE, envp);
wait_for_completion_timeout(&cinfo->newdisk_completion,
NEW_DEV_TIMEOUT);
+ clear_bit(MD_CLUSTER_WAITING_FOR_NEWDISK, &cinfo->state);
}
@@ -832,13 +839,19 @@ static int add_new_disk_finish(struct mddev *mddev)
return ret;
}
-static void new_disk_ack(struct mddev *mddev, bool ack)
+static int new_disk_ack(struct mddev *mddev, bool ack)
{
struct md_cluster_info *cinfo = mddev->cluster_info;
+ if (!test_bit(MD_CLUSTER_WAITING_FOR_NEWDISK, &cinfo->state)) {
+ pr_warn("md-cluster(%s): Spurious cluster confirmation\n", mdname(mddev));
+ return -EINVAL;
+ }
+
if (ack)
dlm_unlock_sync(cinfo->no_new_dev_lockres);
complete(&cinfo->newdisk_completion);
+ return 0;
}
static struct md_cluster_operations cluster_ops = {
diff --git a/drivers/md/md-cluster.h b/drivers/md/md-cluster.h
index 60d7e58..7417133 100644
--- a/drivers/md/md-cluster.h
+++ b/drivers/md/md-cluster.h
@@ -21,7 +21,7 @@ struct md_cluster_operations {
int (*area_resyncing)(struct mddev *mddev, sector_t lo, sector_t hi);
int (*add_new_disk_start)(struct mddev *mddev, struct md_rdev *rdev);
int (*add_new_disk_finish)(struct mddev *mddev);
- void (*new_disk_ack)(struct mddev *mddev, bool ack);
+ int (*new_disk_ack)(struct mddev *mddev, bool ack);
};
#endif /* _MD_CLUSTER_H */
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 23784988..461024d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5757,7 +5755,7 @@ static int add_new_disk(struct mddev *mddev, mdu_disk_info_t *info)
if (mddev_is_clustered(mddev) &&
!(info->state & ((1 << MD_DISK_CLUSTER_ADD) | (1 << MD_DISK_CANDIDATE)))) {
- pr_err("%s: Cannot add to clustered mddev. Try --cluster-add\n",
+ pr_err("%s: Cannot add to clustered mddev.\n",
mdname(mddev));
return -EINVAL;
}
@@ -5855,7 +5853,11 @@ static int add_new_disk(struct mddev *mddev, mdu_disk_info_t *info)
if (info->state & (1 << MD_DISK_CANDIDATE)) {
/* Through --cluster-confirm */
set_bit(Candidate, &rdev->flags);
- md_cluster_ops->new_disk_ack(mddev, true);
+ err = md_cluster_ops->new_disk_ack(mddev, true);
+ if (err) {
+ export_rdev(rdev);
+ return err;
+ }
} else if (info->state & (1 << MD_DISK_CLUSTER_ADD)) {
/* --add initiated by this node */
err = md_cluster_ops->add_new_disk_start(mddev, rdev);
^ permalink raw reply related
* Re: Riad scrub generated errors, should I worry?
From: Thomas Fjellstrom @ 2015-03-02 17:43 UTC (permalink / raw)
To: Mikael Abrahamsson; +Cc: Wilson, Jonathan, linux-raid
In-Reply-To: <alpine.DEB.2.02.1503021618580.20507@uplift.swm.pp.se>
On Mon 02 Mar 2015 04:22:00 PM Mikael Abrahamsson wrote:
> On Mon, 2 Mar 2015, Wilson, Jonathan wrote:
> > While the monthly scrub was running the following errors (at the bottom
> > of the post, copied from syslog) were issued.
>
> As soon as you get UNC, it's the drive reporting that it can't
> successfully read a sector. Usually this sector is then reported as
> "pending" in your SMART output.
>
> Since the log you provided shows a lot of sectors being corrected and you
> after that have 0 pending sectors on the drive, I'd say you are now fine.
> I would run a new scrub manually in a few days just to check, but you
> might be fine going forward. There is no really good way to know, but
> generally, a drive that throws a bunch of UNC should be monitored so this
> isn't becoming a common problem. I tend to replace drives that have thrown
> these kinds of errors if it happens on any kind of regular basis.
Dumb question, but after pending, I assume they go into the reallocated
column? I think after a certain number of those, you should start thinking
about a replacement. Like with my recent issues, I had two drives with a few
too many reallocated sectors. One was over 16k and the other was over 32k.
They still "work", but I replaced them with WD Reds anyhow. Another drive
seemed to max out the start-stop count field at 65536. Hah. No more cheap
desktop seagates in raid for this fellow.
--
Thomas Fjellstrom
thomas@fjellstrom.ca
^ permalink raw reply
* Re: Riad scrub generated errors, should I worry?
From: Chris Murphy @ 2015-03-02 18:32 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <BLU436-SMTP203777F3704141DF5C096B898100@phx.gbl>
[253981.512570] sd 5:0:0:0: [sdf]
[253970.368375] Add. Sense: Unrecovered read error - auto reallocate failed
[253970.368380] end_request: I/O error, dev sdf, sector 5680577752
I'm confused. The above happens twice. So it seems clear the problem is
with /dev/sdf and sector 5680577752. Since it's an AF drive, technically
sectors 5680577752 - 5680577760 are affected, since those are the LBA's for
a single physical sector.
However, all of the "read error corrected" that follow have completely
different values, 5478837464 through 547883753.
And then 3 seconds later another read error at the same LBA:
[253977.980604] sd 5:0:0:0: [sdf]
253977.980605] Add. Sense: Unrecovered read error - auto reallocate failed
[253977.980612] end_request: I/O error, dev sdf, sector 5680579352
and 4 seconds later
[253981.512576] sd 5:0:0:0: [sdf]
[253981.512577] Add. Sense: Unrecovered read error - auto reallocate failed
[253981.512582] end_request: I/O error, dev sdf, sector 5680579352
And then "read error corrected" 5478839064 through 5478839136 which are
different than the first batch.
So there's a single LBA reported by libata as URE multiple times, each with
identical address. But then two corrected events, each with a different
range of sectors, neither of which match the URE address.
??
Chris Murphy
^ permalink raw reply
* Re: Riad scrub generated errors, should I worry?
From: Wilson, Jonathan @ 2015-03-02 19:45 UTC (permalink / raw)
To: Chris Murphy; +Cc: linux-raid
In-Reply-To: <CAJCQCtR1th-V67c7dwZxNBFqKZ=96sAVLzyp4zscO-wPv9jA9g@mail.gmail.com>
On Mon, 2015-03-02 at 11:32 -0700, Chris Murphy wrote:
> [253981.512570] sd 5:0:0:0: [sdf]
> [253970.368375] Add. Sense: Unrecovered read error - auto reallocate failed
> [253970.368380] end_request: I/O error, dev sdf, sector 5680577752
>
> I'm confused. The above happens twice. So it seems clear the problem is
> with /dev/sdf and sector 5680577752. Since it's an AF drive, technically
> sectors 5680577752 - 5680577760 are affected, since those are the LBA's for
> a single physical sector.
>
> However, all of the "read error corrected" that follow have completely
> different values, 5478837464 through 547883753.
>
> And then 3 seconds later another read error at the same LBA:
>
> [253977.980604] sd 5:0:0:0: [sdf]
> 253977.980605] Add. Sense: Unrecovered read error - auto reallocate failed
> [253977.980612] end_request: I/O error, dev sdf, sector 5680579352
>
> and 4 seconds later
>
> [253981.512576] sd 5:0:0:0: [sdf]
> [253981.512577] Add. Sense: Unrecovered read error - auto reallocate failed
> [253981.512582] end_request: I/O error, dev sdf, sector 5680579352
>
>
> And then "read error corrected" 5478839064 through 5478839136 which are
> different than the first batch.
>
> So there's a single LBA reported by libata as URE multiple times, each with
> identical address. But then two corrected events, each with a different
> range of sectors, neither of which match the URE address.
>
> ??
I have no idea about the differing sector locations, way beyond my
knowledge... however one thought did occur to me.
As the drives are WD reds with TLER enabled, as the drive realised that
an error occurred, instead of performing a few read tests and then
possibly a relocate or re write or what ever a drive may try... would
its first imperative be to "chuck the error out, let the OS/raid card
deal with it" which is why no pending or relocates or other errors
showed in the smartctl except the increase in the RRER to 4 prior to
running a smartctl scan. After the smartctl scan no values changed,
except for the addition of
> # 1 Extended offline Completed without error 00% 10949
> -
The messages about "read error corrected" were generated by mdadm (I'm
assuming given the text), and as you say the initial errors were
generated by libata (which I assume is the disk subsystem?) so perhaps
it has a different idea about sectors (logical v physical?) or sectors
within the raid device (the raid data location within the logical
partition within the raid member device?)
The numbers seem well off, 5680577752 (disk) v 5478837464-5478837536
(mdadm) so perhaps the mdadm figure is the sector within the raid member
within partition 5 within the disk sdf?
>
>
> Chris Murphy
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: Riad scrub generated errors, should I worry?
From: Phil Turmel @ 2015-03-02 21:09 UTC (permalink / raw)
To: thomas, Mikael Abrahamsson; +Cc: Wilson, Jonathan, linux-raid
In-Reply-To: <1786620.4mivBfObx3@balsa>
On 03/02/2015 12:43 PM, Thomas Fjellstrom wrote:
> On Mon 02 Mar 2015 04:22:00 PM Mikael Abrahamsson wrote:
>> On Mon, 2 Mar 2015, Wilson, Jonathan wrote:
>>> While the monthly scrub was running the following errors (at the bottom
>>> of the post, copied from syslog) were issued.
>>
>> As soon as you get UNC, it's the drive reporting that it can't
>> successfully read a sector. Usually this sector is then reported as
>> "pending" in your SMART output.
>>
>> Since the log you provided shows a lot of sectors being corrected and you
>> after that have 0 pending sectors on the drive, I'd say you are now fine.
>> I would run a new scrub manually in a few days just to check, but you
>> might be fine going forward. There is no really good way to know, but
>> generally, a drive that throws a bunch of UNC should be monitored so this
>> isn't becoming a common problem. I tend to replace drives that have thrown
>> these kinds of errors if it happens on any kind of regular basis.
>
> Dumb question, but after pending, I assume they go into the reallocated
> column? I think after a certain number of those, you should start thinking
> about a replacement. Like with my recent issues, I had two drives with a few
> too many reallocated sectors. One was over 16k and the other was over 32k.
> They still "work", but I replaced them with WD Reds anyhow. Another drive
> seemed to max out the start-stop count field at 65536. Hah. No more cheap
> desktop seagates in raid for this fellow.
If the URE was simply due to magnetic decay without actual damage, you
can expect MD to rewrite the sector and fix it. No more pending, no
relocation. If the spot on the media is truly failing, the rewrite and
recheck the drive does for pending sectors will expose the problem, and
the firmware will relocate.
Read errors like this are normal and expected. The drive data shows
10k+ hours of operation, so the honeymoon (no errors at all) is over.
Scrub weekly or monthly so these UREs don't accumulate and carry on.
When actual *relocations* climb into double digits, replace the drive.
HTH,
Phil
^ permalink raw reply
* Re: Riad scrub generated errors, should I worry?
From: Chris Murphy @ 2015-03-02 21:10 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <CAJCQCtTVA6ntASWFtWMw7ZEwu=8jH+UjvN8avPZ8jXZ1_4BQXg@mail.gmail.com>
This won't help me, but you should report kernel and mdadm version,
and include the mdadm -E and -D output for the array and one of the
drives. Someone who knows more about the details might know of some
obscure bug that explains this. More likely it's normal behavior and
I'm just not understanding why the two sets of values are off.
^ 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