* 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: [PATCH 3/5] add_orom(): Compare content of struct imsm_orom rather than pointers to it
From: Jes Sorensen @ 2015-02-25 17:15 UTC (permalink / raw)
To: Artur Paszkiewicz; +Cc: neilb, linux-raid
In-Reply-To: <54EDF91F.40200@intel.com>
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?
Cheers,
Jes
^ 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-25 16:32 UTC (permalink / raw)
To: Jes Sorensen; +Cc: neilb, linux-raid
In-Reply-To: <wrfjvbiq2mzo.fsf@redhat.com>
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?
Regards,
Artur
^ permalink raw reply
* Re: [PATCH 4/5] IncrementalScan(): Make sure 'st' is valid before dereferencing it
From: John Stoffel @ 2015-02-25 15:42 UTC (permalink / raw)
To: Jes Sorensen; +Cc: John Stoffel, neilb, artur.paszkiewicz, linux-raid
In-Reply-To: <wrfjioeq2e9p.fsf@redhat.com>
>>>>> "Jes" == Jes Sorensen <Jes.Sorensen@redhat.com> writes:
Jes> "John Stoffel" <john@stoffel.org> writes:
>>>>>>> "Jes" == Jes Sorensen <Jes.Sorensen@redhat.com> writes:
>>
Jes> From: Jes Sorensen <Jes.Sorensen@redhat.com>
Jes> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Jes> ---
Jes> Incremental.c | 2 +-
Jes> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
Jes> diff --git a/Incremental.c b/Incremental.c
Jes> index 87d9114..33c0d7f 100644
Jes> --- a/Incremental.c
Jes> +++ b/Incremental.c
Jes> @@ -1354,7 +1354,7 @@ restart:
Jes> if (st && st->ss->load_container)
Jes> ret = st->ss->load_container(st, mdfd, NULL);
Jes> close(mdfd);
Jes> - if (!ret && st->ss->container_content) {
Jes> + if (!ret && st && st->ss->container_content) {
Jes> if (map_lock(&map))
Jes> pr_err("failed to get exclusive lock on mapfile\n");
Jes> ret = Incremental_container(st, me->path, c, only);
Jes> --
Jes> 2.1.0
>>
>> Forgive my stupidity, but how does this really help anything? You
>> already did the check above for a valid 'st', and now you're just
>> repeating it. Maybe if needs to be more of:
>>
>> if (st) {
>> if (st->ss->load_container)
>> ret = st->ss->load_container(st,mdfd, NULL);
>> close(mdfd);
>> if (!ret && st->ss->container_content) {
>> .....
>> }
>> }
>>
>> but maybe I'm just missing something here.
Jes> Please look more carefully - the checks above are in place so 'st' is
Jes> only dereferenced if 'st is valid. The code does not bail out if
Jes> st = NULL.
Jes> Your example results in mdfd not getting closed if st == NULL.
Right, I agree my example isn't perfect either, but I was just
commenting more that I think the check for st not being NULL should be
performed once, instead of multiple times. The closing the mdfd is
just a detail of the structure of the code. It's a nitpick I agree...
^ permalink raw reply
* Re: [PATCH 4/5] IncrementalScan(): Make sure 'st' is valid before dereferencing it
From: Jes Sorensen @ 2015-02-25 15:37 UTC (permalink / raw)
To: John Stoffel; +Cc: neilb, artur.paszkiewicz, linux-raid
In-Reply-To: <21741.58242.604155.228473@quad.stoffel.home>
"John Stoffel" <john@stoffel.org> writes:
>>>>>> "Jes" == Jes Sorensen <Jes.Sorensen@redhat.com> writes:
>
> Jes> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> Jes> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> Jes> ---
> Jes> Incremental.c | 2 +-
> Jes> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Jes> diff --git a/Incremental.c b/Incremental.c
> Jes> index 87d9114..33c0d7f 100644
> Jes> --- a/Incremental.c
> Jes> +++ b/Incremental.c
> Jes> @@ -1354,7 +1354,7 @@ restart:
> Jes> if (st && st->ss->load_container)
> Jes> ret = st->ss->load_container(st, mdfd, NULL);
> Jes> close(mdfd);
> Jes> - if (!ret && st->ss->container_content) {
> Jes> + if (!ret && st && st->ss->container_content) {
> Jes> if (map_lock(&map))
> Jes> pr_err("failed to get exclusive lock on mapfile\n");
> Jes> ret = Incremental_container(st, me->path, c, only);
> Jes> --
> Jes> 2.1.0
>
> Forgive my stupidity, but how does this really help anything? You
> already did the check above for a valid 'st', and now you're just
> repeating it. Maybe if needs to be more of:
>
> if (st) {
> if (st->ss->load_container)
> ret = st->ss->load_container(st,mdfd, NULL);
> close(mdfd);
> if (!ret && st->ss->container_content) {
> .....
> }
> }
>
> but maybe I'm just missing something here.
Please look more carefully - the checks above are in place so 'st' is
only dereferenced if 'st is valid. The code does not bail out if
st = NULL.
Your example results in mdfd not getting closed if st == NULL.
Jes
^ permalink raw reply
* Re: [PATCH 4/5] IncrementalScan(): Make sure 'st' is valid before dereferencing it
From: John Stoffel @ 2015-02-25 15:00 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: neilb, artur.paszkiewicz, linux-raid
In-Reply-To: <1424811640-26569-5-git-send-email-Jes.Sorensen@redhat.com>
>>>>> "Jes" == Jes Sorensen <Jes.Sorensen@redhat.com> writes:
Jes> From: Jes Sorensen <Jes.Sorensen@redhat.com>
Jes> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Jes> ---
Jes> Incremental.c | 2 +-
Jes> 1 file changed, 1 insertion(+), 1 deletion(-)
Jes> diff --git a/Incremental.c b/Incremental.c
Jes> index 87d9114..33c0d7f 100644
Jes> --- a/Incremental.c
Jes> +++ b/Incremental.c
Jes> @@ -1354,7 +1354,7 @@ restart:
Jes> if (st && st->ss->load_container)
Jes> ret = st->ss->load_container(st, mdfd, NULL);
Jes> close(mdfd);
Jes> - if (!ret && st->ss->container_content) {
Jes> + if (!ret && st && st->ss->container_content) {
Jes> if (map_lock(&map))
Jes> pr_err("failed to get exclusive lock on mapfile\n");
Jes> ret = Incremental_container(st, me->path, c, only);
Jes> --
Jes> 2.1.0
Forgive my stupidity, but how does this really help anything? You
already did the check above for a valid 'st', and now you're just
repeating it. Maybe if needs to be more of:
if (st) {
if (st->ss->load_container)
ret = st->ss->load_container(st,mdfd, NULL);
close(mdfd);
if (!ret && st->ss->container_content) {
.....
}
}
but maybe I'm just missing something here.
John
^ permalink raw reply
* Re: udev rules and scripts (erc timeout fix)
From: Chris @ 2015-02-25 14:37 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150222112309.7fa20ad0@smtp.arcor.de>
[-- Attachment #1: Type: text/plain, Size: 2379 bytes --]
Hello,
managed to test and bugfix the udev rules.
Simply dropping all files into /etc/udev/rules.d/ is enough, and
an "udevadm test /block/<drive_dev>/<partition_dev>" (dryrun) for a
raid member seems to run ok and tells me it would execute the
appropriate script.
However, for me, un- and re-plugging a device still does not seem to
execute the script. The _possibly-redundant-disk.sh currently logs
into /tmp/timeout-tst, but udev events don't appear there.
If you have the possiblity to test this in your /etc/udev/rules.d/
setup, it would be much appreciated.
Cheers,
Chris
# /etc/udev/rules.d/99-test_mdadm_smartctl-timeouts.rules
SUBSYSTEM!="block", GOTO="md_inc_end"
# handle potential components of arrays (the ones supported by md)
ENV{ID_FS_TYPE}=="linux_raid_member", GOTO="md_inc"
# "noiswmd" on kernel command line stops mdadm from handling
# "isw" (aka IMSM - Intel RAID).
# "nodmraid" on kernel command line stops mdadm from handling
# "isw" or "ddf".
IMPORT{cmdline}="noiswmd"
IMPORT{cmdline}="nodmraid"
ENV{nodmraid}=="?*", GOTO="md_inc_end"
ENV{ID_FS_TYPE}=="ddf_raid_member", GOTO="md_inc"
ENV{noiswmd}=="?*", GOTO="md_inc_end"
ENV{ID_FS_TYPE}=="isw_raid_member", GOTO="md_inc"
GOTO="md_inc_end"
LABEL="md_inc"
# initialize redundancy possibility status
# (only the kernel module could set actual run-time state, and may in the future
# set a dynamic FASTFAIL kernel device property instead of calling smartctl-timeout scripts)
IMPORT{program}="/sbin/mdadm --examine --export $tempnode"
ENV{MD_LEVEL}=="raid[1-9]*", ENV{REDUNDANT_DEV}="possibly"
ENV{MD_LEVEL}=="raid0", ENV{REDUNDANT_DEV}="false"
LABEL="md_inc_end"
# call initial HDD error correction timeouts adjustment
ENV{DEVTYPE}=="partition", ENV{REDUNDANT_DEV}=="possibly", TEST=="/usr/sbin/smartctl", RUN+="/etc/udev/rules.d/smartctl-timeouts_possibly-redundant-partition.sh $parent"
ENV{DEVTYPE}=="partition", ENV{REDUNDANT_DEV}=="false|", TEST=="/usr/sbin/smartctl", RUN+="/etc/udev/rules.d/smartctl-timeouts_non-redundant-partition.sh $parent"
ENV{DEVTYPE}=="disk", ENV{REDUNDANT_DEV}=="possibly", TEST=="/usr/sbin/smartctl", RUN+="/etc/udev/rules.d/smartctl-timeouts_posibly-redundant-disk.sh $devnode"
ENV{DEVTYPE}=="disk", ENV{REDUNDANT_DEV}=="false", TEST=="/usr/sbin/smartctl", RUN+="/etc/udev/rules.d/smartctl-timeouts_non-redundant-disk.sh $devnode"
[-- Attachment #2: smartctl-timeouts_udevadm-test.tar.gz --]
[-- Type: application/x-gzip, Size: 5448 bytes --]
^ permalink raw reply
* Re: [PATCH 3/5] add_orom(): Compare content of struct imsm_orom rather than pointers to it
From: Jes Sorensen @ 2015-02-25 12:29 UTC (permalink / raw)
To: Artur Paszkiewicz; +Cc: neilb, linux-raid
In-Reply-To: <54EDA91E.5050005@intel.com>
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;
}
Cheers,
Jes
^ 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-25 10:51 UTC (permalink / raw)
To: Jes.Sorensen, neilb; +Cc: linux-raid
In-Reply-To: <1424811640-26569-4-git-send-email-Jes.Sorensen@redhat.com>
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
^ permalink raw reply
* Re: [dm-devel] [PATCH 0/3] md raid: enhancements to support the device mapper dm-raid target
From: Heinz Mauelshagen @ 2015-02-25 10:10 UTC (permalink / raw)
To: NeilBrown
Cc: device-mapper development, jbras >> Brassow Jonathan,
linux RAID
In-Reply-To: <20150225091243.33fdd5f6@notabene.brown>
On 02/24/2015 11:12 PM, NeilBrown wrote:
> On Mon, 23 Feb 2015 12:49:00 +0100 Heinz Mauelshagen <heinzm@redhat.com>
> wrote:
>
>> Sure.
>> I'd like to see the raid0 conditonal request queue patch though.
>>
>
> ok, I've applied that one. It should turn up in my -next by the end of the
> week.
Thanks for the heads up.
>
> I'll look forward to your other results.
Ok, will inform when I have something working.
Cheers,
Heinz
>
> Thanks,
> NeilBrown
>
^ permalink raw reply
* Re: [PATCH 2/5] Check return of stat() to avoid covscan complaining
From: Jes Sorensen @ 2015-02-25 0:13 UTC (permalink / raw)
To: NeilBrown; +Cc: artur.paszkiewicz, linux-raid
In-Reply-To: <20150225090301.5158921d@notabene.brown>
NeilBrown <neilb@suse.de> writes:
> On Tue, 24 Feb 2015 16:56:29 -0500 Jes Sorensen <Jes.Sorensen@redhat.com>
> wrote:
>
>> NeilBrown <neilb@suse.de> writes:
>> > On Tue, 24 Feb 2015 16:00:37 -0500 Jes.Sorensen@redhat.com wrote:
>> >
>> >> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> >>
>> >> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>> >> ---
>> >> Assemble.c | 6 +++++-
>> >> 1 file changed, 5 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/Assemble.c b/Assemble.c
>> >> index 131f871..b392214 100644
>> >> --- a/Assemble.c
>> >> +++ b/Assemble.c
>> >> @@ -688,7 +688,11 @@ static int load_devices(struct devs *devices, char *devmap,
>> >> close(dfd);
>> >> }
>> >>
>> >> - stat(devname, &stb);
>> >> + if (stat(devname, &stb)) {
>> >> + pr_err("Unsable to stat(%s) - skipping device.\n",
>> >> + devname);
>> >> + continue;
>> >> + }
>> >>
>> >> if (c->verbose > 0)
>> >> pr_err("%s is identified as a member of %s, slot %d%s.\n",
>> >
>> > I've applied the other 4. I think I'd rather this one was fixed by changing
>> > stat(devname,
>> > to
>> > fstat(dfd,
>> > and keep dfd open a bit longer.
>> >
>> > Does this look OK to you?
>>
>> I got the warning from covscan because we ignored the return value from
>> stat, so I think you still need to check the return value from fstat()
>> as well.
>
> I hope not.
> You can only get errors from fstat if you do something stupid like passing
> NULL as the stat pointer, or passing a non-open file descriptior.
> So if covscan complains, then covscan is broken.
>
> In contrast, stat can certainly given an error, such a ENOENT, which cannot
> possibly be avoided by not being stupid.
Hmmm OK you got a point, even if the file is removed, it shouldn't
disappear until all users close it.
Cheers,
Jes
^ permalink raw reply
* Re: [dm-devel] [PATCH 0/3] md raid: enhancements to support the device mapper dm-raid target
From: NeilBrown @ 2015-02-24 22:12 UTC (permalink / raw)
To: Heinz Mauelshagen
Cc: device-mapper development, jbras >> Brassow Jonathan,
linux RAID
In-Reply-To: <54EB13AC.4010006@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 321 bytes --]
On Mon, 23 Feb 2015 12:49:00 +0100 Heinz Mauelshagen <heinzm@redhat.com>
wrote:
> Sure.
> I'd like to see the raid0 conditonal request queue patch though.
>
ok, I've applied that one. It should turn up in my -next by the end of the
week.
I'll look forward to your other results.
Thanks,
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 2/5] Check return of stat() to avoid covscan complaining
From: NeilBrown @ 2015-02-24 22:03 UTC (permalink / raw)
To: Jes Sorensen; +Cc: artur.paszkiewicz, linux-raid
In-Reply-To: <wrfj61ar55yq.fsf@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1674 bytes --]
On Tue, 24 Feb 2015 16:56:29 -0500 Jes Sorensen <Jes.Sorensen@redhat.com>
wrote:
> NeilBrown <neilb@suse.de> writes:
> > On Tue, 24 Feb 2015 16:00:37 -0500 Jes.Sorensen@redhat.com wrote:
> >
> >> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> >>
> >> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> >> ---
> >> Assemble.c | 6 +++++-
> >> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Assemble.c b/Assemble.c
> >> index 131f871..b392214 100644
> >> --- a/Assemble.c
> >> +++ b/Assemble.c
> >> @@ -688,7 +688,11 @@ static int load_devices(struct devs *devices, char *devmap,
> >> close(dfd);
> >> }
> >>
> >> - stat(devname, &stb);
> >> + if (stat(devname, &stb)) {
> >> + pr_err("Unsable to stat(%s) - skipping device.\n",
> >> + devname);
> >> + continue;
> >> + }
> >>
> >> if (c->verbose > 0)
> >> pr_err("%s is identified as a member of %s, slot %d%s.\n",
> >
> > I've applied the other 4. I think I'd rather this one was fixed by changing
> > stat(devname,
> > to
> > fstat(dfd,
> > and keep dfd open a bit longer.
> >
> > Does this look OK to you?
>
> I got the warning from covscan because we ignored the return value from
> stat, so I think you still need to check the return value from fstat()
> as well.
I hope not.
You can only get errors from fstat if you do something stupid like passing
NULL as the stat pointer, or passing a non-open file descriptior.
So if covscan complains, then covscan is broken.
In contrast, stat can certainly given an error, such a ENOENT, which cannot
possibly be avoided by not being stupid.
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 2/5] Check return of stat() to avoid covscan complaining
From: Jes Sorensen @ 2015-02-24 21:56 UTC (permalink / raw)
To: NeilBrown; +Cc: artur.paszkiewicz, linux-raid
In-Reply-To: <20150225081243.1fe91420@notabene.brown>
NeilBrown <neilb@suse.de> writes:
> On Tue, 24 Feb 2015 16:00:37 -0500 Jes.Sorensen@redhat.com wrote:
>
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>
>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>> ---
>> Assemble.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/Assemble.c b/Assemble.c
>> index 131f871..b392214 100644
>> --- a/Assemble.c
>> +++ b/Assemble.c
>> @@ -688,7 +688,11 @@ static int load_devices(struct devs *devices, char *devmap,
>> close(dfd);
>> }
>>
>> - stat(devname, &stb);
>> + if (stat(devname, &stb)) {
>> + pr_err("Unsable to stat(%s) - skipping device.\n",
>> + devname);
>> + continue;
>> + }
>>
>> if (c->verbose > 0)
>> pr_err("%s is identified as a member of %s, slot %d%s.\n",
>
> I've applied the other 4. I think I'd rather this one was fixed by changing
> stat(devname,
> to
> fstat(dfd,
> and keep dfd open a bit longer.
>
> Does this look OK to you?
I got the warning from covscan because we ignored the return value from
stat, so I think you still need to check the return value from fstat()
as well.
Cheers,
Jes
>
> Thanks,
> NeilBrown
>
> diff --git a/Assemble.c b/Assemble.c
> index 131f871a6d1e..1e529c1b3126 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -576,13 +576,13 @@ static int load_devices(struct devs *devices, char *devmap,
> struct stat stb;
> struct supertype *tst;
> int i;
> + int dfd;
>
> if (tmpdev->used != 1)
> continue;
> /* looks like a good enough match to update the super block if needed */
> #ifndef MDASSEMBLE
> if (c->update) {
> - int dfd;
> /* prepare useful information in info structures */
> struct stat stb2;
> int err;
> @@ -652,7 +652,6 @@ static int load_devices(struct devs *devices, char *devmap,
> if (tst->ss->store_super(tst, dfd))
> pr_err("Could not re-write superblock on %s.\n",
> devname);
> - close(dfd);
>
> if (strcmp(c->update, "uuid")==0 &&
> ident->bitmap_fd >= 0 && !bitmap_done) {
> @@ -666,9 +665,9 @@ static int load_devices(struct devs *devices, char *devmap,
> } else
> #endif
> {
> - int dfd = dev_open(devname,
> - tmpdev->disposition == 'I'
> - ? O_RDWR : (O_RDWR|O_EXCL));
> + dfd = dev_open(devname,
> + tmpdev->disposition == 'I'
> + ? O_RDWR : (O_RDWR|O_EXCL));
> tst = dup_super(st);
>
> if (dfd < 0 || tst->ss->load_super(tst, dfd, NULL) != 0) {
> @@ -685,10 +684,10 @@ static int load_devices(struct devs *devices, char *devmap,
> return -1;
> }
> tst->ss->getinfo_super(tst, content, devmap + devcnt * content->array.raid_disks);
> - close(dfd);
> }
>
> - stat(devname, &stb);
> + fstat(dfd, &stb);
> + close(dfd);
>
> if (c->verbose > 0)
> pr_err("%s is identified as a member of %s, slot %d%s.\n",
^ permalink raw reply
* Re: [PATCH 2/5] Check return of stat() to avoid covscan complaining
From: NeilBrown @ 2015-02-24 21:12 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: artur.paszkiewicz, linux-raid
In-Reply-To: <1424811640-26569-3-git-send-email-Jes.Sorensen@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2608 bytes --]
On Tue, 24 Feb 2015 16:00:37 -0500 Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
> Assemble.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Assemble.c b/Assemble.c
> index 131f871..b392214 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -688,7 +688,11 @@ static int load_devices(struct devs *devices, char *devmap,
> close(dfd);
> }
>
> - stat(devname, &stb);
> + if (stat(devname, &stb)) {
> + pr_err("Unsable to stat(%s) - skipping device.\n",
> + devname);
> + continue;
> + }
>
> if (c->verbose > 0)
> pr_err("%s is identified as a member of %s, slot %d%s.\n",
I've applied the other 4. I think I'd rather this one was fixed by changing
stat(devname,
to
fstat(dfd,
and keep dfd open a bit longer.
Does this look OK to you?
Thanks,
NeilBrown
diff --git a/Assemble.c b/Assemble.c
index 131f871a6d1e..1e529c1b3126 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -576,13 +576,13 @@ static int load_devices(struct devs *devices, char *devmap,
struct stat stb;
struct supertype *tst;
int i;
+ int dfd;
if (tmpdev->used != 1)
continue;
/* looks like a good enough match to update the super block if needed */
#ifndef MDASSEMBLE
if (c->update) {
- int dfd;
/* prepare useful information in info structures */
struct stat stb2;
int err;
@@ -652,7 +652,6 @@ static int load_devices(struct devs *devices, char *devmap,
if (tst->ss->store_super(tst, dfd))
pr_err("Could not re-write superblock on %s.\n",
devname);
- close(dfd);
if (strcmp(c->update, "uuid")==0 &&
ident->bitmap_fd >= 0 && !bitmap_done) {
@@ -666,9 +665,9 @@ static int load_devices(struct devs *devices, char *devmap,
} else
#endif
{
- int dfd = dev_open(devname,
- tmpdev->disposition == 'I'
- ? O_RDWR : (O_RDWR|O_EXCL));
+ dfd = dev_open(devname,
+ tmpdev->disposition == 'I'
+ ? O_RDWR : (O_RDWR|O_EXCL));
tst = dup_super(st);
if (dfd < 0 || tst->ss->load_super(tst, dfd, NULL) != 0) {
@@ -685,10 +684,10 @@ static int load_devices(struct devs *devices, char *devmap,
return -1;
}
tst->ss->getinfo_super(tst, content, devmap + devcnt * content->array.raid_disks);
- close(dfd);
}
- stat(devname, &stb);
+ fstat(dfd, &stb);
+ close(dfd);
if (c->verbose > 0)
pr_err("%s is identified as a member of %s, slot %d%s.\n",
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply related
* [PATCH 5/5] write_super_imsm_spares(): C statements are terminated by ;
From: Jes.Sorensen @ 2015-02-24 21:00 UTC (permalink / raw)
To: neilb; +Cc: artur.paszkiewicz, linux-raid, Jes Sorensen
In-Reply-To: <1424811640-26569-1-git-send-email-Jes.Sorensen@redhat.com>
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
super-intel.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 819e0da..7f75b53 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -5115,13 +5115,13 @@ static int write_super_imsm_spares(struct intel_super *super, int doclose)
__u32 sum;
struct dl *d;
- spare->mpb_size = __cpu_to_le32(sizeof(struct imsm_super)),
- spare->generation_num = __cpu_to_le32(1UL),
+ spare->mpb_size = __cpu_to_le32(sizeof(struct imsm_super));
+ spare->generation_num = __cpu_to_le32(1UL);
spare->attributes = MPB_ATTRIB_CHECKSUM_VERIFY;
- spare->num_disks = 1,
- spare->num_raid_devs = 0,
- spare->cache_size = mpb->cache_size,
- spare->pwr_cycle_count = __cpu_to_le32(1),
+ spare->num_disks = 1;
+ spare->num_raid_devs = 0;
+ spare->cache_size = mpb->cache_size;
+ spare->pwr_cycle_count = __cpu_to_le32(1);
snprintf((char *) spare->sig, MAX_SIGNATURE_LENGTH,
MPB_SIGNATURE MPB_VERSION_RAID0);
--
2.1.0
^ permalink raw reply related
* [PATCH 4/5] IncrementalScan(): Make sure 'st' is valid before dereferencing it
From: Jes.Sorensen @ 2015-02-24 21:00 UTC (permalink / raw)
To: neilb; +Cc: artur.paszkiewicz, linux-raid, Jes Sorensen
In-Reply-To: <1424811640-26569-1-git-send-email-Jes.Sorensen@redhat.com>
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
Incremental.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Incremental.c b/Incremental.c
index 87d9114..33c0d7f 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -1354,7 +1354,7 @@ restart:
if (st && st->ss->load_container)
ret = st->ss->load_container(st, mdfd, NULL);
close(mdfd);
- if (!ret && st->ss->container_content) {
+ if (!ret && st && st->ss->container_content) {
if (map_lock(&map))
pr_err("failed to get exclusive lock on mapfile\n");
ret = Incremental_container(st, me->path, c, only);
--
2.1.0
^ permalink raw reply related
* [PATCH 3/5] add_orom(): Compare content of struct imsm_orom rather than pointers to it
From: Jes.Sorensen @ 2015-02-24 21:00 UTC (permalink / raw)
To: neilb; +Cc: artur.paszkiewicz, linux-raid, Jes Sorensen
In-Reply-To: <1424811640-26569-1-git-send-email-Jes.Sorensen@redhat.com>
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;
--
2.1.0
^ permalink raw reply related
* [PATCH 2/5] Check return of stat() to avoid covscan complaining
From: Jes.Sorensen @ 2015-02-24 21:00 UTC (permalink / raw)
To: neilb; +Cc: artur.paszkiewicz, linux-raid, Jes Sorensen
In-Reply-To: <1424811640-26569-1-git-send-email-Jes.Sorensen@redhat.com>
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
Assemble.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/Assemble.c b/Assemble.c
index 131f871..b392214 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -688,7 +688,11 @@ static int load_devices(struct devs *devices, char *devmap,
close(dfd);
}
- stat(devname, &stb);
+ if (stat(devname, &stb)) {
+ pr_err("Unsable to stat(%s) - skipping device.\n",
+ devname);
+ continue;
+ }
if (c->verbose > 0)
pr_err("%s is identified as a member of %s, slot %d%s.\n",
--
2.1.0
^ permalink raw reply related
* [PATCH 1/5] Grow.c: Fix classic readlink() buffer overflow
From: Jes.Sorensen @ 2015-02-24 21:00 UTC (permalink / raw)
To: neilb; +Cc: artur.paszkiewicz, linux-raid, Jes Sorensen
In-Reply-To: <1424811640-26569-1-git-send-email-Jes.Sorensen@redhat.com>
From: Jes Sorensen <Jes.Sorensen@redhat.com>
The buffer passed on to readlink() needs to contain space for the
terminating \0. See 'man 3 readlink' for details.
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
Grow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Grow.c b/Grow.c
index b78d063..a8bbf2b 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3319,7 +3319,7 @@ started:
bul = make_backup(sra->sys_name);
if (bul) {
char buf[1024];
- int l = readlink(bul, buf, sizeof(buf));
+ int l = readlink(bul, buf, sizeof(buf) - 1);
if (l > 0) {
buf[l]=0;
unlink(buf);
--
2.1.0
^ permalink raw reply related
* [PATCH 0/5] Fix issues reported by covscan and newer GCC
From: Jes.Sorensen @ 2015-02-24 21:00 UTC (permalink / raw)
To: neilb; +Cc: artur.paszkiewicz, linux-raid, Jes Sorensen
From: Jes Sorensen <Jes.Sorensen@redhat.com>
I had some errors thrown at me by covscan and GCC 4.9.2 which prompted
some furthe inspection. In particular patch 3 could result in a local
stack variable passed back to the calling function, and patch 5 with
code not being executed as expected.
Please have a look.
Cheers,
Jes
Jes Sorensen (5):
Grow.c: Fix classic readlink() buffer overflow
Check return of stat() to avoid covscan complaining
add_orom(): Compare content of struct imsm_orom rather than pointers
to it
IncrementalScan(): Make sure 'st' is valid before dereferencing it
write_super_imsm_spares(): C statements are terminated by ;
Assemble.c | 6 +++++-
Grow.c | 2 +-
Incremental.c | 2 +-
platform-intel.c | 4 ++--
super-intel.c | 12 ++++++------
5 files changed, 15 insertions(+), 11 deletions(-)
--
2.1.0
^ permalink raw reply
* Re: An old "write-mostly" read balance issue
From: Tomáš Hodek @ 2015-02-24 8:20 UTC (permalink / raw)
To: NeilBrown, Dark Penguin; +Cc: linux-raid
In-Reply-To: <20150223110332.4c135de9@notabene.brown>
Dne 23.2.2015 v 01:03 NeilBrown napsal(a):
> Hi,
> thanks for reporting this. It is definitely a bug. It was introduced by
>
>
Hello,
Thank you that you have marked a current write-mostly behaviour as the bug.
If you think that I can help you, please tell me.
Best regards,
Tomas
^ permalink raw reply
* AW: RAID6 write I/O amplification?
From: Markus Stockhausen @ 2015-02-24 6:29 UTC (permalink / raw)
To: Roman Mamedov, linux-raid@vger.kernel.org
In-Reply-To: <20150224045835.14e40dcb@natsu>
[-- Attachment #1: Type: text/plain, Size: 1850 bytes --]
> 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. That said, you can only
reduce the I/O overhead by writing data that is larger than your configured
stripe size (e.g. 64K).
Looking at Neils development GIT you will find patches that allow
read-modify-write cycles for RAID6. So we only need the old block, the
old parities, recaluclate them and write the new block and the new parities.
In your case that would reduce the I/Os to 3x4KB read + 3x4KB write.
See http://git.neil.brown.name/?p=md.git;a=shortlog;h=refs/heads/devel
I posted them 6 months ago but they did not made their way into the
stable tree. Additionally it conatins patches to batch adjacent writes
to be processed in less & larger I/Os. Currently Linux Raid will break
each operation into 4K I/Os.
Markus
=
[-- Attachment #2: InterScan_Disclaimer.txt --]
[-- Type: text/plain, Size: 1650 bytes --]
****************************************************************************
Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte
Informationen. Wenn Sie nicht der richtige Adressat sind oder diese E-Mail
irrtümlich erhalten haben, informieren Sie bitte sofort den Absender und
vernichten Sie diese Mail. Das unerlaubte Kopieren sowie die unbefugte
Weitergabe dieser Mail ist nicht gestattet.
Über das Internet versandte E-Mails können unter fremden Namen erstellt oder
manipuliert werden. Deshalb ist diese als E-Mail verschickte Nachricht keine
rechtsverbindliche Willenserklärung.
Collogia
Unternehmensberatung AG
Ubierring 11
D-50678 Köln
Vorstand:
Kadir Akin
Dr. Michael Höhnerbach
Vorsitzender des Aufsichtsrates:
Hans Kristian Langva
Registergericht: Amtsgericht Köln
Registernummer: HRB 52 497
This e-mail may contain confidential and/or privileged information. If you
are not the intended recipient (or have received this e-mail in error)
please notify the sender immediately and destroy this e-mail. Any
unauthorized copying, disclosure or distribution of the material in this
e-mail is strictly forbidden.
e-mails sent over the internet may have been written under a wrong name or
been manipulated. That is why this message sent as an e-mail is not a
legally binding declaration of intention.
Collogia
Unternehmensberatung AG
Ubierring 11
D-50678 Köln
executive board:
Kadir Akin
Dr. Michael Höhnerbach
President of the supervisory board:
Hans Kristian Langva
Registry office: district court Cologne
Register number: HRB 52 497
****************************************************************************
^ permalink raw reply
* RAID6 write I/O amplification?
From: Roman Mamedov @ 2015-02-23 23:58 UTC (permalink / raw)
To: linux-raid
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?
--
With respect,
Roman
^ permalink raw reply
* Inject I/O latency for RAID5/6 read and writes
From: Alireza Haghdoost @ 2015-02-23 18:32 UTC (permalink / raw)
To: Linux RAID; +Cc: Neil Brown
I needed to inject I/O completion latency in the RAID5/6 codes for
test purpose. I was wondering where would be the good place in
md/raid5.c code to add delay ?
So far I have tried adding mdelay/udelay in raid5_end_write_request()
and it seems it works to increase I/O completion of writes. However,
adding delay in raid5_end_read_request() does not really change the
read I/O latency. Any idea ?
--Alireza
^ 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