* Re: Bit-Rot
From: Gandalf Corvotempesta @ 2017-03-17 16:59 UTC (permalink / raw)
To: Brassow Jonathan
Cc: Pasi Kärkkäinen, Mikael Abrahamsson, Anthony Youngman,
linux-raid
In-Reply-To: <BD49CADF-6076-44CF-8291-DFE74EA2C6A7@redhat.com>
2017-03-17 16:37 GMT+01:00 Brassow Jonathan <jbrassow@redhat.com>:
> There is a device-mapper target that is designed to do precisely this - dm-integrity (see dm-devel mailing list). It currently being developed as part of an authenticated encryption project, but could be used for this too. Note that there is a performance penalty that comes from emulating this.
Probably something similar could be obtained by checking, during a
scrub, the majority of responses from all replica
A sort of quorum
If you have a 3 way mirror, and 2 disks reply with "1" and another
reply with "0", the disk with "0" has triggered a bit rot
Is mdadm able to make this decision? In a 2 way mirror would be
impossible, as you can't know which disk has the correct data, but in
a 3 way mirrors you have a majority.
Probably the same could be done in RAID-6, where you have 2 parity to evaluate.
^ permalink raw reply
* [PATCH 2/2] super1: check and output faulty dev role
From: Gioh Kim @ 2017-03-17 16:47 UTC (permalink / raw)
To: jes.sorensen; +Cc: neilb, linux-raid, linux-kernel, Jack Wang
In-Reply-To: <1489769222-31855-1-git-send-email-gi-oh.kim@profitbricks.com>
From: Jack Wang <jinpu.wang@profitbricks.com>
Output the real dev role in examine_super1, it will help to
find problem.
Signed-off-by: Jack Wang <jinpu.wang@profitbricks.com>
Reviewed-by: Gioh Kim <gi-oh.kim@profitbricks.com>
---
super1.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/super1.c b/super1.c
index 1da33ef..0bf4715 100644
--- a/super1.c
+++ b/super1.c
@@ -501,8 +501,10 @@ static void examine_super1(struct supertype *st, char *homehost)
#endif
printf(" Device Role : ");
role = role_from_sb(sb);
- if (role >= MD_DISK_ROLE_FAULTY)
- printf("spare\n");
+ if (role == MD_DISK_ROLE_SPARE)
+ printf("Spare\n");
+ else if (role == MD_DISK_ROLE_FAULTY)
+ printf("Faulty\n");
else if (role == MD_DISK_ROLE_JOURNAL)
printf("Journal\n");
else if (sb->feature_map & __cpu_to_le32(MD_FEATURE_REPLACEMENT))
--
2.5.0
^ permalink raw reply related
* [PATCH 1/2] super1: ignore failfast flag for setting device role
From: Gioh Kim @ 2017-03-17 16:47 UTC (permalink / raw)
To: jes.sorensen; +Cc: neilb, linux-raid, linux-kernel, Gioh Kim, Jack Wang
In-Reply-To: <1489769222-31855-1-git-send-email-gi-oh.kim@profitbricks.com>
There is corner case for setting device role,
if new device has failfast flag.
The failfast flag should be ignored.
Signed-off-by: Gioh Kim <gi-oh.kim@profitbricks.com>
Signed-off-by: Jack Wang <jinpu.wang@profitbricks.com>
---
super1.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/super1.c b/super1.c
index 882cd61..1da33ef 100644
--- a/super1.c
+++ b/super1.c
@@ -1491,6 +1491,7 @@ static int add_to_super1(struct supertype *st, mdu_disk_info_t *dk,
struct devinfo *di, **dip;
bitmap_super_t *bms = (bitmap_super_t*)(((char*)sb) + MAX_SB_SIZE);
int rv, lockid;
+ int dk_state;
if (bms->version == BITMAP_MAJOR_CLUSTERED && dlm_funs_ready()) {
rv = cluster_get_dlmlock(&lockid);
@@ -1501,11 +1502,12 @@ static int add_to_super1(struct supertype *st, mdu_disk_info_t *dk,
}
}
- if ((dk->state & 6) == 6) /* active, sync */
+ dk_state &= ~(1<<MD_DISK_FAILFAST);
+ if ((dk_state & 6) == 6) /* active, sync */
*rp = __cpu_to_le16(dk->raid_disk);
- else if (dk->state & (1<<MD_DISK_JOURNAL))
+ else if (dk_state & (1<<MD_DISK_JOURNAL))
*rp = MD_DISK_ROLE_JOURNAL;
- else if ((dk->state & ~2) == 0) /* active or idle -> spare */
+ else if ((dk_state & ~2) == 0) /* active or idle -> spare */
*rp = MD_DISK_ROLE_SPARE;
else
*rp = MD_DISK_ROLE_FAULTY;
--
2.5.0
^ permalink raw reply related
* [PATCH 0/2] mdadm: setting device role of raid1 disk with failfast
From: Gioh Kim @ 2017-03-17 16:47 UTC (permalink / raw)
To: jes.sorensen; +Cc: neilb, linux-raid, linux-kernel, Gioh Kim
Hi,
I've found a case that failfast option of mdadm set a disk faulty wrongly.
Following is my test case.
mdadm --create /dev/md100 -l 1 --failfast -e 1.2 -n 2 /dev/vdb /dev/vdc
mdadm /dev/md100 -a --failfast /dev/vdd
If I use failfast option, the vdd disk was faulty wrongly.
If not, it was spare.
This patch fixes a corner case for setting device role and
prints device role if it's faulty.
This patch is based on "mdadm - v4.0-8-g72b616a - 2017-03-07".
Gioh Kim (1):
super1: ignore failfast flag for setting device role
Jack Wang (1):
super1: check and output faulty dev role
super1.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
--
2.5.0
^ permalink raw reply
* Re: Bit-Rot
From: Brassow Jonathan @ 2017-03-17 15:37 UTC (permalink / raw)
To: Pasi Kärkkäinen
Cc: Mikael Abrahamsson, Gandalf Corvotempesta, Anthony Youngman,
linux-raid
In-Reply-To: <20170306115648.GG7497@reaktio.net>
> On Mar 6, 2017, at 5:56 AM, Pasi Kärkkäinen <pasik@iki.fi> wrote:
>
> On Sun, Mar 05, 2017 at 09:15:39AM +0100, Mikael Abrahamsson wrote:
>> On Fri, 3 Mar 2017, Gandalf Corvotempesta wrote:
>>
>>> This is what ZFS does. This is what Gluster does. This is what BRTFS does.
>>> Adding this in mdadm could be an interesting feature.
>>
>> This has been discussed several times. Yes, it would be interesting.
>> It's not easy to do because mdadm maps 4k blocks to 4k blocks. Only
>> way to "easily" add this I imagine, would be to have an additional
>> "checksum" block, so that raid6 would require 3 extra drives instead
>> of 2.
>>
>> The answer historically has been "patches welcome".
>>
>
> There was/is an early prototype implementation of checksums for Linux MD RAID:
>
> http://pages.cs.wisc.edu/~bpkroth/cs736/md-checksums/
> http://pages.cs.wisc.edu/~bpkroth/cs736/md-checksums/md-checksums-presentation.pdf
> http://pages.cs.wisc.edu/~bpkroth/cs736/md-checksums/md-checksums-paper.pdf
> http://pages.cs.wisc.edu/~bpkroth/cs736/md-checksums/md-checksums-code.tar.bz2
>
>
> Also there's the T10 DIF / DIX (Data Integrity Fields / Data Integrity eXtensions) functionality that could be used, at least if the hardware setup is SAS-based (SAS HBA + enterprise SAS disks and modern enough firmware on both that enable DIF/DIX..).
>
> I guess MD RAID could also 'emulate' T10 DIF/DIX even if the HBA/disks don't support it.. but dunno if that makes any sense.
There is a device-mapper target that is designed to do precisely this - dm-integrity (see dm-devel mailing list). It currently being developed as part of an authenticated encryption project, but could be used for this too. Note that there is a performance penalty that comes from emulating this.
brassow
^ permalink raw reply
* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Peter Zijlstra @ 2017-03-17 12:44 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild, LKML,
linux-crypto, linux-raid
In-Reply-To: <CAG_fn=XcQLtcHRSm2BHYce9LMtyFJ+fWxfE4vYPCw+7U0DcrmQ@mail.gmail.com>
On Fri, Mar 17, 2017 at 01:31:23PM +0100, Alexander Potapenko wrote:
> On Fri, Mar 17, 2017 at 1:08 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Thu, Mar 16, 2017 at 05:15:19PM -0700, Michael Davidson wrote:
> >> Replace a variable length array in a struct by allocating
> >> the memory for the entire struct in a char array on the stack.
> >>
> >> Signed-off-by: Michael Davidson <md@google.com>
> >> ---
> >> drivers/md/raid10.c | 9 ++++-----
> >> 1 file changed, 4 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> >> index 063c43d83b72..158ebdff782c 100644
> >> --- a/drivers/md/raid10.c
> >> +++ b/drivers/md/raid10.c
> >> @@ -4654,11 +4654,10 @@ static int handle_reshape_read_error(struct mddev *mddev,
> >> /* Use sync reads to get the blocks from somewhere else */
> >> int sectors = r10_bio->sectors;
> >> struct r10conf *conf = mddev->private;
> >> - struct {
> >> - struct r10bio r10_bio;
> >> - struct r10dev devs[conf->copies];
> >> - } on_stack;
> >> - struct r10bio *r10b = &on_stack.r10_bio;
> >> + char on_stack_r10_bio[sizeof(struct r10bio) +
> >> + conf->copies * sizeof(struct r10dev)]
> >> + __aligned(__alignof__(struct r10bio));
> >> + struct r10bio *r10b = (struct r10bio *)on_stack_r10_bio;
> >> int slot = 0;
> >> int idx = 0;
> >> struct bio_vec *bvec = r10_bio->master_bio->bi_io_vec;
> >
> >
> > That's disgusting. Why not fix LLVM to support this?
>
> IIUC there's only a handful of VLAIS instances in LLVM code, why not
> just drop them for the sake of better code portability?
> (To quote Linus, "this feature is an abomination":
> https://lkml.org/lkml/2013/9/23/500)
Be that as it may; what you construct above is disgusting. Surely the
code can be refactored to not look like dog vomit?
Also; its not immediately obvious conf->copies is 'small' and this
doesn't blow up the stack; I feel that deserves a comment somewhere.
^ permalink raw reply
* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Alexander Potapenko @ 2017-03-17 12:32 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild, LKML,
linux-crypto, linux-raid
In-Reply-To: <CAG_fn=XcQLtcHRSm2BHYce9LMtyFJ+fWxfE4vYPCw+7U0DcrmQ@mail.gmail.com>
On Fri, Mar 17, 2017 at 1:31 PM, Alexander Potapenko <glider@google.com> wrote:
> On Fri, Mar 17, 2017 at 1:08 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>> On Thu, Mar 16, 2017 at 05:15:19PM -0700, Michael Davidson wrote:
>>> Replace a variable length array in a struct by allocating
>>> the memory for the entire struct in a char array on the stack.
>>>
>>> Signed-off-by: Michael Davidson <md@google.com>
>>> ---
>>> drivers/md/raid10.c | 9 ++++-----
>>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>>> index 063c43d83b72..158ebdff782c 100644
>>> --- a/drivers/md/raid10.c
>>> +++ b/drivers/md/raid10.c
>>> @@ -4654,11 +4654,10 @@ static int handle_reshape_read_error(struct mddev *mddev,
>>> /* Use sync reads to get the blocks from somewhere else */
>>> int sectors = r10_bio->sectors;
>>> struct r10conf *conf = mddev->private;
>>> - struct {
>>> - struct r10bio r10_bio;
>>> - struct r10dev devs[conf->copies];
>>> - } on_stack;
>>> - struct r10bio *r10b = &on_stack.r10_bio;
>>> + char on_stack_r10_bio[sizeof(struct r10bio) +
>>> + conf->copies * sizeof(struct r10dev)]
>>> + __aligned(__alignof__(struct r10bio));
>>> + struct r10bio *r10b = (struct r10bio *)on_stack_r10_bio;
>>> int slot = 0;
>>> int idx = 0;
>>> struct bio_vec *bvec = r10_bio->master_bio->bi_io_vec;
>>
>>
>> That's disgusting. Why not fix LLVM to support this?
>
> IIUC there's only a handful of VLAIS instances in LLVM code, why not
Sorry, "kernel code", not "LLVM code".
> just drop them for the sake of better code portability?
> (To quote Linus, "this feature is an abomination":
> https://lkml.org/lkml/2013/9/23/500)
>
> --
> Alexander Potapenko
> Software Engineer
>
> Google Germany GmbH
> Erika-Mann-Straße, 33
> 80636 München
>
> Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply
* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Alexander Potapenko @ 2017-03-17 12:31 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild, LKML,
linux-crypto, linux-raid
In-Reply-To: <20170317120837.pr74cv3xuj7qpoin@hirez.programming.kicks-ass.net>
On Fri, Mar 17, 2017 at 1:08 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Mar 16, 2017 at 05:15:19PM -0700, Michael Davidson wrote:
>> Replace a variable length array in a struct by allocating
>> the memory for the entire struct in a char array on the stack.
>>
>> Signed-off-by: Michael Davidson <md@google.com>
>> ---
>> drivers/md/raid10.c | 9 ++++-----
>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index 063c43d83b72..158ebdff782c 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -4654,11 +4654,10 @@ static int handle_reshape_read_error(struct mddev *mddev,
>> /* Use sync reads to get the blocks from somewhere else */
>> int sectors = r10_bio->sectors;
>> struct r10conf *conf = mddev->private;
>> - struct {
>> - struct r10bio r10_bio;
>> - struct r10dev devs[conf->copies];
>> - } on_stack;
>> - struct r10bio *r10b = &on_stack.r10_bio;
>> + char on_stack_r10_bio[sizeof(struct r10bio) +
>> + conf->copies * sizeof(struct r10dev)]
>> + __aligned(__alignof__(struct r10bio));
>> + struct r10bio *r10b = (struct r10bio *)on_stack_r10_bio;
>> int slot = 0;
>> int idx = 0;
>> struct bio_vec *bvec = r10_bio->master_bio->bi_io_vec;
>
>
> That's disgusting. Why not fix LLVM to support this?
IIUC there's only a handful of VLAIS instances in LLVM code, why not
just drop them for the sake of better code portability?
(To quote Linus, "this feature is an abomination":
https://lkml.org/lkml/2013/9/23/500)
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply
* [PATCH 1/3] mdadm: Replace snprintf with strncpy at some places to avoid truncation
From: Xiao Ni @ 2017-03-17 12:18 UTC (permalink / raw)
To: linux-raid; +Cc: Jes.Sorensen, ncroxon, artur.paszkiewicz
In gcc7 there are some building errors like:
directive output may be truncated writing up to 31 bytes into a region of size 24
snprintf(str, MPB_SIG_LEN, %s, mpb->sig);
It just need to copy one string to target. So use strncpy to replace it.
Signed-off-by: Xiao Ni <xni@redhat.com>
---
super-intel.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index d5e9517..e1618f1 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1811,7 +1811,8 @@ static void examine_super_imsm(struct supertype *st, char *homehost)
__u32 reserved = imsm_reserved_sectors(super, super->disks);
struct dl *dl;
- snprintf(str, MPB_SIG_LEN, "%s", mpb->sig);
+ strncpy(str, (char *)mpb->sig, MPB_SIG_LEN);
+ str[MPB_SIG_LEN-1] = '\0';
printf(" Magic : %s\n", str);
snprintf(str, strlen(MPB_VERSION_RAID0), "%s", get_imsm_version(mpb));
printf(" Version : %s\n", get_imsm_version(mpb));
@@ -5227,7 +5228,7 @@ static int init_super_imsm_volume(struct supertype *st, mdu_array_info_t *info,
disk->status = CONFIGURED_DISK | FAILED_DISK;
disk->scsi_id = __cpu_to_le32(~(__u32)0);
snprintf((char *) disk->serial, MAX_RAID_SERIAL_LEN,
- "missing:%d", i);
+ "missing:%d", (__u8)i);
}
find_missing(super);
} else {
@@ -7142,14 +7143,16 @@ static int update_subarray_imsm(struct supertype *st, char *subarray,
u->type = update_rename_array;
u->dev_idx = vol;
- snprintf((char *) u->name, MAX_RAID_SERIAL_LEN, "%s", name);
+ strncpy((char *) u->name, name, MAX_RAID_SERIAL_LEN);
+ u->name[MAX_RAID_SERIAL_LEN-1] = '\0';
append_metadata_update(st, u, sizeof(*u));
} else {
struct imsm_dev *dev;
int i;
dev = get_imsm_dev(super, vol);
- snprintf((char *) dev->volume, MAX_RAID_SERIAL_LEN, "%s", name);
+ strncpy((char *) dev->volume, name, MAX_RAID_SERIAL_LEN);
+ dev->volume[MAX_RAID_SERIAL_LEN-1] = '\0';
for (i = 0; i < mpb->num_raid_devs; i++) {
dev = get_imsm_dev(super, i);
handle_missing(super, dev);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 5/7] x86, boot, LLVM: Use regparm=0 for memcpy and memset
From: Peter Zijlstra @ 2017-03-17 12:08 UTC (permalink / raw)
To: Michael Davidson
Cc: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild, linux-kernel,
linux-crypto, linux-raid
In-Reply-To: <20170317001520.85223-6-md@google.com>
On Thu, Mar 16, 2017 at 05:15:18PM -0700, Michael Davidson wrote:
> Use the standard regparm=0 calling convention for memcpy and
> memset when building with clang.
>
> This is a work around for a long standing clang bug
> (see https://llvm.org/bugs/show_bug.cgi?id=3997) where
> clang always uses the standard regparm=0 calling convention
> for any implcit calls to memcpy and memset that it generates
> (eg for structure assignments and initialization) even if an
> alternate calling convention such as regparm=3 has been specified.
Seriously, fix LLVM already.
^ permalink raw reply
* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Peter Zijlstra @ 2017-03-17 12:08 UTC (permalink / raw)
To: Michael Davidson
Cc: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild, linux-kernel,
linux-crypto, linux-raid
In-Reply-To: <20170317001520.85223-7-md@google.com>
On Thu, Mar 16, 2017 at 05:15:19PM -0700, Michael Davidson wrote:
> Replace a variable length array in a struct by allocating
> the memory for the entire struct in a char array on the stack.
>
> Signed-off-by: Michael Davidson <md@google.com>
> ---
> drivers/md/raid10.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 063c43d83b72..158ebdff782c 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -4654,11 +4654,10 @@ static int handle_reshape_read_error(struct mddev *mddev,
> /* Use sync reads to get the blocks from somewhere else */
> int sectors = r10_bio->sectors;
> struct r10conf *conf = mddev->private;
> - struct {
> - struct r10bio r10_bio;
> - struct r10dev devs[conf->copies];
> - } on_stack;
> - struct r10bio *r10b = &on_stack.r10_bio;
> + char on_stack_r10_bio[sizeof(struct r10bio) +
> + conf->copies * sizeof(struct r10dev)]
> + __aligned(__alignof__(struct r10bio));
> + struct r10bio *r10b = (struct r10bio *)on_stack_r10_bio;
> int slot = 0;
> int idx = 0;
> struct bio_vec *bvec = r10_bio->master_bio->bi_io_vec;
That's disgusting. Why not fix LLVM to support this?
^ permalink raw reply
* [PATCH 3/3] mdadm: Specify enough length when write to buffer
From: Xiao Ni @ 2017-03-17 11:55 UTC (permalink / raw)
To: linux-raid; +Cc: Jes.Sorensen, ncroxon, artur.paszkiewicz
In-Reply-To: <1489751743-11397-1-git-send-email-xni@redhat.com>
In Detail.c the buffer path in function Detail is defined as path[200],
in fact the max lenth of content which needs to write to the buffer is
287. Because the length of dname of struct dirent is 255.
During building it reports error:
error: ‘%s’ directive writing up to 255 bytes into a region of size 189
[-Werror=format-overflow=]
In function examine_super0 there is a buffer nb with length 5.
But it need to show a int type argument. The lenght of max
number of int is 10. So the buffer length should be 11.
In human_size function the length of buf is 30. During building
there is a error:
output between 20 and 47 bytes into a destination of size 30.
Change the length to 47.
Signed-off-by: Xiao Ni <xni@redhat.com>
---
Detail.c | 2 +-
super0.c | 2 +-
util.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Detail.c b/Detail.c
index 509b0d4..cb33794 100644
--- a/Detail.c
+++ b/Detail.c
@@ -575,7 +575,7 @@ This is pretty boring
printf(" Member Arrays :");
while (dir && (de = readdir(dir)) != NULL) {
- char path[200];
+ char path[287];
char vbuf[1024];
int nlen = strlen(sra->sys_name);
dev_t devid;
diff --git a/super0.c b/super0.c
index 938cfd9..f5b4507 100644
--- a/super0.c
+++ b/super0.c
@@ -231,7 +231,7 @@ static void examine_super0(struct supertype *st, char *homehost)
d++) {
mdp_disk_t *dp;
char *dv;
- char nb[5];
+ char nb[11];
int wonly, failfast;
if (d>=0) dp = &sb->disks[d];
else dp = &sb->this_disk;
diff --git a/util.c b/util.c
index f100972..32bd909 100644
--- a/util.c
+++ b/util.c
@@ -811,7 +811,7 @@ unsigned long calc_csum(void *super, int bytes)
#ifndef MDASSEMBLE
char *human_size(long long bytes)
{
- static char buf[30];
+ static char buf[47];
/* We convert bytes to either centi-M{ega,ibi}bytes or
* centi-G{igi,ibi}bytes, with appropriate rounding,
--
2.7.4
^ permalink raw reply related
* [PATCH 2/3] mdadm: Add Wimplicit-fallthrough=0 in Makefile
From: Xiao Ni @ 2017-03-17 11:55 UTC (permalink / raw)
To: linux-raid; +Cc: Jes.Sorensen, ncroxon, artur.paszkiewicz
In-Reply-To: <1489751743-11397-1-git-send-email-xni@redhat.com>
There are many errors like 'error: this statement may fall through'.
But the logic is right. So add the flag Wimplicit-fallthrough=0
to disable the error messages. The method I use is from
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
#index-Wimplicit-fallthrough-375
Signed-off-by: Xiao Ni <xni@redhat.com>
---
Makefile | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Makefile b/Makefile
index a6f464c..d1a6ac4 100644
--- a/Makefile
+++ b/Makefile
@@ -48,6 +48,11 @@ ifdef WARN_UNUSED
CWFLAGS += -Wp,-D_FORTIFY_SOURCE=2 -O3
endif
+FALLTHROUGH := $(shell gcc -v --help 2>&1 | grep "implicit-fallthrough" | wc -l)
+ifneq "$(FALLTHROUGH)" "0"
+CWFLAGS += -Wimplicit-fallthrough=0
+endif
+
ifdef DEBIAN
CPPFLAGS += -DDEBIAN
endif
--
2.7.4
^ permalink raw reply related
* [PATCH 1/3] mdadm: Replace snprintf with strncpy at some places to avoid truncation
From: Xiao Ni @ 2017-03-17 11:55 UTC (permalink / raw)
To: linux-raid; +Cc: Jes.Sorensen, ncroxon, artur.paszkiewicz
In-Reply-To: <1489751743-11397-1-git-send-email-xni@redhat.com>
In gcc7 there are some building errors like:
directive output may be truncated writing up to 31 bytes into a region of size 24
snprintf(str, MPB_SIG_LEN, %s, mpb->sig);
It just need to copy one string to target. So use strncpy to replace it.
Signed-off-by: Xiao Ni <xni@redhat.com>
---
super-intel.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index d5e9517..e1618f1 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1811,7 +1811,8 @@ static void examine_super_imsm(struct supertype *st, char *homehost)
__u32 reserved = imsm_reserved_sectors(super, super->disks);
struct dl *dl;
- snprintf(str, MPB_SIG_LEN, "%s", mpb->sig);
+ strncpy(str, (char *)mpb->sig, MPB_SIG_LEN);
+ str[MPB_SIG_LEN-1] = '\0';
printf(" Magic : %s\n", str);
snprintf(str, strlen(MPB_VERSION_RAID0), "%s", get_imsm_version(mpb));
printf(" Version : %s\n", get_imsm_version(mpb));
@@ -5227,7 +5228,7 @@ static int init_super_imsm_volume(struct supertype *st, mdu_array_info_t *info,
disk->status = CONFIGURED_DISK | FAILED_DISK;
disk->scsi_id = __cpu_to_le32(~(__u32)0);
snprintf((char *) disk->serial, MAX_RAID_SERIAL_LEN,
- "missing:%d", i);
+ "missing:%d", (__u8)i);
}
find_missing(super);
} else {
@@ -7142,14 +7143,16 @@ static int update_subarray_imsm(struct supertype *st, char *subarray,
u->type = update_rename_array;
u->dev_idx = vol;
- snprintf((char *) u->name, MAX_RAID_SERIAL_LEN, "%s", name);
+ strncpy((char *) u->name, name, MAX_RAID_SERIAL_LEN);
+ u->name[MAX_RAID_SERIAL_LEN-1] = '\0';
append_metadata_update(st, u, sizeof(*u));
} else {
struct imsm_dev *dev;
int i;
dev = get_imsm_dev(super, vol);
- snprintf((char *) dev->volume, MAX_RAID_SERIAL_LEN, "%s", name);
+ strncpy((char *) dev->volume, name, MAX_RAID_SERIAL_LEN);
+ dev->volume[MAX_RAID_SERIAL_LEN-1] = '\0';
for (i = 0; i < mpb->num_raid_devs; i++) {
dev = get_imsm_dev(super, i);
handle_missing(super, dev);
--
2.7.4
^ permalink raw reply related
* [PATCH 0/3] mdadm: Fix some building errors
From: Xiao Ni @ 2017-03-17 11:55 UTC (permalink / raw)
To: linux-raid; +Cc: Jes.Sorensen, ncroxon, artur.paszkiewicz
There are some error buildings in Fedora release 26 (Rawhide)
The gcc version is gcc (GCC) 7.0.1 20170211 (Red Hat 7.0.1-0.8)
There are three types of errors:
1. Fall through
mdadm.c:149:28: error: this statement may fall through [-Werror=implicit-fallthrough=]
if (mode == ASSEMBLE || mode == BUILD ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mode == CREATE || mode == GROW ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mode == INCREMENTAL || mode == MANAGE)
~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
mdadm.c:151:3: note: here
case Brief:
^~~~
2. format-overflow
Detail.c: In function ‘Detail’:
Detail.c:584:31: error: ‘%s’ directive writing up to 255 bytes into a region of size 189 [-Werror=format-overflow=]
sprintf(path, "/sys/block/%s/md/metadata_version",
^~
Detail.c:584:5: note: ‘sprintf’ output between 32 and 287 bytes into a destination of size 200
sprintf(path, "/sys/block/%s/md/metadata_version",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
de->d_name);
~~~~~~~~~~~
3. format-truncation
super-intel.c: In function ‘examine_super_imsm’:
super-intel.c:1815:30: error: ‘%s’ directive output may be truncated writing up to 31 bytes into a region of size 24 [-Werror=format-truncation=]
snprintf(str, MPB_SIG_LEN, "%s", mpb->sig);
^~
super-intel.c:1815:2: note: ‘snprintf’ output between 1 and 32 bytes into a destination of size 24
snprintf(str, MPB_SIG_LEN, "%s", mpb->sig);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Xiao Ni (3):
Replace snprintf with strncpy at some places to avoid truncation
Add Wimplicit-fallthrough=0 in Makefile
Specify enough length when write to buffer
Detail.c | 2 +-
Makefile | 5 +++++
super-intel.c | 11 +++++++----
super0.c | 2 +-
util.c | 2 +-
5 files changed, 15 insertions(+), 7 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH V1] md/raid10: refactor some codes from raid10_write_request
From: Guoqing Jiang @ 2017-03-17 9:45 UTC (permalink / raw)
To: linux-raid; +Cc: shli, neilb, Guoqing Jiang
Previously, we clone both bio and repl_bio in raid10_write_request,
then add the cloned bio to plug->pending or conf->pending_bio_list
based on plug or not, and most of the logics are same for the two
conditions.
So introduce raid10_write_one_disk for it, and use replacement parameter
to distinguish the difference. No functional changes in the patch.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
Changes from RFC:
1. rename handle_clonebio to raid10_write_one_disk
2. s/i/n_copy/ and s/int replacement/bool replacement/
drivers/md/raid10.c | 172 ++++++++++++++++++++++------------------------------
1 file changed, 72 insertions(+), 100 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index b1b1f982a722..a931e6e7b280 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1188,18 +1188,81 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
return;
}
-static void raid10_write_request(struct mddev *mddev, struct bio *bio,
- struct r10bio *r10_bio)
+static void raid10_write_one_disk(struct mddev *mddev, struct r10bio *r10_bio,
+ struct bio *bio, bool replacement,
+ int n_copy, int max_sectors)
{
- struct r10conf *conf = mddev->private;
- int i;
const int op = bio_op(bio);
const unsigned long do_sync = (bio->bi_opf & REQ_SYNC);
const unsigned long do_fua = (bio->bi_opf & REQ_FUA);
unsigned long flags;
- struct md_rdev *blocked_rdev;
struct blk_plug_cb *cb;
struct raid10_plug_cb *plug = NULL;
+ struct r10conf *conf = mddev->private;
+ struct md_rdev *rdev;
+ int devnum = r10_bio->devs[i].devnum;
+ struct bio *mbio;
+
+ if (replacement) {
+ rdev = conf->mirrors[devnum].replacement;
+ if (rdev == NULL) {
+ /* Replacement just got moved to main 'rdev' */
+ smp_mb();
+ rdev = conf->mirrors[devnum].rdev;
+ }
+ } else
+ rdev = conf->mirrors[devnum].rdev;
+
+ mbio = bio_clone_fast(bio, GFP_NOIO, mddev->bio_set);
+ bio_trim(mbio, r10_bio->sector - bio->bi_iter.bi_sector, max_sectors);
+ if (replacement)
+ r10_bio->devs[i].repl_bio = mbio;
+ else
+ r10_bio->devs[i].bio = mbio;
+
+ mbio->bi_iter.bi_sector = (r10_bio->devs[i].addr +
+ choose_data_offset(r10_bio, rdev));
+ mbio->bi_bdev = rdev->bdev;
+ mbio->bi_end_io = raid10_end_write_request;
+ bio_set_op_attrs(mbio, op, do_sync | do_fua);
+ if (!replacement && test_bit(FailFast, &conf->mirrors[devnum].rdev->flags)
+ && enough(conf, devnum))
+ mbio->bi_opf |= MD_FAILFAST;
+ mbio->bi_private = r10_bio;
+
+ if (conf->mddev->gendisk)
+ trace_block_bio_remap(bdev_get_queue(mbio->bi_bdev),
+ mbio, disk_devt(conf->mddev->gendisk),
+ r10_bio->sector);
+ /* flush_pending_writes() needs access to the rdev so...*/
+ mbio->bi_bdev = (void *)rdev;
+
+ atomic_inc(&r10_bio->remaining);
+
+ cb = blk_check_plugged(raid10_unplug, mddev, sizeof(*plug));
+ if (cb)
+ plug = container_of(cb, struct raid10_plug_cb, cb);
+ else
+ plug = NULL;
+ spin_lock_irqsave(&conf->device_lock, flags);
+ if (plug) {
+ bio_list_add(&plug->pending, mbio);
+ plug->pending_cnt++;
+ } else {
+ bio_list_add(&conf->pending_bio_list, mbio);
+ conf->pending_count++;
+ }
+ spin_unlock_irqrestore(&conf->device_lock, flags);
+ if (!plug)
+ md_wakeup_thread(mddev->thread);
+}
+
+static void raid10_write_request(struct mddev *mddev, struct bio *bio,
+ struct r10bio *r10_bio)
+{
+ struct r10conf *conf = mddev->private;
+ int i;
+ struct md_rdev *blocked_rdev;
sector_t sectors;
int sectors_handled;
int max_sectors;
@@ -1402,101 +1465,10 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
bitmap_startwrite(mddev->bitmap, r10_bio->sector, r10_bio->sectors, 0);
for (i = 0; i < conf->copies; i++) {
- struct bio *mbio;
- int d = r10_bio->devs[i].devnum;
- if (r10_bio->devs[i].bio) {
- struct md_rdev *rdev = conf->mirrors[d].rdev;
- mbio = bio_clone_fast(bio, GFP_NOIO, mddev->bio_set);
- bio_trim(mbio, r10_bio->sector - bio->bi_iter.bi_sector,
- max_sectors);
- r10_bio->devs[i].bio = mbio;
-
- mbio->bi_iter.bi_sector = (r10_bio->devs[i].addr+
- choose_data_offset(r10_bio, rdev));
- mbio->bi_bdev = rdev->bdev;
- mbio->bi_end_io = raid10_end_write_request;
- bio_set_op_attrs(mbio, op, do_sync | do_fua);
- if (test_bit(FailFast, &conf->mirrors[d].rdev->flags) &&
- enough(conf, d))
- mbio->bi_opf |= MD_FAILFAST;
- mbio->bi_private = r10_bio;
-
- if (conf->mddev->gendisk)
- trace_block_bio_remap(bdev_get_queue(mbio->bi_bdev),
- mbio, disk_devt(conf->mddev->gendisk),
- r10_bio->sector);
- /* flush_pending_writes() needs access to the rdev so...*/
- mbio->bi_bdev = (void*)rdev;
-
- atomic_inc(&r10_bio->remaining);
-
- cb = blk_check_plugged(raid10_unplug, mddev,
- sizeof(*plug));
- if (cb)
- plug = container_of(cb, struct raid10_plug_cb,
- cb);
- else
- plug = NULL;
- spin_lock_irqsave(&conf->device_lock, flags);
- if (plug) {
- bio_list_add(&plug->pending, mbio);
- plug->pending_cnt++;
- } else {
- bio_list_add(&conf->pending_bio_list, mbio);
- conf->pending_count++;
- }
- spin_unlock_irqrestore(&conf->device_lock, flags);
- if (!plug)
- md_wakeup_thread(mddev->thread);
- }
-
- if (r10_bio->devs[i].repl_bio) {
- struct md_rdev *rdev = conf->mirrors[d].replacement;
- if (rdev == NULL) {
- /* Replacement just got moved to main 'rdev' */
- smp_mb();
- rdev = conf->mirrors[d].rdev;
- }
- mbio = bio_clone_fast(bio, GFP_NOIO, mddev->bio_set);
- bio_trim(mbio, r10_bio->sector - bio->bi_iter.bi_sector,
- max_sectors);
- r10_bio->devs[i].repl_bio = mbio;
-
- mbio->bi_iter.bi_sector = (r10_bio->devs[i].addr +
- choose_data_offset(r10_bio, rdev));
- mbio->bi_bdev = rdev->bdev;
- mbio->bi_end_io = raid10_end_write_request;
- bio_set_op_attrs(mbio, op, do_sync | do_fua);
- mbio->bi_private = r10_bio;
-
- if (conf->mddev->gendisk)
- trace_block_bio_remap(bdev_get_queue(mbio->bi_bdev),
- mbio, disk_devt(conf->mddev->gendisk),
- r10_bio->sector);
- /* flush_pending_writes() needs access to the rdev so...*/
- mbio->bi_bdev = (void*)rdev;
-
- atomic_inc(&r10_bio->remaining);
-
- cb = blk_check_plugged(raid10_unplug, mddev,
- sizeof(*plug));
- if (cb)
- plug = container_of(cb, struct raid10_plug_cb,
- cb);
- else
- plug = NULL;
- spin_lock_irqsave(&conf->device_lock, flags);
- if (plug) {
- bio_list_add(&plug->pending, mbio);
- plug->pending_cnt++;
- } else {
- bio_list_add(&conf->pending_bio_list, mbio);
- conf->pending_count++;
- }
- spin_unlock_irqrestore(&conf->device_lock, flags);
- if (!plug)
- md_wakeup_thread(mddev->thread);
- }
+ if (r10_bio->devs[i].bio)
+ raid10_write_one_disk(mddev, r10_bio, bio, false, i, max_sectors);
+ if (r10_bio->devs[i].repl_bio)
+ raid10_write_one_disk(mddev, r10_bio, bio, true, i, max_sectors);
}
/* Don't remove the bias on 'remaining' (one_write_done) until
--
2.6.2
^ permalink raw reply related
* Re: [PATCH 0/7] LLVM: make x86_64 kernel build with clang.
From: Dmitry Vyukov @ 2017-03-17 8:17 UTC (permalink / raw)
To: Michael Davidson
Cc: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
Matthias Kaehlcke, x86@kernel.org, open list:KERNEL BUILD + fi...,
LKML, linux-crypto, linux-raid
In-Reply-To: <20170317001520.85223-1-md@google.com>
On Fri, Mar 17, 2017 at 1:15 AM, Michael Davidson <md@google.com> wrote:
> This patch set is sufficient to get the x86_64 kernel to build
> and boot correctly with clang-3.8 or greater.
>
> The resulting build still has about 300 warnings, very few of
> which appear to be significant. Most of them should be fixable
> with some minor code refactoring although a few of them, such
> as the complaints about implict conversions between enumerated
> types may be candidates for just being disabled.
Thanks, Michael!
This will help us a lot with KMSAN (uninit use detector) and code coverage.
> Michael Davidson (7):
> Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS
> Makefile, x86, LLVM: disable unsupported optimization flags
> x86, LLVM: suppress clang warnings about unaligned accesses
> x86, boot, LLVM: #undef memcpy etc in string.c
> x86, boot, LLVM: Use regparm=0 for memcpy and memset
> md/raid10, LLVM: get rid of variable length array
> crypto, x86, LLVM: aesni - fix token pasting
>
> Makefile | 4 ++++
> arch/x86/Makefile | 7 +++++++
> arch/x86/boot/copy.S | 15 +++++++++++++--
> arch/x86/boot/string.c | 9 +++++++++
> arch/x86/boot/string.h | 13 +++++++++++++
> arch/x86/crypto/aes_ctrby8_avx-x86_64.S | 7 ++-----
> drivers/md/raid10.c | 9 ++++-----
> 7 files changed, 52 insertions(+), 12 deletions(-)
>
> --
> 2.12.0.367.g23dc2f6d3c-goog
>
^ permalink raw reply
* 63129 linux-raid
From: citydesk @ 2017-03-17 7:22 UTC (permalink / raw)
To: linux-raid
[-- Attachment #1: 6.zip --]
[-- Type: application/zip, Size: 6577 bytes --]
^ permalink raw reply
* [PATCH 7/7] crypto, x86, LLVM: aesni - fix token pasting
From: Michael Davidson @ 2017-03-17 0:15 UTC (permalink / raw)
To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Herbert Xu, David S. Miller, Shaohua Li
Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
linux-kbuild, linux-kernel, linux-crypto, linux-raid,
Michael Davidson
In-Reply-To: <20170317001520.85223-1-md@google.com>
aes_ctrby8_avx-x86_64.S uses the C preprocessor for token pasting
of character sequences that are not valid preprocessor tokens.
While this is allowed when preprocessing assembler files it exposes
an incompatibilty between the clang and gcc preprocessors where
clang does not strip leading white space from macro parameters,
leading to the CONCAT(%xmm, i) macro expansion on line 96 resulting
in a token with a space character embedded in it.
While this could be fixed by deleting the offending space character,
the assembler is perfectly capable of handling the token pasting
correctly for itself so it seems preferable to let it do so and to
get rid or the CONCAT(), DDQ() and XMM() preprocessor macros.
Signed-off-by: Michael Davidson <md@google.com>
---
arch/x86/crypto/aes_ctrby8_avx-x86_64.S | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
index a916c4a61165..5f6a5af9c489 100644
--- a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
+++ b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
@@ -65,7 +65,6 @@
#include <linux/linkage.h>
#include <asm/inst.h>
-#define CONCAT(a,b) a##b
#define VMOVDQ vmovdqu
#define xdata0 %xmm0
@@ -92,8 +91,6 @@
#define num_bytes %r8
#define tmp %r10
-#define DDQ(i) CONCAT(ddq_add_,i)
-#define XMM(i) CONCAT(%xmm, i)
#define DDQ_DATA 0
#define XDATA 1
#define KEY_128 1
@@ -131,12 +128,12 @@ ddq_add_8:
/* generate a unique variable for ddq_add_x */
.macro setddq n
- var_ddq_add = DDQ(\n)
+ var_ddq_add = ddq_add_\n
.endm
/* generate a unique variable for xmm register */
.macro setxdata n
- var_xdata = XMM(\n)
+ var_xdata = %xmm\n
.endm
/* club the numeric 'id' to the symbol 'name' */
--
2.12.0.367.g23dc2f6d3c-goog
^ permalink raw reply related
* [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Michael Davidson @ 2017-03-17 0:15 UTC (permalink / raw)
To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Herbert Xu, David S. Miller, Shaohua Li
Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
linux-kbuild, linux-kernel, linux-crypto, linux-raid,
Michael Davidson
In-Reply-To: <20170317001520.85223-1-md@google.com>
Replace a variable length array in a struct by allocating
the memory for the entire struct in a char array on the stack.
Signed-off-by: Michael Davidson <md@google.com>
---
drivers/md/raid10.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 063c43d83b72..158ebdff782c 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4654,11 +4654,10 @@ static int handle_reshape_read_error(struct mddev *mddev,
/* Use sync reads to get the blocks from somewhere else */
int sectors = r10_bio->sectors;
struct r10conf *conf = mddev->private;
- struct {
- struct r10bio r10_bio;
- struct r10dev devs[conf->copies];
- } on_stack;
- struct r10bio *r10b = &on_stack.r10_bio;
+ char on_stack_r10_bio[sizeof(struct r10bio) +
+ conf->copies * sizeof(struct r10dev)]
+ __aligned(__alignof__(struct r10bio));
+ struct r10bio *r10b = (struct r10bio *)on_stack_r10_bio;
int slot = 0;
int idx = 0;
struct bio_vec *bvec = r10_bio->master_bio->bi_io_vec;
--
2.12.0.367.g23dc2f6d3c-goog
^ permalink raw reply related
* [PATCH 5/7] x86, boot, LLVM: Use regparm=0 for memcpy and memset
From: Michael Davidson @ 2017-03-17 0:15 UTC (permalink / raw)
To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Herbert Xu, David S. Miller, Shaohua Li
Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
linux-kbuild, linux-kernel, linux-crypto, linux-raid,
Michael Davidson
In-Reply-To: <20170317001520.85223-1-md@google.com>
Use the standard regparm=0 calling convention for memcpy and
memset when building with clang.
This is a work around for a long standing clang bug
(see https://llvm.org/bugs/show_bug.cgi?id=3997) where
clang always uses the standard regparm=0 calling convention
for any implcit calls to memcpy and memset that it generates
(eg for structure assignments and initialization) even if an
alternate calling convention such as regparm=3 has been specified.
Signed-off-by: Michael Davidson <md@google.com>
---
arch/x86/boot/copy.S | 15 +++++++++++++--
arch/x86/boot/string.h | 13 +++++++++++++
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/arch/x86/boot/copy.S b/arch/x86/boot/copy.S
index 1eb7d298b47d..57142d1ad0d2 100644
--- a/arch/x86/boot/copy.S
+++ b/arch/x86/boot/copy.S
@@ -18,6 +18,12 @@
.text
GLOBAL(memcpy)
+#ifdef __clang__ /* Use normal ABI calling conventions */
+ movw 4(%esp), %ax
+ movw 8(%esp), %dx
+ movw 12(%esp), %cx
+#endif
+_memcpy:
pushw %si
pushw %di
movw %ax, %di
@@ -34,6 +40,11 @@ GLOBAL(memcpy)
ENDPROC(memcpy)
GLOBAL(memset)
+#ifdef __clang__ /* Use normal ABI calling conventions */
+ movw 4(%esp), %ax
+ movw 8(%esp), %dx
+ movw 12(%esp), %cx
+#endif
pushw %di
movw %ax, %di
movzbl %dl, %eax
@@ -52,7 +63,7 @@ GLOBAL(copy_from_fs)
pushw %ds
pushw %fs
popw %ds
- calll memcpy
+ calll _memcpy
popw %ds
retl
ENDPROC(copy_from_fs)
@@ -61,7 +72,7 @@ GLOBAL(copy_to_fs)
pushw %es
pushw %fs
popw %es
- calll memcpy
+ calll _memcpy
popw %es
retl
ENDPROC(copy_to_fs)
diff --git a/arch/x86/boot/string.h b/arch/x86/boot/string.h
index 113588ddb43f..e735cccb3fc8 100644
--- a/arch/x86/boot/string.h
+++ b/arch/x86/boot/string.h
@@ -6,8 +6,21 @@
#undef memset
#undef memcmp
+/*
+ * Use normal ABI calling conventions - i.e. regparm(0) -
+ * for memcpy() and memset() if we are building the real
+ * mode setup code with clang since clang may make implicit
+ * calls to these functions that assume regparm(0).
+ */
+#if defined(_SETUP) && defined(__clang__)
+void __attribute__((regparm(0))) *memcpy(void *dst, const void *src,
+ size_t len);
+void __attribute__((regparm(0))) *memset(void *dst, int c, size_t len);
+#else
void *memcpy(void *dst, const void *src, size_t len);
void *memset(void *dst, int c, size_t len);
+#endif
+
int memcmp(const void *s1, const void *s2, size_t len);
/*
--
2.12.0.367.g23dc2f6d3c-goog
^ permalink raw reply related
* [PATCH 4/7] x86, boot, LLVM: #undef memcpy etc in string.c
From: Michael Davidson @ 2017-03-17 0:15 UTC (permalink / raw)
To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Herbert Xu, David S. Miller, Shaohua Li
Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
linux-kbuild, linux-kernel, linux-crypto, linux-raid,
Michael Davidson
In-Reply-To: <20170317001520.85223-1-md@google.com>
undef memcpy and friends in boot/string.c so that the functions
defined here will have the correct names, otherwise we end up
up trying to redefine __builtin_memcpy etc.
Surprisingly, gcc allows this (and, helpfully, discards the
__builtin_ prefix from the function name when compiling it),
but clang does not.
Adding these #undef's appears to preserve what I assume was
the original intent of the code.
Signed-off-by: Michael Davidson <md@google.com>
---
arch/x86/boot/string.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
index 5457b02fc050..b40266850869 100644
--- a/arch/x86/boot/string.c
+++ b/arch/x86/boot/string.c
@@ -16,6 +16,15 @@
#include "ctype.h"
#include "string.h"
+/*
+ * Undef these macros so that the functions that we provide
+ * here will have the correct names regardless of how string.h
+ * may have chosen to #define them.
+ */
+#undef memcpy
+#undef memset
+#undef memcmp
+
int memcmp(const void *s1, const void *s2, size_t len)
{
bool diff;
--
2.12.0.367.g23dc2f6d3c-goog
^ permalink raw reply related
* [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses
From: Michael Davidson @ 2017-03-17 0:15 UTC (permalink / raw)
To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Herbert Xu, David S. Miller, Shaohua Li
Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
linux-kbuild, linux-kernel, linux-crypto, linux-raid,
Michael Davidson
In-Reply-To: <20170317001520.85223-1-md@google.com>
Suppress clang warnings about potential unaliged accesses
to members in packed structs. This gets rid of almost 10,000
warnings about accesses to the ring 0 stack pointer in the TSS.
Signed-off-by: Michael Davidson <md@google.com>
---
arch/x86/Makefile | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 894a8d18bf97..7f21703c475d 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -128,6 +128,11 @@ endif
KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
endif
+ifeq ($(cc-name),clang)
+# Suppress clang warnings about potential unaligned accesses.
+KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
+endif
+
ifdef CONFIG_X86_X32
x32_ld_ok := $(call try-run,\
/bin/echo -e '1: .quad 1b' | \
--
2.12.0.367.g23dc2f6d3c-goog
^ permalink raw reply related
* [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
From: Michael Davidson @ 2017-03-17 0:15 UTC (permalink / raw)
To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Herbert Xu, David S. Miller, Shaohua Li
Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
linux-kbuild, linux-kernel, linux-crypto, linux-raid,
Michael Davidson
In-Reply-To: <20170317001520.85223-1-md@google.com>
Unfortunately, while clang generates a warning about these flags
being unsupported it still exits with a status of 0 so we have
to explicitly disable them instead of just using a cc-option check.
Signed-off-by: Michael Davidson <md@google.com>
---
Makefile | 2 ++
arch/x86/Makefile | 2 ++
2 files changed, 4 insertions(+)
diff --git a/Makefile b/Makefile
index b21fd0ca2946..5e97e5fc1eea 100644
--- a/Makefile
+++ b/Makefile
@@ -629,7 +629,9 @@ ARCH_AFLAGS :=
ARCH_CFLAGS :=
include arch/$(SRCARCH)/Makefile
+ifneq ($(cc-name),clang)
KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,)
+endif
KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 2d449337a360..894a8d18bf97 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -87,11 +87,13 @@ else
KBUILD_AFLAGS += -m64
KBUILD_CFLAGS += -m64
+ifneq ($(cc-name),clang)
# Align jump targets to 1 byte, not the default 16 bytes:
KBUILD_CFLAGS += -falign-jumps=1
# Pack loops tightly as well:
KBUILD_CFLAGS += -falign-loops=1
+endif
# Don't autogenerate traditional x87 instructions
KBUILD_CFLAGS += $(call cc-option,-mno-80387)
--
2.12.0.367.g23dc2f6d3c-goog
^ permalink raw reply related
* [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS
From: Michael Davidson @ 2017-03-17 0:15 UTC (permalink / raw)
To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Herbert Xu, David S. Miller, Shaohua Li
Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
linux-kbuild, linux-kernel, linux-crypto, linux-raid,
Michael Davidson
In-Reply-To: <20170317001520.85223-1-md@google.com>
Add -no-integrated-as to KBUILD_AFLAGS and KBUILD_CFLAGS
for clang.
Signed-off-by: Michael Davidson <md@google.com>
---
Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Makefile b/Makefile
index b841fb36beb2..b21fd0ca2946 100644
--- a/Makefile
+++ b/Makefile
@@ -704,6 +704,8 @@ KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
# See modpost pattern 2
KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,)
KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
+KBUILD_CFLAGS += $(call cc-option, -no-integrated-as)
+KBUILD_AFLAGS += $(call cc-option, -no-integrated-as)
else
# These warnings generated too much noise in a regular build.
--
2.12.0.367.g23dc2f6d3c-goog
^ permalink raw reply related
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