| Message ID | 20180315150814.9412-16-bart.vanassche@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15/03/2018 11:08 PM, Bart Van Assche wrote: > Ensure that byte swapping occurs on big endian architectures when > reading or writing the superblock. > > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> Hi Bart, The code still does not work with your patch on big endian machine (e.g. s390x). When I fix the bcache s390x bug, I did something like this patch in my first try. It turns out more things need to be fixed, 1, some cache_sb members only swapped to cpu endianness when read in, not swapped to little endian when write out. For example data_offset, nbuckets, nr_in_set, nr_this_dev ... 2, checksum is calculated in cpu endiannness, not explicitly in little endian. Also user space tools need to update to support the endianness issue. How about leave the endianness problem to me? I will pick code piece from your patch and set 'From: Bart Van Assche <bart.vanassche@wdc.com>' on the patch(s), and continue my fix for bcache on s390x. Thanks. Coly Li > --- > drivers/md/bcache/bcache.h | 12 ++++++++++++ > drivers/md/bcache/super.c | 4 ++-- > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h > index 72b1ea4576d9..50ddc78596bf 100644 > --- a/drivers/md/bcache/bcache.h > +++ b/drivers/md/bcache/bcache.h > @@ -784,6 +784,18 @@ static inline bool ptr_available(struct cache_set *c, const struct bkey *k, > bch_crc64(p, q - p); \ > }) > > +/* > + * Variant of csum_set() for data structures in which (i)->keys has type > + * __le16. > + */ > +#define csum_set_le(i) ({ \ > + const void *p = (void *)(i) + sizeof(uint64_t); \ > + const void *q = bkey_idx((struct bkey *)(i)->d, \ > + le16_to_cpu((i)->keys)); \ > + \ > + bch_crc64(p, q - p); \ > +}) > + > /* Error handling macros */ > > #define btree_bug(b, ...) \ > diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c > index 39bec137f636..31d700aecd56 100644 > --- a/drivers/md/bcache/super.c > +++ b/drivers/md/bcache/super.c > @@ -110,7 +110,7 @@ static const char *read_super(struct cache_sb *sb, struct block_device *bdev, > goto err; > > err = "Bad checksum"; > - if (s->csum != csum_set(s)) > + if (s->csum != csum_set_le(s)) > goto err; > > err = "Bad UUID"; > @@ -236,7 +236,7 @@ static void __write_super(struct cache_sb *sb, struct bio *bio) > for (i = 0; i < sb->keys; i++) > out->d[i] = cpu_to_le64(sb->d[i]); > > - out->csum = csum_set(out); > + out->csum = csum_set_le(out); > > pr_debug("ver %llu, flags %llu, seq %llu", > sb->version, sb->flags, sb->seq); >
On Fri, 2018-03-16 at 12:47 +0800, Coly Li wrote: > The code still does not work with your patch on big endian machine (e.g. > s390x). When I fix the bcache s390x bug, I did something like this patch > in my first try. It turns out more things need to be fixed, > 1, some cache_sb members only swapped to cpu endianness when read in, > not swapped to little endian when write out. For example data_offset, > nbuckets, nr_in_set, nr_this_dev ... > 2, checksum is calculated in cpu endiannness, not explicitly in little > endian. > > Also user space tools need to update to support the endianness issue. > > How about leave the endianness problem to me? I will pick code piece > from your patch and set 'From: Bart Van Assche <bart.vanassche@wdc.com>' > on the patch(s), and continue my fix for bcache on s390x. Hello Coly, That sounds fine to me. Please let me know if you would like me to repost this series and/or address the feedback that I received for this patch series. Thanks, Bart.
On 16/03/2018 11:00 PM, Bart Van Assche wrote: > On Fri, 2018-03-16 at 12:47 +0800, Coly Li wrote: >> The code still does not work with your patch on big endian machine (e.g. >> s390x). When I fix the bcache s390x bug, I did something like this patch >> in my first try. It turns out more things need to be fixed, >> 1, some cache_sb members only swapped to cpu endianness when read in, >> not swapped to little endian when write out. For example data_offset, >> nbuckets, nr_in_set, nr_this_dev ... >> 2, checksum is calculated in cpu endiannness, not explicitly in little >> endian. >> >> Also user space tools need to update to support the endianness issue. >> >> How about leave the endianness problem to me? I will pick code piece >> from your patch and set 'From: Bart Van Assche <bart.vanassche@wdc.com>' >> on the patch(s), and continue my fix for bcache on s390x. > > Hello Coly, > > That sounds fine to me. Please let me know if you would like me to repost > this series and/or address the feedback that I received for this patch series. Hi Bart, I see Mike already picks some of your patches, repost an updated version for the rested patch should be a good idea. Thanks. Coly Li
diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h index 72b1ea4576d9..50ddc78596bf 100644 --- a/drivers/md/bcache/bcache.h +++ b/drivers/md/bcache/bcache.h @@ -784,6 +784,18 @@ static inline bool ptr_available(struct cache_set *c, const struct bkey *k, bch_crc64(p, q - p); \ }) +/* + * Variant of csum_set() for data structures in which (i)->keys has type + * __le16. + */ +#define csum_set_le(i) ({ \ + const void *p = (void *)(i) + sizeof(uint64_t); \ + const void *q = bkey_idx((struct bkey *)(i)->d, \ + le16_to_cpu((i)->keys)); \ + \ + bch_crc64(p, q - p); \ +}) + /* Error handling macros */ #define btree_bug(b, ...) \ diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c index 39bec137f636..31d700aecd56 100644 --- a/drivers/md/bcache/super.c +++ b/drivers/md/bcache/super.c @@ -110,7 +110,7 @@ static const char *read_super(struct cache_sb *sb, struct block_device *bdev, goto err; err = "Bad checksum"; - if (s->csum != csum_set(s)) + if (s->csum != csum_set_le(s)) goto err; err = "Bad UUID"; @@ -236,7 +236,7 @@ static void __write_super(struct cache_sb *sb, struct bio *bio) for (i = 0; i < sb->keys; i++) out->d[i] = cpu_to_le64(sb->d[i]); - out->csum = csum_set(out); + out->csum = csum_set_le(out); pr_debug("ver %llu, flags %llu, seq %llu", sb->version, sb->flags, sb->seq);
Ensure that byte swapping occurs on big endian architectures when reading or writing the superblock. Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> --- drivers/md/bcache/bcache.h | 12 ++++++++++++ drivers/md/bcache/super.c | 4 ++-- 2 files changed, 14 insertions(+), 2 deletions(-)