Linux RAID subsystem development
 help / color / mirror / Atom feed
* RE: Read data from disk that was part of RAID1 array
From: Peter Sangas @ 2017-03-17 23:17 UTC (permalink / raw)
  To: linux-raid, pete
In-Reply-To: <008801d29ea3$3e858000$bb908000$@warren-selbert.com>



> From: Peter Sangas [mailto:pete@wnsdev.com]
> I have only one disk from a RAID1 array and I would like to read data from
one of the
> partitions.  This there a way to mount this and read the data?

This worked for me:

mdadm --create /dev/md10 -l 1 -n 2 /dev/sda missing     md10 is chosen to
avoid conflicts with existing RAID1

mount /dev/md10 /mnt 


^ permalink raw reply

* Re: dm crypt: remove an impossible condition
From: Mike Snitzer @ 2017-03-17 21:38 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: kernel-janitors, linux-raid, dm-devel, Shaohua Li, Milan Broz,
	Alasdair Kergon
In-Reply-To: <20170317204655.GA28261@mwanda>

On Fri, Mar 17 2017 at  4:46pm -0400,
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> Static checkers complain that it doesn't make sense to check if "sval"
> is NULL.  The intention was to check if strchr() returned NULL, but in
> that situation "sval" would be "NULL + 1" so the check doesn't work.  We
> know from the sscanf() that there is a ':' character in the string so
> the check is unnecessary and can be removed.
> 
> Now that the check doesn't depend on "sval" it can be moved earlier
> for readability.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Thanks Dan, I've folded this fix into the original commit, see:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.12&id=cf22cd5f3afe7335afee5659f7450000e8fa2a15

I didn't add your Signed-off-by though, I can backfill that if you
like.. it is just I didn't want to taint you with all the extensive
changes in that commit.

Mike

^ permalink raw reply

* Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
From: H. Peter Anvin @ 2017-03-17 21:34 UTC (permalink / raw)
  To: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid
In-Reply-To: <ad94b647-4ba0-4ae9-be8f-6fe7e9406def@zytor.com>

On 03/17/17 14:32, H. Peter Anvin wrote:
> 
> NAK.  Fix your compiler, or use a wrapper script or something.  It is
> absolutely *not* acceptable to disable this since future versions of
> clang *should* support that.
> 
> That being said, it might make sense to look for a key pattern like
> "(un|not )supported" on stderr the try-run macro.  Is there really no
> -Wno- or -Werror= option to turn off this craziness?
> 

Well, guess what... I found it myself.

-W{no-,error=}ignored-optimization-argument

Either variant will make this sane.

	-hpa



^ permalink raw reply

* Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
From: H. Peter Anvin @ 2017-03-17 21:32 UTC (permalink / raw)
  To: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid
In-Reply-To: <20170317001520.85223-3-md@google.com>

On 03/16/17 17:15, Michael Davidson wrote:
> 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)
> 

NAK.  Fix your compiler, or use a wrapper script or something.  It is
absolutely *not* acceptable to disable this since future versions of
clang *should* support that.

That being said, it might make sense to look for a key pattern like
"(un|not )supported" on stderr the try-run macro.  Is there really no
-Wno- or -Werror= option to turn off this craziness?

	-hpa


^ permalink raw reply

* [PATCH] dm crypt: remove an impossible condition
From: Dan Carpenter @ 2017-03-17 20:46 UTC (permalink / raw)
  To: Alasdair Kergon, Milan Broz
  Cc: Mike Snitzer, dm-devel, Shaohua Li, linux-raid, kernel-janitors

Static checkers complain that it doesn't make sense to check if "sval"
is NULL.  The intention was to check if strchr() returned NULL, but in
that situation "sval" would be "NULL + 1" so the check doesn't work.  We
know from the sscanf() that there is a ':' character in the string so
the check is unnecessary and can be removed.

