* [PATCH 10/23] afs: switch to use uuid_t and uuid_gen
From: Christoph Hellwig @ 2017-05-18 6:26 UTC (permalink / raw)
To: Andy Shevchenko, Amir Goldstein,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Mimi Zohar,
Steven Whitehouse
In-Reply-To: <20170518062705.25902-1-hch-jcswGhMUV9g@public.gmane.org>
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Reviewed-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/afs/cmservice.c | 46 +++++++++++++++++++++++-----------------------
fs/afs/internal.h | 2 +-
fs/afs/main.c | 4 ++--
3 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c
index 3062cceb5c2a..d4e77d12570c 100644
--- a/fs/afs/cmservice.c
+++ b/fs/afs/cmservice.c
@@ -350,7 +350,7 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
{
struct sockaddr_rxrpc srx;
struct afs_server *server;
- struct uuid_v1 *r;
+ uuid_t *r;
unsigned loop;
__be32 *b;
int ret;
@@ -380,20 +380,20 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
}
_debug("unmarshall UUID");
- call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
+ call->request = kmalloc(sizeof(uuid_t), GFP_KERNEL);
if (!call->request)
return -ENOMEM;
b = call->buffer;
r = call->request;
- r->time_low = b[0];
- r->time_mid = htons(ntohl(b[1]));
- r->time_hi_and_version = htons(ntohl(b[2]));
- r->clock_seq_hi_and_reserved = ntohl(b[3]);
- r->clock_seq_low = ntohl(b[4]);
+ r->v1.time_low = b[0];
+ r->v1.time_mid = htons(ntohl(b[1]));
+ r->v1.time_hi_and_version = htons(ntohl(b[2]));
+ r->v1.clock_seq_hi_and_reserved = ntohl(b[3]);
+ r->v1.clock_seq_low = ntohl(b[4]);
for (loop = 0; loop < 6; loop++)
- r->node[loop] = ntohl(b[loop + 5]);
+ r->v1.node[loop] = ntohl(b[loop + 5]);
call->offset = 0;
call->unmarshall++;
@@ -453,7 +453,7 @@ static int afs_deliver_cb_probe(struct afs_call *call)
static void SRXAFSCB_ProbeUuid(struct work_struct *work)
{
struct afs_call *call = container_of(work, struct afs_call, work);
- struct uuid_v1 *r = call->request;
+ uuid_t *r = call->request;
struct {
__be32 match;
@@ -476,7 +476,7 @@ static void SRXAFSCB_ProbeUuid(struct work_struct *work)
*/
static int afs_deliver_cb_probe_uuid(struct afs_call *call)
{
- struct uuid_v1 *r;
+ uuid_t *r;
unsigned loop;
__be32 *b;
int ret;
@@ -502,20 +502,20 @@ static int afs_deliver_cb_probe_uuid(struct afs_call *call)
}
_debug("unmarshall UUID");
- call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
+ call->request = kmalloc(sizeof(uuid_t), GFP_KERNEL);
if (!call->request)
return -ENOMEM;
b = call->buffer;
r = call->request;
- r->time_low = b[0];
- r->time_mid = htons(ntohl(b[1]));
- r->time_hi_and_version = htons(ntohl(b[2]));
- r->clock_seq_hi_and_reserved = ntohl(b[3]);
- r->clock_seq_low = ntohl(b[4]);
+ r->v1.time_low = b[0];
+ r->v1.time_mid = htons(ntohl(b[1]));
+ r->v1.time_hi_and_version = htons(ntohl(b[2]));
+ r->v1.clock_seq_hi_and_reserved = ntohl(b[3]);
+ r->v1.clock_seq_low = ntohl(b[4]);
for (loop = 0; loop < 6; loop++)
- r->node[loop] = ntohl(b[loop + 5]);
+ r->v1.node[loop] = ntohl(b[loop + 5]);
call->offset = 0;
call->unmarshall++;
@@ -568,13 +568,13 @@ static void SRXAFSCB_TellMeAboutYourself(struct work_struct *work)
memset(&reply, 0, sizeof(reply));
reply.ia.nifs = htonl(nifs);
- reply.ia.uuid[0] = afs_uuid.time_low;
- reply.ia.uuid[1] = htonl(ntohs(afs_uuid.time_mid));
- reply.ia.uuid[2] = htonl(ntohs(afs_uuid.time_hi_and_version));
- reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
- reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
+ reply.ia.uuid[0] = afs_uuid.v1.time_low;
+ reply.ia.uuid[1] = htonl(ntohs(afs_uuid.v1.time_mid));
+ reply.ia.uuid[2] = htonl(ntohs(afs_uuid.v1.time_hi_and_version));
+ reply.ia.uuid[3] = htonl((s8) afs_uuid.v1.clock_seq_hi_and_reserved);
+ reply.ia.uuid[4] = htonl((s8) afs_uuid.v1.clock_seq_low);
for (loop = 0; loop < 6; loop++)
- reply.ia.uuid[loop + 5] = htonl((s8) afs_uuid.node[loop]);
+ reply.ia.uuid[loop + 5] = htonl((s8) afs_uuid.v1.node[loop]);
if (ifs) {
for (loop = 0; loop < nifs; loop++) {
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 393672997cc2..7de45c8686a2 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -544,7 +544,7 @@ extern int afs_drop_inode(struct inode *);
* main.c
*/
extern struct workqueue_struct *afs_wq;
-extern struct uuid_v1 afs_uuid;
+extern uuid_t afs_uuid;
/*
* misc.c
diff --git a/fs/afs/main.c b/fs/afs/main.c
index 51d7d17bca57..75b3d3a8b1ba 100644
--- a/fs/afs/main.c
+++ b/fs/afs/main.c
@@ -31,7 +31,7 @@ static char *rootcell;
module_param(rootcell, charp, 0);
MODULE_PARM_DESC(rootcell, "root AFS cell name and VL server IP addr list");
-struct uuid_v1 afs_uuid;
+uuid_t afs_uuid;
struct workqueue_struct *afs_wq;
/*
@@ -43,7 +43,7 @@ static int __init afs_init(void)
printk(KERN_INFO "kAFS: Red Hat AFS client v0.1 registering.\n");
- generate_random_uuid((unsigned char *)&afs_uuid);
+ uuid_gen(&afs_uuid);
/* create workqueue */
ret = -ENOMEM;
--
2.11.0
^ permalink raw reply related
* [PATCH 09/23] S390/sysinfo: use uuid_is_null instead of opencoding it
From: Christoph Hellwig @ 2017-05-18 6:26 UTC (permalink / raw)
To: Andy Shevchenko, Amir Goldstein,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Mimi Zohar,
Steven Whitehouse
In-Reply-To: <20170518062705.25902-1-hch-jcswGhMUV9g@public.gmane.org>
.. and switch to use uuid_t instead of the old uuid_be type while we're
at it.
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
arch/s390/include/asm/sysinfo.h | 4 ++--
arch/s390/kernel/sysinfo.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/s390/include/asm/sysinfo.h b/arch/s390/include/asm/sysinfo.h
index 73bff45ced55..e7f8e1b77d92 100644
--- a/arch/s390/include/asm/sysinfo.h
+++ b/arch/s390/include/asm/sysinfo.h
@@ -109,7 +109,7 @@ struct sysinfo_2_2_2 {
unsigned short cpus_shared;
char reserved_4[3];
unsigned char vsne;
- uuid_be uuid;
+ uuid_t uuid;
char reserved_5[160];
char ext_name[256];
};
@@ -134,7 +134,7 @@ struct sysinfo_3_2_2 {
char reserved_1[3];
unsigned char evmne;
unsigned int reserved_2;
- uuid_be uuid;
+ uuid_t uuid;
} vm[8];
char reserved_3[1504];
char ext_names[8][256];
diff --git a/arch/s390/kernel/sysinfo.c b/arch/s390/kernel/sysinfo.c
index eefcb54872a5..fb869b103825 100644
--- a/arch/s390/kernel/sysinfo.c
+++ b/arch/s390/kernel/sysinfo.c
@@ -242,7 +242,7 @@ static void print_ext_name(struct seq_file *m, int lvl,
static void print_uuid(struct seq_file *m, int i, struct sysinfo_3_2_2 *info)
{
- if (!memcmp(&info->vm[i].uuid, &NULL_UUID_BE, sizeof(uuid_be)))
+ if (uuid_is_null(&info->vm[i].uuid))
return;
seq_printf(m, "VM%02d UUID: %pUb\n", i, &info->vm[i].uuid);
}
--
2.11.0
^ permalink raw reply related
* [PATCH 08/23] uuid: hoist uuid_is_null() helper from libnvdimm
From: Christoph Hellwig @ 2017-05-18 6:26 UTC (permalink / raw)
To: Andy Shevchenko, Amir Goldstein,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Mimi Zohar,
Steven Whitehouse
In-Reply-To: <20170518062705.25902-1-hch-jcswGhMUV9g@public.gmane.org>
Hoist the libnvdimm helper as an inline helper to linux/uuid.h
using an auxiliary const variable uuid_null in lib/uuid.c.
[hch: also add the guid variant. Both do the same but I'd like
to keep casts to a minimum]
The common helper uses the new abstract type uuid_t * instead of
u8 *.
Suggested-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Cc: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Amir Goldstein <amir73il-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[hch: added guid_is_null]
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Acked-by: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/nvdimm/btt_devs.c | 9 +--------
include/linux/uuid.h | 13 +++++++++++++
lib/uuid.c | 5 +++++
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/nvdimm/btt_devs.c b/drivers/nvdimm/btt_devs.c
index ae00dc0d9791..4c989bb9a8a0 100644
--- a/drivers/nvdimm/btt_devs.c
+++ b/drivers/nvdimm/btt_devs.c
@@ -222,13 +222,6 @@ struct device *nd_btt_create(struct nd_region *nd_region)
return dev;
}
-static bool uuid_is_null(u8 *uuid)
-{
- static const u8 null_uuid[16];
-
- return (memcmp(uuid, null_uuid, 16) == 0);
-}
-
/**
* nd_btt_arena_is_valid - check if the metadata layout is valid
* @nd_btt: device with BTT geometry and backing device info
@@ -249,7 +242,7 @@ bool nd_btt_arena_is_valid(struct nd_btt *nd_btt, struct btt_sb *super)
if (memcmp(super->signature, BTT_SIG, BTT_SIG_LEN) != 0)
return false;
- if (!uuid_is_null(super->parent_uuid))
+ if (!guid_is_null((guid_t *)&super->parent_uuid))
if (memcmp(super->parent_uuid, parent_uuid, 16) != 0)
return false;
diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 7aff929007dd..47ca06d846d4 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -64,6 +64,9 @@ typedef struct {
*/
#define UUID_STRING_LEN 36
+extern const guid_t guid_null;
+extern const uuid_t uuid_null;
+
static inline bool guid_equal(const guid_t *u1, const guid_t *u2)
{
return memcmp(u1, u2, sizeof(guid_t)) == 0;
@@ -74,6 +77,11 @@ static inline void guid_copy(guid_t *dst, const guid_t *src)
memcpy(dst, src, sizeof(guid_t));
}
+static inline bool guid_is_null(guid_t *guid)
+{
+ return guid_equal(guid, &guid_null);
+}
+
static inline bool uuid_equal(const uuid_t *u1, const uuid_t *u2)
{
return memcmp(u1, u2, sizeof(uuid_t)) == 0;
@@ -84,6 +92,11 @@ static inline void uuid_copy(uuid_t *dst, const uuid_t *src)
memcpy(dst, src, sizeof(uuid_t));
}
+static inline bool uuid_is_null(uuid_t *uuid)
+{
+ return uuid_equal(uuid, &uuid_null);
+}
+
void generate_random_uuid(unsigned char uuid[16]);
extern void guid_gen(guid_t *u);
diff --git a/lib/uuid.c b/lib/uuid.c
index 003bf6823003..25145bfb0eaa 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -21,6 +21,11 @@
#include <linux/uuid.h>
#include <linux/random.h>
+const guid_t guid_null;
+EXPORT_SYMBOL(guid_null);
+const uuid_t uuid_null;
+EXPORT_SYMBOL(uuid_null);
+
const u8 guid_index[16] = {3,2,1,0,5,4,7,6,8,9,10,11,12,13,14,15};
const u8 uuid_index[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
--
2.11.0
^ permalink raw reply related
* [PATCH 07/23] uuid: hoist helpers uuid_equal() and uuid_copy() from xfs
From: Christoph Hellwig @ 2017-05-18 6:26 UTC (permalink / raw)
To: Andy Shevchenko, Amir Goldstein, linux-fsdevel
Cc: Shaohua Li, Dan Williams, David Howells, Steven Whitehouse,
Mimi Zohar, linux-xfs, linux-raid, linux-nvdimm, linux-kernel
In-Reply-To: <20170518062705.25902-1-hch@lst.de>
These helper are used to compare and copy two uuid_t type objects.
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
[hch: also provide the respective guid_ versions]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/uuid.c | 6 ------
fs/xfs/uuid.h | 7 -------
include/linux/uuid.h | 20 ++++++++++++++++++++
lib/test_uuid.c | 4 ++--
4 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/fs/xfs/uuid.c b/fs/xfs/uuid.c
index b83f76b6d410..29ed78c8637b 100644
--- a/fs/xfs/uuid.c
+++ b/fs/xfs/uuid.c
@@ -55,9 +55,3 @@ uuid_is_nil(uuid_t *uuid)
if (*cp++) return 0; /* not nil */
return 1; /* is nil */
}
-
-int
-uuid_equal(uuid_t *uuid1, uuid_t *uuid2)
-{
- return memcmp(uuid1, uuid2, sizeof(uuid_t)) ? 0 : 1;
-}
diff --git a/fs/xfs/uuid.h b/fs/xfs/uuid.h
index 4f1441ba4fa5..86bbed071e79 100644
--- a/fs/xfs/uuid.h
+++ b/fs/xfs/uuid.h
@@ -19,13 +19,6 @@
#define __XFS_SUPPORT_UUID_H__
extern int uuid_is_nil(uuid_t *uuid);
-extern int uuid_equal(uuid_t *uuid1, uuid_t *uuid2);
extern void uuid_getnodeuniq(uuid_t *uuid, int fsid [2]);
-static inline void
-uuid_copy(uuid_t *dst, uuid_t *src)
-{
- memcpy(dst, src, sizeof(uuid_t));
-}
-
#endif /* __XFS_SUPPORT_UUID_H__ */
diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 8bd1da4ec95d..7aff929007dd 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -64,6 +64,26 @@ typedef struct {
*/
#define UUID_STRING_LEN 36
+static inline bool guid_equal(const guid_t *u1, const guid_t *u2)
+{
+ return memcmp(u1, u2, sizeof(guid_t)) == 0;
+}
+
+static inline void guid_copy(guid_t *dst, const guid_t *src)
+{
+ memcpy(dst, src, sizeof(guid_t));
+}
+
+static inline bool uuid_equal(const uuid_t *u1, const uuid_t *u2)
+{
+ return memcmp(u1, u2, sizeof(uuid_t)) == 0;
+}
+
+static inline void uuid_copy(uuid_t *dst, const uuid_t *src)
+{
+ memcpy(dst, src, sizeof(uuid_t));
+}
+
void generate_random_uuid(unsigned char uuid[16]);
extern void guid_gen(guid_t *u);
diff --git a/lib/test_uuid.c b/lib/test_uuid.c
index 9cad846fd805..4d05189e5f87 100644
--- a/lib/test_uuid.c
+++ b/lib/test_uuid.c
@@ -71,7 +71,7 @@ static void __init test_uuid_test(const struct test_uuid_data *data)
test_uuid_failed("conversion", false, false, data->uuid, NULL);
total_tests++;
- if (uuid_le_cmp(data->le, le)) {
+ if (!guid_equal(&data->le, &le)) {
sprintf(buf, "%pUl", &le);
test_uuid_failed("cmp", false, false, data->uuid, buf);
}
@@ -82,7 +82,7 @@ static void __init test_uuid_test(const struct test_uuid_data *data)
test_uuid_failed("conversion", false, true, data->uuid, NULL);
total_tests++;
- if (uuid_be_cmp(data->be, be)) {
+ if (uuid_equal(&data->be, &be)) {
sprintf(buf, "%pUb", &be);
test_uuid_failed("cmp", false, true, data->uuid, buf);
}
--
2.11.0
^ permalink raw reply related
* [PATCH 06/23] uuid: add the v1 layout to uuid_t
From: Christoph Hellwig @ 2017-05-18 6:26 UTC (permalink / raw)
To: Andy Shevchenko, Amir Goldstein,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Mimi Zohar,
Steven Whitehouse
In-Reply-To: <20170518062705.25902-1-hch-jcswGhMUV9g@public.gmane.org>
Turn the content of uuid_t into a union and add the fields for the v1
interpretation to it.
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Reviewed-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
include/linux/uuid.h | 59 ++++++++++++++++++++++++++++------------------------
1 file changed, 32 insertions(+), 27 deletions(-)
diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 0410cd002282..8bd1da4ec95d 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -18,40 +18,45 @@
#include <uapi/linux/uuid.h>
-typedef struct {
- __u8 b[16];
-} uuid_t;
-
-#define UUID(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
-((uuid_t) \
-{{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \
- ((b) >> 8) & 0xff, (b) & 0xff, \
- ((c) >> 8) & 0xff, (c) & 0xff, \
- (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
-
-/*
- * V1 (time-based) UUID definition [RFC 4122].
- * - the timestamp is a 60-bit value, split 32/16/12, and goes in 100ns
- * increments since midnight 15th October 1582
- * - add AFS_UUID_TO_UNIX_TIME to convert unix time in 100ns units to UUID
- * time
- * - the clock sequence is a 14-bit counter to avoid duplicate times
- */
-struct uuid_v1 {
- __be32 time_low; /* low part of timestamp */
- __be16 time_mid; /* mid part of timestamp */
- __be16 time_hi_and_version; /* high part of timestamp and version */
+/* v1 UUID defintions: */
#define UUID_TO_UNIX_TIME 0x01b21dd213814000ULL
#define UUID_TIMEHI_MASK 0x0fff
#define UUID_VERSION_TIME 0x1000 /* time-based UUID */
#define UUID_VERSION_NAME 0x3000 /* name-based UUID */
#define UUID_VERSION_RANDOM 0x4000 /* (pseudo-)random generated UUID */
- u8 clock_seq_hi_and_reserved; /* clock seq hi and variant */
#define UUID_CLOCKHI_MASK 0x3f
#define UUID_VARIANT_STD 0x80
- u8 clock_seq_low; /* clock seq low */
- u8 node[6]; /* spatially unique node ID (MAC addr) */
-};
+
+typedef struct {
+ union {
+ __u8 b[16];
+
+ /*
+ * V1 (time-based) UUID definition [RFC 4122]:
+ * - the timestamp is a 60-bit value, split 32/16/12, and goes
+ * in 100ns increments since midnight 15th October 1582
+ * - add UUID_TO_UNIX_TIME to convert unix time in 100ns
+ * units to UUID time
+ * - the clock sequence is a 14-bit counter to avoid duplicate
+ * times
+ */
+ struct uuid_v1 {
+ __be32 time_low;
+ __be16 time_mid;
+ __be16 time_hi_and_version;
+ u8 clock_seq_hi_and_reserved;
+ u8 clock_seq_low;
+ u8 node[6];
+ } v1;
+ };
+} uuid_t;
+
+#define UUID(a, _b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
+{.b = { \
+ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \
+ ((_b) >> 8) & 0xff, (_b) & 0xff, \
+ ((c) >> 8) & 0xff, (c) & 0xff, \
+ (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }}
/*
* The length of a UUID string ("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
--
2.11.0
^ permalink raw reply related
* [PATCH 05/23] uuid: don't export guid_index and uuid_index
From: Christoph Hellwig @ 2017-05-18 6:26 UTC (permalink / raw)
To: Andy Shevchenko, Amir Goldstein,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Mimi Zohar,
Steven Whitehouse
In-Reply-To: <20170518062705.25902-1-hch-jcswGhMUV9g@public.gmane.org>
These are only used in uuid.c and vsprintf.c and aren't something modules
should use directly.
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
lib/uuid.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/lib/uuid.c b/lib/uuid.c
index f80dc63f6ca8..003bf6823003 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -22,9 +22,7 @@
#include <linux/random.h>
const u8 guid_index[16] = {3,2,1,0,5,4,7,6,8,9,10,11,12,13,14,15};
-EXPORT_SYMBOL(guid_index);
const u8 uuid_index[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
-EXPORT_SYMBOL(uuid_index);
/***************************************************************
* Random UUID interface
--
2.11.0
^ permalink raw reply related
* [PATCH 04/23] uuid: rename uuid types
From: Christoph Hellwig @ 2017-05-18 6:26 UTC (permalink / raw)
To: Andy Shevchenko, Amir Goldstein,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Mimi Zohar,
Steven Whitehouse
In-Reply-To: <20170518062705.25902-1-hch-jcswGhMUV9g@public.gmane.org>
Our "little endian" UUID really is a Wintel GUID, so rename it and its
helpers such (guid_t). The big endian UUID is the only true one, so
give it the name uuid_t. The uuid_le and uuid_be names are retained for
now, but will hopefully go away soon. The exception to that are the _cmp
helpers that will be replaced by better primites ASAP and thus don't
get the new names.
Also remove the existing typedef in XFS that's now been superceeded by
the generic type name.
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
[andy: also update the UUID_LE/UUID_BE macros including fallout]
Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
fs/xfs/xfs_linux.h | 2 --
include/linux/uuid.h | 55 +++++++++++++++++++++++++++--------------------
include/uapi/linux/uuid.h | 12 +++++++----
lib/test_uuid.c | 32 +++++++++++++--------------
lib/uuid.c | 28 ++++++++++++------------
lib/vsprintf.c | 4 ++--
6 files changed, 72 insertions(+), 61 deletions(-)
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index 89ee5ec66837..2c33d915e550 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -39,8 +39,6 @@ typedef __s64 xfs_daddr_t; /* <disk address> type */
typedef __u32 xfs_dev_t;
typedef __u32 xfs_nlink_t;
-typedef uuid_be uuid_t;
-
#include "xfs_types.h"
#include "kmem.h"
diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index de3aea206562..0410cd002282 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -20,19 +20,15 @@
typedef struct {
__u8 b[16];
-} uuid_be;
+} uuid_t;
-#define UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
-((uuid_be) \
+#define UUID(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
+((uuid_t) \
{{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \
((b) >> 8) & 0xff, (b) & 0xff, \
((c) >> 8) & 0xff, (c) & 0xff, \
(d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
-#define NULL_UUID_BE \
- UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, \
- 0x00, 0x00, 0x00, 0x00)
-
/*
* V1 (time-based) UUID definition [RFC 4122].
* - the timestamp is a 60-bit value, split 32/16/12, and goes in 100ns
@@ -63,27 +59,40 @@ struct uuid_v1 {
*/
#define UUID_STRING_LEN 36
-static inline int uuid_le_cmp(const uuid_le u1, const uuid_le u2)
-{
- return memcmp(&u1, &u2, sizeof(uuid_le));
-}
-
-static inline int uuid_be_cmp(const uuid_be u1, const uuid_be u2)
-{
- return memcmp(&u1, &u2, sizeof(uuid_be));
-}
-
void generate_random_uuid(unsigned char uuid[16]);
-extern void uuid_le_gen(uuid_le *u);
-extern void uuid_be_gen(uuid_be *u);
+extern void guid_gen(guid_t *u);
+extern void uuid_gen(uuid_t *u);
bool __must_check uuid_is_valid(const char *uuid);
-extern const u8 uuid_le_index[16];
-extern const u8 uuid_be_index[16];
+extern const u8 guid_index[16];
+extern const u8 uuid_index[16];
+
+int guid_to_bin(const char *uuid, guid_t *u);
+int uuid_to_bin(const char *uuid, uuid_t *u);
-int uuid_le_to_bin(const char *uuid, uuid_le *u);
-int uuid_be_to_bin(const char *uuid, uuid_be *u);
+/* backwards compatibility, don't use in new code */
+typedef uuid_t uuid_be;
+#define UUID_BE(a, _b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
+ UUID(a, _b, c, d0, d1, d2, d3, d4, d5, d6, d7)
+#define NULL_UUID_BE \
+ UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, \
+ 0x00, 0x00, 0x00, 0x00)
+
+#define uuid_le_gen(u) guid_gen(u)
+#define uuid_be_gen(u) uuid_gen(u)
+#define uuid_le_to_bin(guid, u) guid_to_bin(guid, u)
+#define uuid_be_to_bin(uuid, u) uuid_to_bin(uuid, u)
+
+static inline int uuid_le_cmp(const guid_t u1, const guid_t u2)
+{
+ return memcmp(&u1, &u2, sizeof(guid_t));
+}
+
+static inline int uuid_be_cmp(const uuid_t u1, const uuid_t u2)
+{
+ return memcmp(&u1, &u2, sizeof(uuid_t));
+}
#endif
diff --git a/include/uapi/linux/uuid.h b/include/uapi/linux/uuid.h
index 0099756c4bac..1eeeca973315 100644
--- a/include/uapi/linux/uuid.h
+++ b/include/uapi/linux/uuid.h
@@ -22,17 +22,21 @@
typedef struct {
__u8 b[16];
-} uuid_le;
+} guid_t;
-#define UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
-((uuid_le) \
+#define GUID(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
+((guid_t) \
{{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \
(b) & 0xff, ((b) >> 8) & 0xff, \
(c) & 0xff, ((c) >> 8) & 0xff, \
(d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
+/* backwards compatibility, don't use in new code */
+typedef guid_t uuid_le;
+#define UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
+ GUID(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)
#define NULL_UUID_LE \
UUID_LE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, \
- 0x00, 0x00, 0x00, 0x00)
+ 0x00, 0x00, 0x00, 0x00)
#endif /* _UAPI_LINUX_UUID_H_ */
diff --git a/lib/test_uuid.c b/lib/test_uuid.c
index 547d3127a3cf..9cad846fd805 100644
--- a/lib/test_uuid.c
+++ b/lib/test_uuid.c
@@ -11,25 +11,25 @@
struct test_uuid_data {
const char *uuid;
- uuid_le le;
- uuid_be be;
+ guid_t le;
+ uuid_t be;
};
static const struct test_uuid_data test_uuid_test_data[] = {
{
.uuid = "c33f4995-3701-450e-9fbf-206a2e98e576",
- .le = UUID_LE(0xc33f4995, 0x3701, 0x450e, 0x9f, 0xbf, 0x20, 0x6a, 0x2e, 0x98, 0xe5, 0x76),
- .be = UUID_BE(0xc33f4995, 0x3701, 0x450e, 0x9f, 0xbf, 0x20, 0x6a, 0x2e, 0x98, 0xe5, 0x76),
+ .le = GUID(0xc33f4995, 0x3701, 0x450e, 0x9f, 0xbf, 0x20, 0x6a, 0x2e, 0x98, 0xe5, 0x76),
+ .be = UUID(0xc33f4995, 0x3701, 0x450e, 0x9f, 0xbf, 0x20, 0x6a, 0x2e, 0x98, 0xe5, 0x76),
},
{
.uuid = "64b4371c-77c1-48f9-8221-29f054fc023b",
- .le = UUID_LE(0x64b4371c, 0x77c1, 0x48f9, 0x82, 0x21, 0x29, 0xf0, 0x54, 0xfc, 0x02, 0x3b),
- .be = UUID_BE(0x64b4371c, 0x77c1, 0x48f9, 0x82, 0x21, 0x29, 0xf0, 0x54, 0xfc, 0x02, 0x3b),
+ .le = GUID(0x64b4371c, 0x77c1, 0x48f9, 0x82, 0x21, 0x29, 0xf0, 0x54, 0xfc, 0x02, 0x3b),
+ .be = UUID(0x64b4371c, 0x77c1, 0x48f9, 0x82, 0x21, 0x29, 0xf0, 0x54, 0xfc, 0x02, 0x3b),
},
{
.uuid = "0cb4ddff-a545-4401-9d06-688af53e7f84",
- .le = UUID_LE(0x0cb4ddff, 0xa545, 0x4401, 0x9d, 0x06, 0x68, 0x8a, 0xf5, 0x3e, 0x7f, 0x84),
- .be = UUID_BE(0x0cb4ddff, 0xa545, 0x4401, 0x9d, 0x06, 0x68, 0x8a, 0xf5, 0x3e, 0x7f, 0x84),
+ .le = GUID(0x0cb4ddff, 0xa545, 0x4401, 0x9d, 0x06, 0x68, 0x8a, 0xf5, 0x3e, 0x7f, 0x84),
+ .be = UUID(0x0cb4ddff, 0xa545, 0x4401, 0x9d, 0x06, 0x68, 0x8a, 0xf5, 0x3e, 0x7f, 0x84),
},
};
@@ -61,13 +61,13 @@ static void __init test_uuid_failed(const char *prefix, bool wrong, bool be,
static void __init test_uuid_test(const struct test_uuid_data *data)
{
- uuid_le le;
- uuid_be be;
+ guid_t le;
+ uuid_t be;
char buf[48];
/* LE */
total_tests++;
- if (uuid_le_to_bin(data->uuid, &le))
+ if (guid_to_bin(data->uuid, &le))
test_uuid_failed("conversion", false, false, data->uuid, NULL);
total_tests++;
@@ -78,7 +78,7 @@ static void __init test_uuid_test(const struct test_uuid_data *data)
/* BE */
total_tests++;
- if (uuid_be_to_bin(data->uuid, &be))
+ if (uuid_to_bin(data->uuid, &be))
test_uuid_failed("conversion", false, true, data->uuid, NULL);
total_tests++;
@@ -90,17 +90,17 @@ static void __init test_uuid_test(const struct test_uuid_data *data)
static void __init test_uuid_wrong(const char *data)
{
- uuid_le le;
- uuid_be be;
+ guid_t le;
+ uuid_t be;
/* LE */
total_tests++;
- if (!uuid_le_to_bin(data, &le))
+ if (!guid_to_bin(data, &le))
test_uuid_failed("negative", true, false, data, NULL);
/* BE */
total_tests++;
- if (!uuid_be_to_bin(data, &be))
+ if (!uuid_to_bin(data, &be))
test_uuid_failed("negative", true, true, data, NULL);
}
diff --git a/lib/uuid.c b/lib/uuid.c
index 37687af77ff8..f80dc63f6ca8 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -21,10 +21,10 @@
#include <linux/uuid.h>
#include <linux/random.h>
-const u8 uuid_le_index[16] = {3,2,1,0,5,4,7,6,8,9,10,11,12,13,14,15};
-EXPORT_SYMBOL(uuid_le_index);
-const u8 uuid_be_index[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
-EXPORT_SYMBOL(uuid_be_index);
+const u8 guid_index[16] = {3,2,1,0,5,4,7,6,8,9,10,11,12,13,14,15};
+EXPORT_SYMBOL(guid_index);
+const u8 uuid_index[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
+EXPORT_SYMBOL(uuid_index);
/***************************************************************
* Random UUID interface
@@ -53,21 +53,21 @@ static void __uuid_gen_common(__u8 b[16])
b[8] = (b[8] & 0x3F) | 0x80;
}
-void uuid_le_gen(uuid_le *lu)
+void guid_gen(guid_t *lu)
{
__uuid_gen_common(lu->b);
/* version 4 : random generation */
lu->b[7] = (lu->b[7] & 0x0F) | 0x40;
}
-EXPORT_SYMBOL_GPL(uuid_le_gen);
+EXPORT_SYMBOL_GPL(guid_gen);
-void uuid_be_gen(uuid_be *bu)
+void uuid_gen(uuid_t *bu)
{
__uuid_gen_common(bu->b);
/* version 4 : random generation */
bu->b[6] = (bu->b[6] & 0x0F) | 0x40;
}
-EXPORT_SYMBOL_GPL(uuid_be_gen);
+EXPORT_SYMBOL_GPL(uuid_gen);
/**
* uuid_is_valid - checks if UUID string valid
@@ -115,14 +115,14 @@ static int __uuid_to_bin(const char *uuid, __u8 b[16], const u8 ei[16])
return 0;
}
-int uuid_le_to_bin(const char *uuid, uuid_le *u)
+int guid_to_bin(const char *uuid, guid_t *u)
{
- return __uuid_to_bin(uuid, u->b, uuid_le_index);
+ return __uuid_to_bin(uuid, u->b, guid_index);
}
-EXPORT_SYMBOL(uuid_le_to_bin);
+EXPORT_SYMBOL(guid_to_bin);
-int uuid_be_to_bin(const char *uuid, uuid_be *u)
+int uuid_to_bin(const char *uuid, uuid_t *u)
{
- return __uuid_to_bin(uuid, u->b, uuid_be_index);
+ return __uuid_to_bin(uuid, u->b, uuid_index);
}
-EXPORT_SYMBOL(uuid_be_to_bin);
+EXPORT_SYMBOL(uuid_to_bin);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 2d41de3f98a1..9f37d6208e99 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1308,14 +1308,14 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
char uuid[UUID_STRING_LEN + 1];
char *p = uuid;
int i;
- const u8 *index = uuid_be_index;
+ const u8 *index = uuid_index;
bool uc = false;
switch (*(++fmt)) {
case 'L':
uc = true; /* fall-through */
case 'l':
- index = uuid_le_index;
+ index = guid_index;
break;
case 'B':
uc = true;
--
2.11.0
^ permalink raw reply related
* [PATCH 03/23] uuid: remove uuid_be defintions from the uapi header
From: Christoph Hellwig @ 2017-05-18 6:26 UTC (permalink / raw)
To: Andy Shevchenko, Amir Goldstein, linux-fsdevel
Cc: Shaohua Li, Dan Williams, David Howells, Steven Whitehouse,
Mimi Zohar, linux-xfs, linux-raid, linux-nvdimm, linux-kernel
In-Reply-To: <20170518062705.25902-1-hch@lst.de>
We don't use uuid_be and the UUID_BE constants in any uapi headers, so make
them private to the kernel.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/uuid.h | 15 +++++++++++++++
include/uapi/linux/uuid.h | 16 ----------------
2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 4dff73a89758..de3aea206562 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -18,6 +18,21 @@
#include <uapi/linux/uuid.h>
+typedef struct {
+ __u8 b[16];
+} uuid_be;
+
+#define UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
+((uuid_be) \
+{{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \
+ ((b) >> 8) & 0xff, (b) & 0xff, \
+ ((c) >> 8) & 0xff, (c) & 0xff, \
+ (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
+
+#define NULL_UUID_BE \
+ UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, \
+ 0x00, 0x00, 0x00, 0x00)
+
/*
* V1 (time-based) UUID definition [RFC 4122].
* - the timestamp is a 60-bit value, split 32/16/12, and goes in 100ns
diff --git a/include/uapi/linux/uuid.h b/include/uapi/linux/uuid.h
index 3738e5fb6a4d..0099756c4bac 100644
--- a/include/uapi/linux/uuid.h
+++ b/include/uapi/linux/uuid.h
@@ -24,10 +24,6 @@ typedef struct {
__u8 b[16];
} uuid_le;
-typedef struct {
- __u8 b[16];
-} uuid_be;
-
#define UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
((uuid_le) \
{{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \
@@ -35,20 +31,8 @@ typedef struct {
(c) & 0xff, ((c) >> 8) & 0xff, \
(d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
-#define UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
-((uuid_be) \
-{{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \
- ((b) >> 8) & 0xff, (b) & 0xff, \
- ((c) >> 8) & 0xff, (c) & 0xff, \
- (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
-
#define NULL_UUID_LE \
UUID_LE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, \
0x00, 0x00, 0x00, 0x00)
-#define NULL_UUID_BE \
- UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, \
- 0x00, 0x00, 0x00, 0x00)
-
-
#endif /* _UAPI_LINUX_UUID_H_ */
--
2.11.0
^ permalink raw reply related
* [PATCH 02/23] xfs: use uuid_be to implement the uuid_t type
From: Christoph Hellwig @ 2017-05-18 6:26 UTC (permalink / raw)
To: Andy Shevchenko, Amir Goldstein,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Mimi Zohar,
Steven Whitehouse
In-Reply-To: <20170518062705.25902-1-hch-jcswGhMUV9g@public.gmane.org>
Use the generic Linux definition to implement our UUID type, this will
allow using more generic infrastructure in the future.
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
fs/xfs/uuid.h | 4 ----
fs/xfs/xfs_linux.h | 3 +++
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/uuid.h b/fs/xfs/uuid.h
index 104db0f3bed6..4f1441ba4fa5 100644
--- a/fs/xfs/uuid.h
+++ b/fs/xfs/uuid.h
@@ -18,10 +18,6 @@
#ifndef __XFS_SUPPORT_UUID_H__
#define __XFS_SUPPORT_UUID_H__
-typedef struct {
- unsigned char __u_bits[16];
-} uuid_t;
-
extern int uuid_is_nil(uuid_t *uuid);
extern int uuid_equal(uuid_t *uuid1, uuid_t *uuid2);
extern void uuid_getnodeuniq(uuid_t *uuid, int fsid [2]);
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index 044fb0e15390..89ee5ec66837 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -19,6 +19,7 @@
#define __XFS_LINUX__
#include <linux/types.h>
+#include <linux/uuid.h>
/*
* Kernel specific type declarations for XFS
@@ -38,6 +39,8 @@ typedef __s64 xfs_daddr_t; /* <disk address> type */
typedef __u32 xfs_dev_t;
typedef __u32 xfs_nlink_t;
+typedef uuid_be uuid_t;
+
#include "xfs_types.h"
#include "kmem.h"
--
2.11.0
^ permalink raw reply related
* [PATCH 01/23] xfs: use uuid_copy() helper to abstract uuid_t
From: Christoph Hellwig @ 2017-05-18 6:26 UTC (permalink / raw)
To: Andy Shevchenko, Amir Goldstein,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Mimi Zohar,
Steven Whitehouse
In-Reply-To: <20170518062705.25902-1-hch-jcswGhMUV9g@public.gmane.org>
From: Amir Goldstein <amir73il-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
uuid_t definition is about to change.
Signed-off-by: Amir Goldstein <amir73il-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
fs/xfs/xfs_inode_item.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 08cb7d1a4a3a..013cc78d7daf 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -834,9 +834,7 @@ xfs_inode_item_format_convert(
in_f->ilf_dsize = in_f32->ilf_dsize;
in_f->ilf_ino = in_f32->ilf_ino;
/* copy biggest field of ilf_u */
- memcpy(in_f->ilf_u.ilfu_uuid.__u_bits,
- in_f32->ilf_u.ilfu_uuid.__u_bits,
- sizeof(uuid_t));
+ uuid_copy(&in_f->ilf_u.ilfu_uuid, &in_f32->ilf_u.ilfu_uuid);
in_f->ilf_blkno = in_f32->ilf_blkno;
in_f->ilf_len = in_f32->ilf_len;
in_f->ilf_boffset = in_f32->ilf_boffset;
@@ -851,9 +849,7 @@ xfs_inode_item_format_convert(
in_f->ilf_dsize = in_f64->ilf_dsize;
in_f->ilf_ino = in_f64->ilf_ino;
/* copy biggest field of ilf_u */
- memcpy(in_f->ilf_u.ilfu_uuid.__u_bits,
- in_f64->ilf_u.ilfu_uuid.__u_bits,
- sizeof(uuid_t));
+ uuid_copy(&in_f->ilf_u.ilfu_uuid, &in_f64->ilf_u.ilfu_uuid);
in_f->ilf_blkno = in_f64->ilf_blkno;
in_f->ilf_len = in_f64->ilf_len;
in_f->ilf_boffset = in_f64->ilf_boffset;
--
2.11.0
^ permalink raw reply related
* cleanup UUID types V3
From: Christoph Hellwig @ 2017-05-18 6:26 UTC (permalink / raw)
To: Andy Shevchenko, Amir Goldstein, linux-fsdevel
Cc: Shaohua Li, Dan Williams, David Howells, Steven Whitehouse,
Mimi Zohar, linux-xfs, linux-raid, linux-nvdimm, linux-kernel
Hi all,
this series, which is a combined effort from Amir, Andy and me introduces
new uuid_t and guid_t type names that are less confusing than the existing
types, adds new helpers for them and starts switching the fs code over to
it. Andy has additional patches on top to convert many of the users
that use char arrays for UUIDs and GUIDs to these (or rather a predecessor
for now until updated).
Changes since V3:
- stop exposing uuid_be/uuid_t to userspace
- remove uuid_be entirely
Changes since V2:
- various cleanups
^ permalink raw reply
* Re: [PATCH] md-cluster: fix potential lock issue in add_new_disk
From: Shaohua Li @ 2017-05-18 0:07 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: linux-raid, shli, neilb, Goldwyn Rodrigues
In-Reply-To: <1494914485-16288-1-git-send-email-gqjiang@suse.com>
On Tue, May 16, 2017 at 02:01:25PM +0800, Guoqing Jiang wrote:
> The add_new_disk returns with communication locked if
> __sendmsg returns failure, fix it with call unlock_comm
> before return.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> CC: Goldwyn Rodrigues <rgoldwyn@suse.com>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
queued, thanks!
> ---
> drivers/md/md-cluster.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
> index 7299ce2..03082e1 100644
> --- a/drivers/md/md-cluster.c
> +++ b/drivers/md/md-cluster.c
> @@ -1311,8 +1311,10 @@ static int add_new_disk(struct mddev *mddev, struct md_rdev *rdev)
> cmsg.raid_slot = cpu_to_le32(rdev->desc_nr);
> lock_comm(cinfo, 1);
> ret = __sendmsg(cinfo, &cmsg);
> - if (ret)
> + if (ret) {
> + unlock_comm(cinfo);
> return ret;
> + }
> cinfo->no_new_dev_lockres->flags |= DLM_LKF_NOQUEUE;
> ret = dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_EX);
> cinfo->no_new_dev_lockres->flags &= ~DLM_LKF_NOQUEUE;
> --
> 2.6.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: A sector-of-mismatch warning patch (was Re: Fault tolerance with badblocks)
From: Shaohua Li @ 2017-05-18 0:07 UTC (permalink / raw)
To: Nix
Cc: NeilBrown, Chris Murphy, David Brown, Anthony Youngman,
Phil Turmel, Ravi (Tom) Hale, Linux-RAID
In-Reply-To: <871sroscey.fsf@esperi.org.uk>
On Tue, May 16, 2017 at 10:46:13PM +0100, Nix wrote:
> On 16 May 2017, NeilBrown spake thusly:
>
> > Actually, I have another caveat. I don't think we want these messages
> > during initial resync, or any resync. Only during a 'check' or
> > 'repair'.
> > So add a check for MD_RECOVERY_REQUESTED or maybe for
> > sh->sectors >= conf->mddev->recovery_cp
>
> I completely agree, but it's already inside MD_RECOVERY_CHECK:
>
> if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery)) {
> /* don't try to repair!! */
> set_bit(STRIPE_INSYNC, &sh->state);
> pr_warn_ratelimited("%s: mismatch sector in range "
> "%llu-%llu\n", mdname(conf->mddev),
> (unsigned long long) sh->sector,
> (unsigned long long) sh->sector +
> STRIPE_SECTORS);
> } else {
>
> Doesn't that already mean that someone has explicitly triggered a check
> action?
Hi,
So the idea is: run 'check' and report mismatch, userspace (raid6check for
example) uses the reported info to fix the mismatch. The pr_warn_ratelimited
isn't a good way to communicate the info to userspace. I'm wondering why we
don't just run raid6check solely, it can do the job like what kernel does and
we avoid the crappy pr_warn_ratelimited.
Thanks,
Shaohua
^ permalink raw reply
* Re: How to delay mdadm assembly until all component drives are recognized/ready?
From: Ram Ramesh @ 2017-05-16 23:49 UTC (permalink / raw)
To: NeilBrown, Linux Raid
In-Reply-To: <877f1gmr49.fsf@notabene.neil.brown.name>
On 05/16/2017 04:25 PM, NeilBrown wrote:
> On Tue, May 09 2017, Ram Ramesh wrote:
>
>> Today, I noticed that my RAID6 md0 was assembled in degraded state with
>> two drives in failed state after a pm-suspend and restart. Both of these
>> drives were attached toSAS9211-8I controller. The other drives are
>> attached to motherboard. I have not had this on a normal boot/reboot.
>> Also, in this particular case, mythtv recording was going on when
>> suspended and therefore as soon as resumed that used this md0.
>>
>> Upon inspection, it appears (I am not sure here) that mdadm assembled
>> the array even before the drives were ready to be used. All I had to do
>> was to remove and re-add them to bring the array back to "good" state. I
>> am wondering if there is a way to tell mdadm to wait for all drives to
>> be ready before assembling. Also, if there is something that I can add
>> to resume scripts that will help, please let me know.
>>
>> Kernel: Linux zym 3.13.0-106-generic #153-Ubuntu SMP
>> mdadm - v3.2.5 - 18th May 2012
>>
>> Failed drives are HGST NAS and WD Gold with less than a year of usage.
>> So I doubt they are bad drives by any means.
> This is a question that needs to be addressed by your distro. mdadm
> just does what it is told to do by init/udev/systemd scripts.
>
> The preferred way for array startup to happen is that when udev
> discovers a new device, "mdadm --incremental $DEV" is run, and mdadm
> includes the device into an array as appropriate. mdadm will not
> normally activate the array until all expected devices have appeared.
> After some timeout "mdadm -IRs" or "mdadm --run /dev/mdXX" can be run to
> start the array even though it is degraded.
>
> The udev-* scripts and systemd/* unit files provided with current
> upstream mdadm do this, with a 30 second timeout.
> If a given distro doesn't use these scripts, you need to take it up
> with them.
>
> NeilBrown
Neil,
Thanks. I was hoping that there is something that I can add to
mdadm.conf that will make this work. That is why I checked here, as my
mdadm expertise is liminited. Anyway, it appears that the problem is due
ext4lazyinit which accesses md instantaneously after resume. I will take
this up with the distro folks. My machine badly needs an upgrade. I
think it is time
Ramesh
^ permalink raw reply
* Re: A sector-of-mismatch warning patch (was Re: Fault tolerance with badblocks)
From: Nix @ 2017-05-16 21:46 UTC (permalink / raw)
To: NeilBrown
Cc: Chris Murphy, David Brown, Anthony Youngman, Phil Turmel,
Ravi (Tom) Hale, Linux-RAID
In-Reply-To: <87bmqsmrre.fsf@notabene.neil.brown.name>
On 16 May 2017, NeilBrown spake thusly:
> Actually, I have another caveat. I don't think we want these messages
> during initial resync, or any resync. Only during a 'check' or
> 'repair'.
> So add a check for MD_RECOVERY_REQUESTED or maybe for
> sh->sectors >= conf->mddev->recovery_cp
I completely agree, but it's already inside MD_RECOVERY_CHECK:
if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery)) {
/* don't try to repair!! */
set_bit(STRIPE_INSYNC, &sh->state);
pr_warn_ratelimited("%s: mismatch sector in range "
"%llu-%llu\n", mdname(conf->mddev),
(unsigned long long) sh->sector,
(unsigned long long) sh->sector +
STRIPE_SECTORS);
} else {
Doesn't that already mean that someone has explicitly triggered a check
action?
^ permalink raw reply
* Re: How to delay mdadm assembly until all component drives are recognized/ready?
From: NeilBrown @ 2017-05-16 21:25 UTC (permalink / raw)
To: Ram Ramesh, Linux Raid
In-Reply-To: <243ef080-c32f-f423-3a98-af70194f994b@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1875 bytes --]
On Tue, May 09 2017, Ram Ramesh wrote:
> Today, I noticed that my RAID6 md0 was assembled in degraded state with
> two drives in failed state after a pm-suspend and restart. Both of these
> drives were attached toSAS9211-8I controller. The other drives are
> attached to motherboard. I have not had this on a normal boot/reboot.
> Also, in this particular case, mythtv recording was going on when
> suspended and therefore as soon as resumed that used this md0.
>
> Upon inspection, it appears (I am not sure here) that mdadm assembled
> the array even before the drives were ready to be used. All I had to do
> was to remove and re-add them to bring the array back to "good" state. I
> am wondering if there is a way to tell mdadm to wait for all drives to
> be ready before assembling. Also, if there is something that I can add
> to resume scripts that will help, please let me know.
>
> Kernel: Linux zym 3.13.0-106-generic #153-Ubuntu SMP
> mdadm - v3.2.5 - 18th May 2012
>
> Failed drives are HGST NAS and WD Gold with less than a year of usage.
> So I doubt they are bad drives by any means.
This is a question that needs to be addressed by your distro. mdadm
just does what it is told to do by init/udev/systemd scripts.
The preferred way for array startup to happen is that when udev
discovers a new device, "mdadm --incremental $DEV" is run, and mdadm
includes the device into an array as appropriate. mdadm will not
normally activate the array until all expected devices have appeared.
After some timeout "mdadm -IRs" or "mdadm --run /dev/mdXX" can be run to
start the array even though it is degraded.
The udev-* scripts and systemd/* unit files provided with current
upstream mdadm do this, with a 30 second timeout.
If a given distro doesn't use these scripts, you need to take it up
with them.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: A sector-of-mismatch warning patch (was Re: Fault tolerance with badblocks)
From: NeilBrown @ 2017-05-16 21:11 UTC (permalink / raw)
To: Nix, Chris Murphy
Cc: David Brown, Anthony Youngman, Phil Turmel, Ravi (Tom) Hale,
Linux-RAID
In-Reply-To: <87efvpmqf6.fsf@notabene.neil.brown.name>
[-- Attachment #1: Type: text/plain, Size: 4656 bytes --]
On Tue, May 16 2017, NeilBrown wrote:
> On Tue, May 09 2017, Nix wrote:
>
>> On 9 May 2017, Chris Murphy verbalised:
>>
>>> 1. md reports all data drives and the LBAs for the affected stripe
>>
>> Enough rambling from me. Here's a hilariously untested patch against
>> 4.11 (as in I haven't even booted with it: my systems are kind of in
>> flux right now as I migrate to the md-based server that got me all
>> concerned about this). It compiles! And it's definitely safer than
>> trying a repair, and makes it possible to recover from a real mismatch
>> without losing all your hair in the process, or determine that a
>> mismatch is spurious or irrelevant. And that's enough for me, frankly.
>> This is a very rare problem, one hopes.
>>
>> (It's probably not ideal, because the error is just known to be
>> somewhere in that stripe, not on that sector, which makes determining
>> the affected data somewhat harder. But at least you can figure out what
>> filesystem it's on. :) )
>>
>> 8<------------------------------------------------------------->8
>> From: Nick Alcock <nick.alcock@oracle.com>
>> Subject: [PATCH] md: report sector of stripes with check mismatches
>>
>> This makes it possible, with appropriate filesystem support, for a
>> sysadmin to tell what is affected by the mismatch, and whether
>> it should be ignored (if it's inside a swap partition, for
>> instance).
>>
>> We ratelimit to prevent log flooding: if there are so many
>> mismatches that ratelimiting is necessary, the individual messages
>> are relatively unlikely to be important (either the machine is
>> swapping like crazy or something is very wrong with the disk).
>>
>> Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
>> ---
>> drivers/md/raid5.c | 16 ++++++++++++----
>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index ed5cd705b985..bcd2e5150e29 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -3959,10 +3959,14 @@ static void handle_parity_checks5(struct r5conf *conf, struct stripe_head *sh,
>> set_bit(STRIPE_INSYNC, &sh->state);
>> else {
>> atomic64_add(STRIPE_SECTORS, &conf->mddev->resync_mismatches);
>> - if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery))
>> + if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery)) {
>> /* don't try to repair!! */
>> set_bit(STRIPE_INSYNC, &sh->state);
>> - else {
>> + pr_warn_ratelimited("%s: mismatch around sector "
>> + "%llu\n", __func__,
>> + (unsigned long long)
>> + sh->sector);
>> + } else {
>
> I think there is no point giving the function name,
> but that you should give the name of the array.
> Also "around" is a little vague.
> Maybe something like:
>
>> + pr_warn_ratelimited("%s: mismatch sector in range "
>> + "%llu-%llu\n", mdname(conf->mddev),
>> + (unsigned long long) sh->sector,
>> + (unsigned long long) sh->sector + STRIPE_SECTORS);
>
> As an optional enhancement, you could add "will recalculate P/Q" or
> "left unchanged" as appropriate.
>
> Providing at least that the array name is included in the message, I
> support this patch.
Actually, I have another caveat. I don't think we want these messages
during initial resync, or any resync. Only during a 'check' or
'repair'.
So add a check for MD_RECOVERY_REQUESTED or maybe for
sh->sectors >= conf->mddev->recovery_cp
NeilBrown
>
> NeilBrown
>
>
>
>> sh->check_state = check_state_compute_run;
>> set_bit(STRIPE_COMPUTE_RUN, &sh->state);
>> set_bit(STRIPE_OP_COMPUTE_BLK, &s->ops_request);
>> @@ -4111,10 +4115,14 @@ static void handle_parity_checks6(struct r5conf *conf, struct stripe_head *sh,
>> }
>> } else {
>> atomic64_add(STRIPE_SECTORS, &conf->mddev->resync_mismatches);
>> - if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery))
>> + if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery)) {
>> /* don't try to repair!! */
>> set_bit(STRIPE_INSYNC, &sh->state);
>> - else {
>> + pr_warn_ratelimited("%s: mismatch around sector "
>> + "%llu\n", __func__,
>> + (unsigned long long)
>> + sh->sector);
>> + } else {
>> int *target = &sh->ops.target;
>>
>> sh->ops.target = -1;
>> --
>> 2.12.2.212.gea238cf35.dirty
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: RFC - Raid error detection and auto-recovery (was Fault tolerance with badblocks)
From: Anthonys Lists @ 2017-05-16 16:11 UTC (permalink / raw)
To: Nix, Phil Turmel; +Cc: NeilBrown, linux-raid
In-Reply-To: <87efvosstq.fsf@esperi.org.uk>
On 16/05/2017 16:51, Nix wrote:
> On 16 May 2017, Phil Turmel spake thusly:
>
>> >On 05/16/2017 10:53 AM, Wols Lists wrote:
>>> >>First thing we do is read the entire stripe.
>> >
>> >A substantial performance degradation, right out of the gate...
> I'm fairly sure Wol's intention is to do this as part of a check/repair
> operation. You don't want to run like this in normal usage! (Though I
> agree that it seems unclear whether you want to do this at all.)
It's not meant to be on by default. Imho, if the sysadmin switches it
on, it's their lookout :-) Integrity over speed - some sysadmins might
make that choice.
Cheers,
Wol
^ permalink raw reply
* Re: [PATCH 17/17] fs: switch ->s_uuid to uuid_t
From: Christoph Hellwig @ 2017-05-16 15:53 UTC (permalink / raw)
To: Amir Goldstein
Cc: Christoph Hellwig, Andy Shevchenko, linux-fsdevel, Shaohua Li,
Dan Williams, David Howells, Steven Whitehouse, Mimi Zohar,
linux-xfs, linux-raid, linux-nvdimm, linux-kernel
In-Reply-To: <CAOQ4uxgyetFTbtuE3H2Msv9Zg1LEdoxr0YqZJ6yv0cFgiTAmAw@mail.gmail.com>
On Mon, May 15, 2017 at 10:38:18PM +0300, Amir Goldstein wrote:
> On Mon, May 15, 2017 at 6:43 PM, Christoph Hellwig <hch@lst.de> wrote:
> > For some file systems we still memcpy into it, but in various places this
> > already allows us to use the proper uuid helpers. More to come..
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
>
> > --- a/fs/xfs/xfs_mount.c
> > +++ b/fs/xfs/xfs_mount.c
> > @@ -75,7 +75,7 @@ xfs_uuid_mount(
> >
> > /* Publish UUID in struct super_block */
> > BUILD_BUG_ON(sizeof(mp->m_super->s_uuid) != sizeof(uuid_t));
>
> BUILD_BUG_ON not needed anymore now that s_uuid is uuid_t
Yeah, you mentioned that last time and I forgot about it. Fixed up
in the git tree and for the next repost.
^ permalink raw reply
* Re: RFC - Raid error detection and auto-recovery (was Fault tolerance with badblocks)
From: Nix @ 2017-05-16 15:51 UTC (permalink / raw)
To: Phil Turmel; +Cc: Wols Lists, NeilBrown, linux-raid
In-Reply-To: <1bccccd1-1ff8-1cb3-492e-42468a3c8a8f@turmel.org>
On 16 May 2017, Phil Turmel spake thusly:
> On 05/16/2017 10:53 AM, Wols Lists wrote:
>> First thing we do is read the entire stripe.
>
> A substantial performance degradation, right out of the gate...
I'm fairly sure Wol's intention is to do this as part of a check/repair
operation. You don't want to run like this in normal usage! (Though I
agree that it seems unclear whether you want to do this at all.)
However, the existence of raid6check, which is new enough that I'd not
noticed it (not being installed by default doesn't help there, as does
the lack of mention of autorepair mode in the manpage), makes this
entire conversation/argument moot, as far as I can see. If you're really
concerned, stick raid6check in early userspace, like mdadm, so you can
recover from *anything*. :)
The requirement to run raid6check by hand is no more annoying than the
requirement to run scrubs by hand: userspace is clearly a better place
for this sort of rare obscurity than the kernel, though it's a minor
shame raid6check can't tell md that the block device has changed so you
wouldn't need to impair availability by taking the array down after
fixing things. (Still, this is likely to be needed so rarely that it's
completely unimportant.)
--
NULL && (void)
^ permalink raw reply
* Re: RFC - Raid error detection and auto-recovery (was Fault tolerance with badblocks)
From: Phil Turmel @ 2017-05-16 15:31 UTC (permalink / raw)
To: Wols Lists, Nix, NeilBrown; +Cc: linux-raid
In-Reply-To: <591B125A.1000307@youngman.org.uk>
On 05/16/2017 10:53 AM, Wols Lists wrote:
> I'll give a car example. I'm talking about a car in a ditch. You're
> talking about a motorway pile-up AND YOU'RE ASSUMING I CAN'T TELL THE
> DIFFERENCE. That's why I'm getting so frustrated!
You clearly cannot.
> Please LOOK AT THE MATHS of my scenario.
It's not a math problem. I'm quite familiar with the math, as a matter
of fact. Galois fields are exceedingly cool for a math geek like me.
> First thing we do is read the entire stripe.
A substantial performance degradation, right out of the gate...
> IF the integrity check passes, we return the data. If it fails and our
> raid can't reconstruct (two-disk mirror, raid-4, raid-5) we return an error.
Where we currently return the data and let the upper layer decide its
value. An error here is a regression in my book.
> Second - we now have a stripe that fails integrity, so we pass it
> through Peter's equation. If it returns "one block is corrupt and here's
> the correct version" we return the correct version. If it returns "can't
> solve the equation - too many unknowns" we return a read error.
Changing the data returned from what was written is another regression
in my book. Since the drive not returning a read error is far more
significant indication that the data is correct than a mismatch saying
its wrong.
> We *have* to assume that if the stripe passes the integrity check that
> it's correct - but we could have had an error that fools the integrity
> check! We just assume it's highly unlikely.
If the data blocks are successfully read from there drives, we *have* to
assume they're correct. There are so many zeroes between the decimal
point and the first significant digit of that error probability that a
physical explanation elsewhere is a virtual certainty.
> What is the probability that Peter's equation screws up? We *KNOW* that
> if only one block is corrupt, that it will ALWAYS SUCCESSFULLY correct
> it. And from reading the paper, it seems to me that if *more than one*
> block is corrupt, it will detect it with over 99.9% accuracy.
No. We don't. We have a highly reliable drive saying the data is
correct versus a *system* of reads and writes spread over multiple
physical systems and spread over time that has a constellation of
failure modes, any one of which could have created the situation at hand.
Software flaws galore, particularly incomplete stripe writes. Power
problems truncating stripe writes. System memory bit flips. PCIe
uncaught transmission errors. Controller buffer memory bit flips. SATA
or SAS transmission errors.
All of the above are rare. But not anywhere near as rare as an
undetected sector read error. MD cannot safely fix this automatically,
and shouldn't. And with the performance hit, it is actively stupid.
And I'm done arguing.
Phil
^ permalink raw reply
* Re: RFC - Raid error detection and auto-recovery (was Fault tolerance with badblocks)
From: Wols Lists @ 2017-05-16 14:53 UTC (permalink / raw)
To: Phil Turmel, Nix, NeilBrown; +Cc: linux-raid
In-Reply-To: <d1580b0f-c38d-17ce-3488-33135774ed92@turmel.org>
On 16/05/17 15:17, Phil Turmel wrote:
> On 05/16/2017 06:33 AM, Wols Lists wrote:
>> On 15/05/17 23:31, Phil Turmel wrote:
>
>>> If and only if it is known that all but the supposedly corrupt block
>>> were written together (complete stripe) and no possibility of
>>> perturbation occurred between the original calculation of P,Q in the CPU
>>> and original transmission of all of these blocks to the member drives.
>>
>> NO! This is a "can't see the wood for the trees" situation.
>
> You can shout NO all you want, and make inapplicable metaphors, but you
> are still wrong.
>
>> If one block
>> in a raid-6 is corrupt, we can correct it. That's maths, that's what the
>> maths says, and it is not only possible, but *definite*.
>
> The math has preconditions. If the preconditions are unmet, or unknown,
> you cannot use the math.
>
>> WHAT caused the corruption, and HOW, is irrelevant. The only requirement
>> is that *just one block is lost*. If that's the case we can recover.
>
> WHAT and HOW are the preconditions to the math. The algorithm you seek
> exists as a userspace utility that an administrator can use after
> suitable analysis of the situation. Feel free to script a call to that
> utility on *your* system whenever your check scrub signals a mismatch.
Which is where you can't see the wood from the trees. WHAT and HOW are
*physical* things, therefore they CAN'T have anything to do with pure maths.
The precondition is that we are dealing with only one bad block. That
*IS* the mathematical equivalent of what you are saying. We have two
unknowns - which block is corrupt, and what its original value was. You
can handwave all you like, but at the moment all you're saying is that
Peter doesn't know his maths.
PLEASE *either* treat it as a *maths* problem - in which case you can't
appeal to hardware, *or* treat it as a *physical* problem, in which case
we are arguing at cross purposes.
>
>> At the end of the day, as I see it, MD raid *can* do data integrity. So
>> if the user thinks the performance hit is worth it, why not?
>
> You are seeing a mirage due to a naive application of the math.
No. *Maths* and *reality* are NOT the same thing.
>
>> MD raid *can* do data recovery. So why not?
>
> It *cannot* do it for reasons many of us have tried to explain. Sorry.
>
>> And yes, given the opportunity I will write it myself. I just have to be
>> honest and say my family situation interferes with that desire fairly
>> drastically (which is why I've put a lot of effort in elsewhere, that
>> doesn't require long stretches of concentration).
>
> As I said to Nix, no system administrator who cares about their data
> will touch a kernel that includes such a patch.
>
I'll give a car example. I'm talking about a car in a ditch. You're
talking about a motorway pile-up AND YOU'RE ASSUMING I CAN'T TELL THE
DIFFERENCE. That's why I'm getting so frustrated!
Please LOOK AT THE MATHS of my scenario.
First thing we do is read the entire stripe.
IF the integrity check passes, we return the data. If it fails and our
raid can't reconstruct (two-disk mirror, raid-4, raid-5) we return an error.
Second - we now have a stripe that fails integrity, so we pass it
through Peter's equation. If it returns "one block is corrupt and here's
the correct version" we return the correct version. If it returns "can't
solve the equation - too many unknowns" we return a read error.
We *have* to assume that if the stripe passes the integrity check that
it's correct - but we could have had an error that fools the integrity
check! We just assume it's highly unlikely.
What is the probability that Peter's equation screws up? We *KNOW* that
if only one block is corrupt, that it will ALWAYS SUCCESSFULLY correct
it. And from reading the paper, it seems to me that if *more than one*
block is corrupt, it will detect it with over 99.9% accuracy.
So the *ONLY* way my algorithm can screw up, is if Peter's algorithm
wrongly thinks a multiple-block is a single-block corruption, which by
my simple maths has a probability of about 0.025% !!!
Please can you present me with a PLAUSIBLE scenario where Peter's
algorithm will screw up. And mere handwaving won't do it, because I CAN,
and ALMOST CERTAINLY WILL, detect the motorway pile-up scenario you're
going on about, and I will treat it exactly the way you do - punt it up
to manual intervention.
Cheers,
Wol
^ permalink raw reply
* Re: RFC - Raid error detection and auto-recovery (was Fault tolerance with badblocks)
From: Phil Turmel @ 2017-05-16 14:17 UTC (permalink / raw)
To: Wols Lists, Nix, NeilBrown; +Cc: linux-raid
In-Reply-To: <591AD58E.6090408@youngman.org.uk>
On 05/16/2017 06:33 AM, Wols Lists wrote:
> On 15/05/17 23:31, Phil Turmel wrote:
>> If and only if it is known that all but the supposedly corrupt block
>> were written together (complete stripe) and no possibility of
>> perturbation occurred between the original calculation of P,Q in the CPU
>> and original transmission of all of these blocks to the member drives.
>
> NO! This is a "can't see the wood for the trees" situation.
You can shout NO all you want, and make inapplicable metaphors, but you
are still wrong.
> If one block
> in a raid-6 is corrupt, we can correct it. That's maths, that's what the
> maths says, and it is not only possible, but *definite*.
The math has preconditions. If the preconditions are unmet, or unknown,
you cannot use the math.
> WHAT caused the corruption, and HOW, is irrelevant. The only requirement
> is that *just one block is lost*. If that's the case we can recover.
WHAT and HOW are the preconditions to the math. The algorithm you seek
exists as a userspace utility that an administrator can use after
suitable analysis of the situation. Feel free to script a call to that
utility on *your* system whenever your check scrub signals a mismatch.
> At the end of the day, as I see it, MD raid *can* do data integrity. So
> if the user thinks the performance hit is worth it, why not?
You are seeing a mirage due to a naive application of the math.
> MD raid *can* do data recovery. So why not?
It *cannot* do it for reasons many of us have tried to explain. Sorry.
> And yes, given the opportunity I will write it myself. I just have to be
> honest and say my family situation interferes with that desire fairly
> drastically (which is why I've put a lot of effort in elsewhere, that
> doesn't require long stretches of concentration).
As I said to Nix, no system administrator who cares about their data
will touch a kernel that includes such a patch.
Phil
^ permalink raw reply
* Re: RFC - Raid error detection and auto-recovery (was Fault tolerance with badblocks)
From: Wols Lists @ 2017-05-16 10:33 UTC (permalink / raw)
To: Phil Turmel, Nix, NeilBrown; +Cc: linux-raid
In-Reply-To: <7ba308d7-6954-8cd9-e623-93b940c5e370@turmel.org>
On 15/05/17 23:31, Phil Turmel wrote:
> On 05/15/2017 09:44 AM, Wols Lists wrote:
>> On 15/05/17 12:11, Nix wrote:
>>> I think the point here is that we'd like some way to recover that lets
>>> us get back to the most-likely-consistent state. However, on going over
>>> the RAID-6 maths again I think I see where I was wrong. In the absence
>>> of P, Q, P *or* Q or one of P and Q and a data stripe, you can
>>> reconstruct the rest, but the only reason you can do that is because
>>> they are either correct or absent: you can trust them if they're there,
>>> and you cannot mistake a missing stripe for one that isn't missing.
>>
>> The point of Peter Anvin's paper, though, was that it IS possible to
>> correct raid-6 if ONE of P, Q, or a data stripe is corrupt.
>
> If and only if it is known that all but the supposedly corrupt block
> were written together (complete stripe) and no possibility of
> perturbation occurred between the original calculation of P,Q in the CPU
> and original transmission of all of these blocks to the member drives.
NO! This is a "can't see the wood for the trees" situation. If one block
in a raid-6 is corrupt, we can correct it. That's maths, that's what the
maths says, and it is not only possible, but *definite*.
WHAT caused the corruption, and HOW, is irrelevant. The only requirement
is that *just one block is lost*. If that's the case we can recover.
>
> Since incomplete writes and a whole host of hardware corruptions are
> known to happen, you *don't* have enough information to automatically
> repair.
And I would guess that in most of the cases you are talking about, it's
not just one block that is lost. In that case we don't have enough
information to repair, full stop! And if I feed it into Peter's equation
the result would be nonsense so I wouldn't bother trying. (As in, I
would feed it into Peter's equation, but I'd stop there.)
>
> The only unambiguous signal MD raid receives that a particular block is
> corrupt is an Unrecoverable Read Error from a drive. MD fixes these
> from available redundancy. All other sources of corruption require
> assistance from an upper layer or from administrator input.
>
> There's no magic wand, Wol.
>
I know there isn't a magic wand. BUT. What is the chance of a
multi-block corruption looking like a single-block error? Pretty low I
think, and according to Peter Anvin's paper it gives off some pretty
clear signals that "something's not right".
At the end of the day, as I see it, MD raid *can* do data integrity. So
if the user thinks the performance hit is worth it, why not?
MD raid *can* do data recovery. So why not?
And yes, given the opportunity I will write it myself. I just have to be
honest and say my family situation interferes with that desire fairly
drastically (which is why I've put a lot of effort in elsewhere, that
doesn't require long stretches of concentration).
All your scenarios you are throwing at me, can you come up with ANY that
will BOTH corrupt more than one block AND make it look like a single
block error? As I look at it, I will only bother correcting errors that
look correctable. Which means, in probably 99.9% of cases, I get it
right. (And if I don't bother, the data's lost, anyway!)
Looked at from the other side, IFF we have a correctable error, and fix
it by recalculating P & Q, that gives us AT BEST a 50% chance of getting
it right, and it gets worse the more disks we have. Especially if our
problem is that something has accidentally stomped on just one disk. Or
that we've got several dodgy disks that we've had to ddrescue...
Neil mentioned elsewhere that he's not sure about btrfs and zfs. Can
they actually do data recovery, or just data integrity? And I'm on the
opensuse mailing list. I would NOT say btrfs is ready for the
casual/naive user. I suspect most of the smoke on the mailing list is
people who've been burnt in the past, but there still seems to be a
trickle of people reporting "an update ate my root partition". For which
the usual advice seems to be "reformat and reinstall" :-(
Cheers,
Wol
^ permalink raw reply
* Re: A sector-of-mismatch warning patch (was Re: Fault tolerance with badblocks)
From: Nix @ 2017-05-16 9:13 UTC (permalink / raw)
To: NeilBrown
Cc: Chris Murphy, David Brown, Anthony Youngman, Phil Turmel,
Ravi (Tom) Hale, Linux-RAID
In-Reply-To: <87efvpmqf6.fsf@notabene.neil.brown.name>
On 16 May 2017, NeilBrown said:
>> - else {
>> + pr_warn_ratelimited("%s: mismatch around sector "
>> + "%llu\n", __func__,
>> + (unsigned long long)
>> + sh->sector);
>> + } else {
>
> I think there is no point giving the function name,
> but that you should give the name of the array.
*ouch* I can't believe I forgot that. I have more than one array
myself... "we have a fault but we don't know what array it's on" is not
much of an improvement over the status quo, really! (though you could
make a good guess by looking for preceding sync-start messages, you can
of course sync two arrays at the same time...)
> Also "around" is a little vague.
Intentionally: I couldn't think of the right terminology. Yours is
better.
> Maybe something like:
>
>> + pr_warn_ratelimited("%s: mismatch sector in range "
>> + "%llu-%llu\n", mdname(conf->mddev),
>> + (unsigned long long) sh->sector,
>> + (unsigned long long) sh->sector + STRIPE_SECTORS);
Nice! Here's a rerolled patch. (We exceed the 80-char limit but that's
pr_warn_ratelimited()'s fault for having such a long name!)
Tested by making a raid array on a bunch of sparse files then dding a
byte of garbage into one of them and checking it. I got a nice error
message, name and all, and the sector count looked good.
From f05a451d46900849c7965a0e7dde085f1fb50dfc Mon Sep 17 00:00:00 2001
From: Nick Alcock <nick.alcock@oracle.com>
Date: Tue, 9 May 2017 21:55:17 +0100
Subject: [PATCH] md: report sector of stripes with check mismatches
This makes it possible, with appropriate filesystem support, for a
sysadmin to tell what is affected by the mismatch, and whether
it should be ignored (if it's inside a swap partition, for
instance).
We ratelimit to prevent log flooding: if there are so many
mismatches that ratelimiting is necessary, the individual messages
are relatively unlikely to be important (either the machine is
swapping like crazy or something is very wrong with the disk).
Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
---
drivers/md/raid5.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index ed5cd705b985..937314051be5 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3959,10 +3959,15 @@ static void handle_parity_checks5(struct r5conf *conf, struct stripe_head *sh,
set_bit(STRIPE_INSYNC, &sh->state);
else {
atomic64_add(STRIPE_SECTORS, &conf->mddev->resync_mismatches);
- if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery))
+ if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery)) {
/* don't try to repair!! */
set_bit(STRIPE_INSYNC, &sh->state);
- else {
+ pr_warn_ratelimited("%s: mismatch sector in range "
+ "%llu-%llu\n", mdname(conf->mddev),
+ (unsigned long long) sh->sector,
+ (unsigned long long) sh->sector +
+ STRIPE_SECTORS);
+ } else {
sh->check_state = check_state_compute_run;
set_bit(STRIPE_COMPUTE_RUN, &sh->state);
set_bit(STRIPE_OP_COMPUTE_BLK, &s->ops_request);
@@ -4111,10 +4116,15 @@ static void handle_parity_checks6(struct r5conf *conf, struct stripe_head *sh,
}
} else {
atomic64_add(STRIPE_SECTORS, &conf->mddev->resync_mismatches);
- if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery))
+ if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery)) {
/* don't try to repair!! */
set_bit(STRIPE_INSYNC, &sh->state);
- else {
+ pr_warn_ratelimited("%s: mismatch sector in range "
+ "%llu-%llu\n", mdname(conf->mddev),
+ (unsigned long long) sh->sector,
+ (unsigned long long) sh->sector +
+ STRIPE_SECTORS);
+ } else {
int *target = &sh->ops.target;
sh->ops.target = -1;
--
2.12.2.212.gea238cf35.dirty
^ 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