Now that the check doesn't depend on "sval" it can be moved earlier
for readability.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index ec09bd703b7d..eba218737bdb 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2401,12 +2401,12 @@ static int crypt_ctr_optional(struct dm_target *ti, unsigned int argc, char **ar
 		else if (!strcasecmp(opt_string, "submit_from_crypt_cpus"))
 			set_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags);
 		else if (sscanf(opt_string, "integrity:%u:", &val) == 1) {
-			cc->on_disk_tag_size = val;
-			sval = strchr(opt_string + strlen("integrity:"), ':') + 1;
-			if (val == 0 || val > MAX_TAG_SIZE || !sval) {
+			if (val == 0 || val > MAX_TAG_SIZE) {
 				ti->error = "Invalid integrity arguments";
 				return -EINVAL;
 			}
+			cc->on_disk_tag_size = val;
+			sval = strchr(opt_string + strlen("integrity:"), ':') + 1;
 			if (!strcasecmp(sval, "aead")) {
 				set_bit(CRYPT_MODE_INTEGRITY_AEAD, &cc->cipher_flags);
 			} else  if (!strncasecmp(sval, "hmac(", strlen("hmac("))) {

^ permalink raw reply related

* Re: [PATCH 1/2] super1: ignore failfast flag for setting device role
From: jes.sorensen @ 2017-03-17 20:20 UTC (permalink / raw)
  To: Gioh Kim; +Cc: neilb, linux-raid, linux-kernel, Jack Wang
In-Reply-To: <1489769222-31855-2-git-send-email-gi-oh.kim@profitbricks.com>

Gioh Kim <gi-oh.kim@profitbricks.com> writes:
> 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);

This does not look right - you haven't assigned a value to dk_state, but
then start masking bits out of it.

Cheers,
Jes

^ permalink raw reply

* Re: [PATCH v3 3/6] imsm: PPL support
From: jes.sorensen @ 2017-03-17 20:11 UTC (permalink / raw)
  To: Artur Paszkiewicz; +Cc: linux-raid
In-Reply-To: <20170316210948.21093-4-artur.paszkiewicz@intel.com>

Artur Paszkiewicz <artur.paszkiewicz@intel.com> writes:
> Enable creating and assembling IMSM raid5 arrays with PPL. Update the
> IMSM metadata format to include new fields used for PPL.
>
> Add structures for PPL metadata. They are used also by super1 and shared
> with the kernel, so put them in md_p.h.
>
> Write the initial empty PPL header when creating an array. When
> assembling an array with PPL, validate the PPL header and in case it is
> not correct allow to overwrite it if --force was provided.
>
> Write the PPL location and size for a device to the new rdev sysfs
> attributes 'ppl_sector' and 'ppl_size'. Enable PPL in the kernel by
> writing to 'consistency_policy' before the array is activated.
>
> Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
> ---
>  Assemble.c    |  49 +++++++++++
>  Makefile      |   5 +-
>  md_p.h        |  25 ++++++
>  mdadm.h       |   6 ++
>  super-intel.c | 274 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  sysfs.c       |  14 +++
>  6 files changed, 349 insertions(+), 24 deletions(-)
>
> diff --git a/Assemble.c b/Assemble.c
> index 3da09033..8e55b49f 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -1942,6 +1942,55 @@ int assemble_container_content(struct supertype *st, int mdfd,
>  	map_update(NULL, fd2devnm(mdfd), content->text_version,
>  		   content->uuid, chosen_name);
>  
> +	if (content->consistency_policy == CONSISTENCY_POLICY_PPL &&
> +	    st->ss->validate_ppl) {
> +		content->array.state |= 1;
> +		err = 0;
> +
> +		for (dev = content->devs; dev; dev = dev->next) {
> +			int dfd;
> +			char *devpath;
> +			int ret;
> +
> +			ret = st->ss->validate_ppl(st, content, dev);
> +			if (ret == 0)
> +				continue;
> +
> +			if (ret < 0) {
> +				err = 1;
> +				break;
> +			}
> +
> +			if (!c->force) {
> +				pr_err("%s contains invalid PPL - consider --force or --update-subarray with --update=no-ppl\n",
> +					chosen_name);
> +				content->array.state &= ~1;
> +				avail[dev->disk.raid_disk] = 0;
> +				break;
> +			}
> +
> +			/* have --force - overwrite the invalid ppl */
> +			devpath = map_dev(dev->disk.major, dev->disk.minor, 0);
> +			dfd = dev_open(devpath, O_RDWR);
> +			if (dfd < 0) {
> +				pr_err("Failed to open %s\n", devpath);
> +				err = 1;
> +				break;
> +			}
> +
> +			err = st->ss->write_init_ppl(st, content, dfd);
> +			close(dfd);
> +
> +			if (err)
> +				break;
> +		}
> +
> +		if (err) {
> +			free(avail);
> +			return err;
> +		}
> +	}
> +
>  	if (enough(content->array.level, content->array.raid_disks,
>  		   content->array.layout, content->array.state & 1, avail) == 0) {
>  		if (c->export && result)
> diff --git a/Makefile b/Makefile
> index a6f464c3..49da491f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -146,7 +146,7 @@ MON_OBJS = mdmon.o monitor.o managemon.o util.o maps.o mdstat.o sysfs.o \
>  	Kill.o sg_io.o dlink.o ReadMe.o super-intel.o \
>  	super-mbr.o super-gpt.o \
>  	super-ddf.o sha1.o crc32.o msg.o bitmap.o xmalloc.o \
> -	platform-intel.o probe_roms.o
> +	platform-intel.o probe_roms.o crc32c.o
>  
>  MON_SRCS = $(patsubst %.o,%.c,$(MON_OBJS))
>  
> @@ -156,7 +156,8 @@ STATICOBJS = pwgr.o
>  ASSEMBLE_SRCS := mdassemble.c Assemble.c Manage.c config.c policy.c dlink.c util.c \
>  	maps.c lib.c xmalloc.c \
>  	super0.c super1.c super-ddf.c super-intel.c sha1.c crc32.c sg_io.c mdstat.c \
> -	platform-intel.c probe_roms.c sysfs.c super-mbr.c super-gpt.c mapfile.c
> +	platform-intel.c probe_roms.c sysfs.c super-mbr.c super-gpt.c mapfile.c \
> +	crc32c.o

Hi Artur,

This looks odd - sure you don't mean crc32c.c ?

Thanks,
Jes

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: hpa @ 2017-03-17 20:04 UTC (permalink / raw)
  To: Peter Zijlstra, Michael Davidson
  Cc: Alexander Potapenko, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li, Dmitry Vyukov,
	Matthias Kaehlcke, x86, linux-kbuild, LKML, linux-crypto,
	linux-raid
In-Reply-To: <20170317192746.y5ade3bymifdni2p@hirez.programming.kicks-ass.net>

On March 17, 2017 12:27:46 PM PDT, Peter Zijlstra <peterz@infradead.org> wrote:
>On Fri, Mar 17, 2017 at 11:52:01AM -0700, Michael Davidson wrote:
>> On Fri, Mar 17, 2017 at 5:44 AM, Peter Zijlstra
><peterz@infradead.org> wrote:
>> >
>> > 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.
>> >
>> 
>> I agree that the code is horrible.
>> 
>> It is, in fact, exactly the same solution that was used to remove
>> variable length arrays in structs from several of the crypto drivers
>a
>> few years ago - see the definition of SHASH_DESC_ON_STACK() in
>> "crypto/hash.h" - I did not, however, hide the horrors in a macro
>> preferring to leave the implementation visible as a warning to
>whoever
>> might touch the code next.
>> 
>> I believe that the actual stack usage is exactly the same as it was
>previously.
>> 
>> I can certainly wrap this  up in a macro and add comments with
>> appropriately dire warnings in it if you feel that is both necessary
>> and sufficient.
>
>We got away with ugly in the past, so we should get to do it again?

Seriously, you should have taken the hack the first time that this needs to be fixed.  Just because this is a fairly uncommon construct in the kernel doesn't mean it is not in userspace.

I would like to say this falls in the category of "fix your compiler this time".  Once is one thing, twice is unacceptable.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply

* Re: [PATCH 3/3] mdadm: Specify enough length when write to buffer
From: jes.sorensen @ 2017-03-17 19:58 UTC (permalink / raw)
  To: Xiao Ni; +Cc: linux-raid, ncroxon, artur.paszkiewicz
In-Reply-To: <1489751743-11397-4-git-send-email-xni@redhat.com>

Xiao Ni <xni@redhat.com> writes:
> 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(-)

Applied!

Thanks!
Jes

^ permalink raw reply

* Re: [PATCH 2/3] mdadm: Add Wimplicit-fallthrough=0 in Makefile
From: jes.sorensen @ 2017-03-17 19:57 UTC (permalink / raw)
  To: Xiao Ni; +Cc: linux-raid, ncroxon, artur.paszkiewicz
In-Reply-To: <1489751743-11397-3-git-send-email-xni@redhat.com>

Xiao Ni <xni@redhat.com> writes:
> 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(+)

Applied!

Thanks,
Jes

^ permalink raw reply

* Re: [PATCH 1/3] mdadm: Replace snprintf with strncpy at some places to avoid truncation
From: jes.sorensen @ 2017-03-17 19:55 UTC (permalink / raw)
  To: Xiao Ni; +Cc: linux-raid, ncroxon, artur.paszkiewicz
In-Reply-To: <1489751743-11397-2-git-send-email-xni@redhat.com>

Xiao Ni <xni@redhat.com> writes:
> 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));

Given str is defined as 'char str[MAX_SIGNATURE_LENGTH]', it would be
more correct to use MAX_SIGNATURE_LENGTH as the size limit here.

> @@ -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 {

This is unrelated to the commit message.

> @@ -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);

These two look fine.

Jes

^ permalink raw reply

* Re: [PATCH v2] mdadm/r5cache: allow adding journal to array without journal
From: jes.sorensen @ 2017-03-17 19:50 UTC (permalink / raw)
  To: Song Liu; +Cc: linux-raid, neilb, kernel-team
In-Reply-To: <20170316232405.3523257-1-songliubraving@fb.com>

Song Liu <songliubraving@fb.com> writes:
> Currently, --add-journal can be only used to recreate broken journal
> for arrays with journal since  creation. As the kernel code getting
> more mature, this constraint is no longer necessary.
>
> This patch allows --add-journal to add journal to array without
> journal.
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
>  Manage.c   | 9 ---------
>  mdadm.8.in | 5 ++---
>  2 files changed, 2 insertions(+), 12 deletions(-)

Hi Song,

I spotted this one after I had already applied the first patch with
this title, so I went back and removed the original one and applied this
one instead, but now the build breaks.

When posting a v2, please send it out as a response to the patch it is
meant to replace, like this:
  git send-email --in-reply-to=<....>

Second, please put a note in the commit why this is a v2.

I am going to revert them all for now. Please send me a fresh consistent
set to apply.

Thanks,
Jes

cc -Wall -Werror -Wstrict-prototypes -Wextra -Wno-unused-parameter -ggdb -DSendmail=\""/usr/sbin/sendmail -t"\" -DCONFFILE=\"/etc/mdadm.conf\" -DCONFFILE2=\"/etc/mdadm/mdadm.conf\" -DMAP_DIR=\"/run/mdadm\" -DMAP_FILE=\"map\" -DMDMON_DIR=\"/run/mdadm\" -DFAILED_SLOTS_DIR=\"/run/mdadm/failed-slots\" -DNO_COROSYNC -DNO_DLM -DVERSION=\"4.0-10-g707cf0a\" -DVERS_DATE="\"2017-03-17\"" -DUSE_PTHREADS -DBINDIR=\"/sbin\"  -c -o Manage.o Manage.c
Manage.c: In function ‘Manage_remove_journal’:
Manage.c:199:26: error: ‘MD_SB_HAS_JOURNAL’ undeclared (first use in this function)
  if (!(array.state & (1<<MD_SB_HAS_JOURNAL))) {
                          ^~~~~~~~~~~~~~~~~
Manage.c:199:26: note: each undeclared identifier is reported only once for each function it appears in
Manage.c:203:33: error: ‘MD_SB_JOURNAL_REMOVABLE’ undeclared (first use in this function)
  } else if (!(array.state & (1<<MD_SB_JOURNAL_REMOVABLE))) {
                                 ^~~~~~~~~~~~~~~~~~~~~~~
<builtin>: recipe for target 'Manage.o' failed
make: *** [Manage.o] Error 1


^ permalink raw reply

* Re: [PATCH 2/2] mdadm: remove journal with "remove-journal"
From: jes.sorensen @ 2017-03-17 19:39 UTC (permalink / raw)
  To: Song Liu; +Cc: linux-raid, shli, neilb, kernel-team, dan.j.williams, hch
In-Reply-To: <20170315182850.3628010-2-songliubraving@fb.com>

Song Liu <songliubraving@fb.com> writes:
> When journal device of an array fails, the array is forced into read-only
> mode. To make the array normal without adding another journal device, we
> need to remove journal _feature_ from the array.
>
> This patch is the mdadm part of removing journal feature for an array with
> failed or missing journal.
>
> Two flags are added to GET_ARRAY_INFO:
>   1. MD_SB_HAS_JOURNAL: meaning the array have journal feature;
>   2. MD_SB_JOURNAL_REMOVABLE: meaning the journal is faulty or missing
>
> When both flags are set, mdadm will use ioctl SET_ARRAY_INFO to clear
> MD_SB_HAS_JOURNAL, and thus remove journal feature from the array.
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
>  Manage.c | 26 ++++++++++++++++++++++++++
>  ReadMe.c |  1 +
>  mdadm.c  |  9 ++++++++-
>  mdadm.h  |  3 +++
>  4 files changed, 38 insertions(+), 1 deletion(-)

Applied!

Thanks,
Jes

^ permalink raw reply

* Re: [PATCH 1/2] mdadm/r5cache: allow adding journal to array without journal
From: jes.sorensen @ 2017-03-17 19:38 UTC (permalink / raw)
  To: Song Liu; +Cc: linux-raid, neilb, kernel-team
In-Reply-To: <20170315182850.3628010-1-songliubraving@fb.com>

Song Liu <songliubraving@fb.com> writes:
> We use to apply constraints to --add-journal:
>   1. We only support recreate journal for arrays with missing journal;
>   2. The array need to be readonly when the journal is added.
>
> As the kernel code getting more mature, these constraints are no longer
> necessary. This patch removes this constraints from mdadm.
>
> Signed-off-by: Song Liu <songliubraving@fb.com>

Applied!

However, for multi-patch series, please always include a cover letter.
Just like with the kernel. Easier to repond to that than each patch
individually.

Thanks,
Jes

^ permalink raw reply

* Re: [PATCH 0/4] mdadm:checking level once mode has been set
From: jes.sorensen @ 2017-03-17 19:30 UTC (permalink / raw)
  To: Zhilong Liu; +Cc: linux-raid
In-Reply-To: <20170308074831.24683-1-zlliu@suse.com>

Zhilong Liu <zlliu@suse.com> writes:
> mdadm: it would be better to check --level ealier,
> because it would fall to different prompt if user
> forgets to specify the --level. such as:
> ./mdadm -CR /dev/md0 -b internal -n2 -x1 /dev/loop[0-2]
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>

This patchset is completely messed up - 0/4 contains actual code instead
of being a cover letter. 5/5 is a reply to 1/4 and you posted a v1 of
something which I assume needs to be the v2 given the v1 is the original
posting.

Would you mind sending out a clean set of these patches?

Thanks,
Jes

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Peter Zijlstra @ 2017-03-17 19:29 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Borislav Petkov, Alexander Potapenko, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke,
	x86@kernel.org, open list:KERNEL BUILD + fi..., LKML,
	linux-crypto, linux-raid, kbuild-all, Fengguang Wu
In-Reply-To: <20170317192642.qnrf7xuopxzapl2r@hirez.programming.kicks-ass.net>

On Fri, Mar 17, 2017 at 08:26:42PM +0100, Peter Zijlstra wrote:
> On Fri, Mar 17, 2017 at 08:05:16PM +0100, Dmitry Vyukov wrote:
> > You can also find some reasons in the Why section of LLVM-Linux project:
> > http://llvm.linuxfoundation.org/index.php/Main_Page
> 
> From that:
> 
>  - LLVM/Clang is a fast moving project with many things fixed quickly
>    and features added.
> 
> So what's the deal with that 5 year old bug you want us to work around?
> 
> Also, clang doesn't support asm cc flags output and a few other
> extensions last time I checked.
> 

Another great one:

 - BSD License (some people prefer this license to the GPL)

Seems a very weak argument to make when talking about the Linux Kernel
which is very explicitly GPLv2 (and not later).

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Peter Zijlstra @ 2017-03-17 19:27 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Alexander Potapenko, 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: <CA+=D-XXy9yEQc-jLxSqRUwq=4ADnFqaxnsjSzV4=8jOisym=bQ@mail.gmail.com>

On Fri, Mar 17, 2017 at 11:52:01AM -0700, Michael Davidson wrote:
> On Fri, Mar 17, 2017 at 5:44 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > 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.
> >
> 
> I agree that the code is horrible.
> 
> It is, in fact, exactly the same solution that was used to remove
> variable length arrays in structs from several of the crypto drivers a
> few years ago - see the definition of SHASH_DESC_ON_STACK() in
> "crypto/hash.h" - I did not, however, hide the horrors in a macro
> preferring to leave the implementation visible as a warning to whoever
> might touch the code next.
> 
> I believe that the actual stack usage is exactly the same as it was previously.
> 
> I can certainly wrap this  up in a macro and add comments with
> appropriately dire warnings in it if you feel that is both necessary
> and sufficient.

We got away with ugly in the past, so we should get to do it again?

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Peter Zijlstra @ 2017-03-17 19:26 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Borislav Petkov, Alexander Potapenko, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke,
	x86@kernel.org, open list:KERNEL BUILD + fi..., LKML,
	linux-crypto, linux-raid, kbuild-all, Fengguang Wu
In-Reply-To: <CACT4Y+bybdpqB71=inx8amwr188Mb2QyBeRTDdWZ3AFyDwVX0A@mail.gmail.com>

On Fri, Mar 17, 2017 at 08:05:16PM +0100, Dmitry Vyukov wrote:
> You can also find some reasons in the Why section of LLVM-Linux project:
> http://llvm.linuxfoundation.org/index.php/Main_Page

From that:

 - LLVM/Clang is a fast moving project with many things fixed quickly
   and features added.

So what's the deal with that 5 year old bug you want us to work around?

Also, clang doesn't support asm cc flags output and a few other
extensions last time I checked.


^ permalink raw reply

* Re: [RAID recovery] Unable to recover RAID5 array after disk failure
From: Olivier Swinkels @ 2017-03-17 19:25 UTC (permalink / raw)
  To: Phil Turmel; +Cc: linux-raid
In-Reply-To: <CAJ0Qwk+-Zz7RM8p7Ehy8aGuf+CQkPpbnBOaGNN77bP8cA5K9qg@mail.gmail.com>

On Wed, Mar 8, 2017 at 8:01 PM, Olivier Swinkels
<olivier.swinkels@gmail.com> wrote:
> On Tue, Mar 7, 2017 at 3:52 PM, Phil Turmel <philip@turmel.org> wrote:
>> On 03/07/2017 03:39 AM, Olivier Swinkels wrote:
>>
>>> After I used the pvcreate command to recreate pv the vgcfgrestore
>>> command succeeds and the lvm is available (after activating).
>>>
>>> However when I try to mount it I get the following error: sudo mount
>>> -t ext4 /dev/lvm-raid/lvm0 /mnt/raid mount: mount
>>> /dev/mapper/lvm--raid-lvm0 on /mnt/raid failed: Structure needs
>>> cleaning
>>>
>>> So I guess the underlying RAID array is still not ok...
>>
>> No, your underlying array is very likely correct.  But the intervening
>> incorrect --create operation stomped on your filesystems.  Run fsck
>> while unmounted to deal with the corruption and recover what you can.
>>
>> Run fsck with "-n" first, to see just how extensive the problems are,
>> then with "-y" to actually fix things.  Based on your sequence of
>> events, your corruptions should be at low sector addresses (first few
>> Gigs) of your array.  If that's what appears with "-n", proceed.
>>
>> If you are unlucky, the stompage hit one or more of your filesystems'
>> superblocks, requiring access to backup superblocks.  If you still see
>> no progress with either of the above, you might need to search your
>> array for ext2/3/4 superblocks.  This grep would help:
>>
>> dd if=/dev/md0 bs=1M count=16k 2>/dev/null |hexdump -C |grep '30  .\+  53 ef 0'
>>
>> (Not all hits from the grep will be superblocks, but they would be
>> visually distinguishable, and would have decipherable timestamps.)
>>
>> Phil
>
> Hi,
>
> I ran fsck -n disk on the lvm and got the very large response below.
> This didn't look promising, so I ran the ext2/3/4 superblock search.
> I didn't recognize any obvious timestamps, but i'm not sure what to look for.
> (I also pasted this output below)
>
> Can you recommend any further action?
>
> Olivier
>
> ===============================================================================
> fsck /dev/lvm-raid/lvm0
> fsck from util-linux 2.27.1
> ext2fs_check_desc: Corrupt group descriptor: bad block for block bitmap
> fsck.ext4: Group descriptors look bad... trying backup blocks...
> Warning: skipping journal recovery because doing a read-only filesystem check.
> /dev/mapper/lvm--raid-lvm0 has gone 1275 days without being checked,
> check forced.
> Pass 1: Checking inodes, blocks, and sizes
> Pass 2: Checking directory structure
> Pass 3: Checking directory connectivity
> Pass 4: Checking reference counts
> Pass 5: Checking group summary information
> Free blocks count wrong for group #0 (23513, counted=5372).
> Fix? no
> Free blocks count wrong for group #1 (980, counted=485).
> Fix? no
> Free blocks count wrong for group #3 (1023, counted=151).
> Fix? no
> Free blocks count wrong for group #5 (1022, counted=142).
> Fix? no
>
> <<TRIMMED ~200000 lines >>>
>
> Free inodes count wrong for group #60160 (8192, counted=8163).
> Fix? no
> Directories count wrong for group #60160 (0, counted=1).
> Fix? no
> Free inodes count wrong for group #60161 (8192, counted=8191).
> Fix? no
> Directories count wrong for group #60161 (0, counted=1).
> Fix? no
> Free inodes count wrong (488172170, counted=609035768).
> Fix? no
>
> /dev/mapper/lvm--raid-lvm0: ********** WARNING: Filesystem still has
> errors **********
> /dev/mapper/lvm--raid-lvm0: 122295670/610467840 files (0.0%
> non-contiguous), 2291846165/2441871360 blocks
> ===============================================================================
> dd if=/dev/md0 bs=1M count=16k 2>/dev/null |hexdump -C |grep '30  .\+  53 ef 0'
> 00040430  3a 3b ab 58 17 00 19 00  53 ef 01 00 01 00 00 00  |:;.X....S.......|
> 08040030  0c 58 34 52 02 00 19 00  53 ef 01 00 01 00 00 00  |.X4R....S.......|
> 18040030  0c 58 34 52 02 00 19 00  53 ef 01 00 01 00 00 00  |.X4R....S.......|
> 1d220e30  2e 93 9c 90 c1 89 27 62  53 ef 03 12 d4 82 36 76  |......'bS.....6v|
> 27340430  bb 78 c4 e3 9e 56 62 0f  53 ef 04 b9 38 0c d0 26  |.x...Vb.S...8..&|
> 28040030  0c 58 34 52 02 00 19 00  53 ef 01 00 01 00 00 00  |.X4R....S.......|
> 2f02ab30  5e d0 c9 98 83 ce 3b 92  53 ef 08 51 c4 4b dc af  |^.....;.S..Q.K..|
> 38040030  0c 58 34 52 02 00 19 00  53 ef 01 00 01 00 00 00  |.X4R....S.......|
> 44318c30  55 c1 19 f3 10 fb ab 2f  53 ef 04 0d fd c1 dc ed  |U....../S.......|
> 48040030  0c 58 34 52 02 00 19 00  53 ef 01 00 01 00 00 00  |.X4R....S.......|
> 50791730  6b 04 06 e8 97 88 9b 08  53 ef 0b 24 21 68 3d a5  |k.......S..$!h=.|
> 5aab3030  db 52 8d 5c 82 f4 80 cd  53 ef 08 4c f7 a7 c7 a9  |.R.\....S..L....|
> 98b69330  41 07 bb 64 c2 4a 00 5a  53 ef 08 71 88 d2 5a 42  |A..d.J.ZS..q..ZB|
> c8040030  0c 58 34 52 02 00 19 00  53 ef 01 00 01 00 00 00  |.X4R....S.......|
> cefc4130  14 37 b9 ef 1f 89 14 ab  53 ef 02 1c ca 88 f0 c4  |.7......S.......|
> d8040030  0c 58 34 52 02 00 19 00  53 ef 01 00 01 00 00 00  |.X4R....S.......|
> fe124d30  8a 6f 46 db 63 9b c5 9b  53 ef 07 97 74 24 2d e0  |.oF.c...S...t$-.|
> 106432f30  76 d0 fc 66 02 06 e9 50  53 ef 02 44 6c ab 89 ac  |v..f...PS..Dl...|
> 111e92630  f8 59 df b2 af 5d 6b 8b  53 ef 0d 65 c6 29 66 0c  |.Y...]k.S..e.)f.|
> 12883be30  88 63 b9 3a 6a 52 03 a6  53 ef 0c 12 67 dc ad 9e  |.c.:jR..S...g...|
> 14d315530  30 92 ef 27 a0 dc 46 dd  53 ef 09 d7 89 5a ba 95  |0..'..F.S....Z..|
> 17222e430  aa 7d 4f 4a c5 88 fa f0  53 ef 0c 08 04 10 9e ad  |.}OJ....S.......|
> 173717c30  3d fd 91 37 5a 0e 7b aa  53 ef 0b 08 d4 51 89 fa  |=..7Z.{.S....Q..|
> 17665ae30  c9 a8 24 d6 ba 0a 50 f7  53 ef 08 13 da d7 52 0a  |..$...P.S.....R.|
> 188040030  0c 58 34 52 02 00 19 00  53 ef 01 00 01 00 00 00  |.X4R....S.......|
> 18ef3be30  41 c3 91 02 65 af 8d 27  53 ef 0c 1f 4b 47 f5 97  |A...e..'S...KG..|
> 1943f1d30  d6 87 e0 d7 c1 3b e7 53  53 ef 08 a5 88 14 b6 c8  |.....;.SS.......|
> 19bfba930  c3 e4 07 04 b4 d1 05 4a  53 ef 02 c4 69 2f e2 d0  |.......JS...i/..|
> 1a807c530  a1 12 db b0 77 9d c5 6c  53 ef 0d 6e 37 05 74 61  |....w..lS..n7.ta|
> 1b1976330  1a 2b 57 56 46 96 c2 9b  53 ef 03 78 0d 9e 4a e0  |.+WVF...S..x..J.|
> 1ebf5db30  1e 76 bf 9d 25 8f fd 16  53 ef 04 ba 23 14 2f 8d  |.v..%...S...#./.|
> 1eec8b930  d5 dc 1e 90 b1 c8 f4 31  53 ef 06 10 3c 81 fb 37  |.......1S...<..7|
> 1f1392930  89 db 6c 92 6c 10 a5 f9  53 ef 05 31 0e 04 a2 d0  |..l.l...S..1....|
> 227cdff30  8d 8a 0a 61 91 04 c4 15  53 ef 03 5b 28 c4 52 59  |...a....S..[(.RY|
> 23000da30  1d e6 34 ca 2f 36 08 78  53 ef 0e 28 e4 94 5f 42  |..4./6.xS..(.._B|
> 245749e30  83 67 b7 d0 8b 40 0d f0  53 ef 0e ec 13 37 39 89  |.g...@..S....79.|
> 24f40e530  86 3b fc 43 ea ef 81 29  53 ef 09 6c fb 08 3b 6d  |.;.C...)S..l..;m|
> 25a473430  94 24 8d 8c 6b 15 7b 8b  53 ef 00 ae 43 b2 64 a4  |.$..k.{.S...C.d.|
> 288040030  0c 58 34 52 02 00 19 00  53 ef 01 00 01 00 00 00  |.X4R....S.......|
> 29966b530  00 ad 14 71 ea c5 c6 7a  53 ef 0c 11 d2 57 fb a0  |...q...zS....W..|
> 2a5924830  98 43 57 9b 2f 98 18 4f  53 ef 0d 27 72 c3 58 ba  |.CW./..OS..'r.X.|
> 2a7bf9b30  72 f2 a7 7c 73 4f 2f a6  53 ef 02 14 5b 7c 93 9e  |r..|sO/.S...[|..|
> 2aae14530  25 6b d5 ab b0 48 94 cd  53 ef 06 08 41 e0 0b ba  |%k...H..S...A...|
> 2ac470a30  a4 a6 cb a0 40 a2 8f cd  53 ef 06 47 3d d5 e0 38  |....@...S..G=..8|
> 2aca3fb30  99 e4 1a cd b1 a8 be 6c  53 ef 01 03 68 6f 4c f4  |.......lS...hoL.|
> 2b19b6030  30 64 6a d9 08 f0 46 c9  53 ef 07 95 b2 14 fb aa  |0dj...F.S.......|
> 2c53e0a30  59 1b 4b c8 25 a1 56 d9  53 ef 0b 26 3a 76 72 a5  |Y.K.%.V.S..&:vr.|
> 2cb48d730  0a d2 b4 d5 40 fc c5 d0  53 ef 0b 6d 90 9b db 22  |....@...S..m..."|
> 2dc925b30  16 18 5b 30 09 18 f8 2e  53 ef 0a d1 00 74 13 e7  |..[0....S....t..|
> 2dcc76230  e9 75 d0 2e 92 45 59 2c  53 ef 0e 84 63 9a aa b6  |.u...EY,S...c...|
> 2e6d28730  f4 81 47 6a 8f 9c ae 1a  53 ef 09 9d d2 5b e4 35  |..Gj....S....[.5|
> 2f9639f30  82 67 ae 49 2c a1 fc c0  53 ef 07 b1 1f 54 c5 c9  |.g.I,...S....T..|
> 2fa204230  78 42 16 12 e9 83 72 88  53 ef 00 08 df d9 8b 2a  |xB....r.S......*|
> 31b7c1030  99 f0 43 99 1f 52 77 17  53 ef 0c 5f b9 51 a1 42  |..C..Rw.S.._.Q.B|
> 321ad8a30  4e 9b 9e 2e c6 8e 19 8d  53 ef 0e 1d 5d 20 c0 9d  |N.......S...] ..|
> 32ff61630  72 9f 28 d2 9a 35 79 63  53 ef 09 e6 d6 e3 27 c5  |r.(..5ycS.....'.|
> 33a4b8230  58 9d 65 cb da 31 07 e7  53 ef 04 07 e2 4a 9b 17  |X.e..1..S....J..|
> 3472a7230  b4 a9 fa cf 3d 39 c3 95  53 ef 03 a4 07 2a ac 9a  |....=9..S....*..|
> 3534f5130  54 d2 19 77 a4 c2 4a 2f  53 ef 07 b3 0f 60 be ee  |T..w..J/S....`..|
> 356145130  df f7 f8 eb 6b 2a 8b fa  53 ef 09 cf 9d cd db 68  |....k*..S......h|
> 361b68830  53 ab 0c 7a 8e 4e 96 1b  53 ef 0f 3b 78 e9 d5 ce  |S..z.N..S..;x...|
> 37381e630  24 63 4a ce 1b eb b0 df  53 ef 02 68 54 7d 7a ad  |$cJ.....S..hT}z.|
> 375103330  3a 20 84 18 b6 6d f5 3b  53 ef 0a 86 8f ac b2 d8  |: ...m.;S.......|
> 37b3f7230  11 43 a4 46 fd c8 da ae  53 ef 04 f3 80 db 5c 4b  |.C.F....S.....\K|
> 3a5adf530  7f c1 e1 75 26 cf 25 b2  53 ef 01 4d 64 71 10 bf  |...u&.%.S..Mdq..|
> 3ccc7b830  55 cb 6f 68 e3 c0 0f 36  53 ef 06 00 85 8a 17 72  |U.oh...6S......r|
> 3d6743f30  fa ca 0e 36 a6 4e 02 6a  53 ef 05 6d 6f e5 31 78  |...6.N.jS..mo.1x|
> 3e8040030  0c 58 34 52 02 00 19 00  53 ef 01 00 01 00 00 00  |.X4R....S.......|
>
> ===============================================================================

Hi Phil,

Did you already have time to look at the results of the fsck check and
ext2/3/4 superblock search?
I would really like some feedback, as I'm quite out of ideas.

Thanks!

Olivier

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Dmitry Vyukov @ 2017-03-17 19:05 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Alexander Potapenko, Peter Zijlstra, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke,
	x86@kernel.org, open list:KERNEL BUILD + fi..., LKML,
	linux-crypto, linux-raid, kbuild-all, Fengguang Wu
In-Reply-To: <20170317185720.5s7qa6hl233t24ag@pd.tnic>

On Fri, Mar 17, 2017 at 7:57 PM, Borislav Petkov <bp@alien8.de> wrote:
> On Fri, Mar 17, 2017 at 07:47:33PM +0100, Dmitry Vyukov wrote:
>> This problem is more general and is not specific to clang. It equally
>> applies to different versions of gcc, different arches and different
>> configs (namely, anything else than what a developer used for
>> testing).
>
> I guess. We do carry a bunch of gcc workarounds along with the cc-*
> macros in scripts/Kbuild.include.
>
>> A known, reasonably well working solution to this problem is
>> a system of try bots that test patches before commit with different
>> compilers/configs/archs. We already have such system in the form of
>> 0-day bots. It would be useful to extend it with clang as soon as
>> kernel builds.
>
> Has someone actually already talked to Fengguang about it?

+Fengguang

> Oh, and the stupid question: why the effort to build the kernel
> with clang at all? Just because or are there some actual, palpable
> advantages?

On our side it is:
 - clang make it possible to implement KMSAN (dynamic detection of
uses of uninit memory)
 - better code coverage for fuzzing
 - why simpler and faster development (e.g. we can port our user-space
hardening technologies -- CFI and SafeStack)

You can also find some reasons in the Why section of LLVM-Linux project:
http://llvm.linuxfoundation.org/index.php/Main_Page

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Borislav Petkov @ 2017-03-17 18:57 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Alexander Potapenko, Peter Zijlstra, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke,
	x86@kernel.org, open list:KERNEL BUILD + fi..., LKML,
	linux-crypto, linux-raid, kbuild-all
In-Reply-To: <CACT4Y+Ysvjz+UzEi0dTtsNxxnG7WXTiZPr+KSk8HHwSu7vX1ew@mail.gmail.com>

On Fri, Mar 17, 2017 at 07:47:33PM +0100, Dmitry Vyukov wrote:
> This problem is more general and is not specific to clang. It equally
> applies to different versions of gcc, different arches and different
> configs (namely, anything else than what a developer used for
> testing).

I guess. We do carry a bunch of gcc workarounds along with the cc-*
macros in scripts/Kbuild.include.

> A known, reasonably well working solution to this problem is
> a system of try bots that test patches before commit with different
> compilers/configs/archs. We already have such system in the form of
> 0-day bots. It would be useful to extend it with clang as soon as
> kernel builds.

Has someone actually already talked to Fengguang about it?

Oh, and the stupid question: why the effort to build the kernel
with clang at all? Just because or are there some actual, palpable
advantages?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Michael Davidson @ 2017-03-17 18:52 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Alexander Potapenko, 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: <20170317124404.mt3jd5q5vyk63q2w@hirez.programming.kicks-ass.net>

On Fri, Mar 17, 2017 at 5:44 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> 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.
>

I agree that the code is horrible.

It is, in fact, exactly the same solution that was used to remove
variable length arrays in structs from several of the crypto drivers a
few years ago - see the definition of SHASH_DESC_ON_STACK() in
"crypto/hash.h" - I did not, however, hide the horrors in a macro
preferring to leave the implementation visible as a warning to whoever
might touch the code next.

I believe that the actual stack usage is exactly the same as it was previously.

I can certainly wrap this  up in a macro and add comments with
appropriately dire warnings in it if you feel that is both necessary
and sufficient.

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Dmitry Vyukov @ 2017-03-17 18:47 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Alexander Potapenko, Peter Zijlstra, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke,
	x86@kernel.org, open list:KERNEL BUILD + fi..., LKML,
	linux-crypto, linux-raid, kbuild-all
In-Reply-To: <20170317180350.63jjysejk2i6vkon@pd.tnic>

On Fri, Mar 17, 2017 at 7:03 PM, Borislav Petkov <bp@alien8.de> wrote:
> On Fri, Mar 17, 2017 at 01:32:00PM +0100, Alexander Potapenko wrote:
>> > 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?
>
> And what happens if someone else adds a variable thing like this
> somewhere else, builds with gcc, everything's fine and patch gets
> applied? Or something else llvm can't stomach.
>
> Does that mean there'll be the occasional, every-so-often whack-a-mole
> patchset from someone, fixing the kernel build with llvm yet again?


This problem is more general and is not specific to clang. It equally
applies to different versions of gcc, different arches and different
configs (namely, anything else than what a developer used for
testing). A known, reasonably well working solution to this problem is
a system of try bots that test patches before commit with different
compilers/configs/archs. We already have such system in the form of
0-day bots. It would be useful to extend it with clang as soon as
kernel builds.

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Borislav Petkov @ 2017-03-17 18:03 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Peter Zijlstra, 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=WooTLkzex3hqPgO5pr7GsK3v7Z5riSA7K+-_S=S7rmBQ@mail.gmail.com>

On Fri, Mar 17, 2017 at 01:32:00PM +0100, Alexander Potapenko wrote:
> > 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?

And what happens if someone else adds a variable thing like this
somewhere else, builds with gcc, everything's fine and patch gets
applied? Or something else llvm can't stomach.

Does that mean there'll be the occasional, every-so-often whack-a-mole
patchset from someone, fixing the kernel build with llvm yet again?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* 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


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