Skip to contentSkip to conversation

bcache: fix wrong BITMASK offset value for BDEV_CACHE_MODE

PATCH In review Lin Feng 2 participants 3 messages Jul 20, 2021 – May 16, 2022 bcache (block layer cache)

lore
Share & tools
lore

Lin Feng

author
#0 lore raw

PATCH 1/1 bcache: fix wrong BITMASK offset value for BDEV_CACHE_MODE

Original codes:
BITMASK(CACHE_SYNC, struct cache_sb, flags, 0, 1);
BITMASK(CACHE_DISCARD, struct cache_sb, flags, 1, 1);
BITMASK(CACHE_REPLACEMENT, struct cache_sb, flags, 2, 3);
...
BITMASK(BDEV_CACHE_MODE, struct cache_sb, flags, 0, 4);

Should BDEV_CACHE_MODE bits start from bit-nr 5(2+3) else it overlaps with previous defined bit chunks, since we have 4 types of cache modes, BDEV_CACHE_MODE will overwrite CACHE_SYNC and CACHE_DISCARD bits.

This bug stays there since first upstream version of bcache, don't know why it lives so long, or am i missing something, please point me out if I'm wrong, thanks!

---
 include/uapi/linux/bcache.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/uapi/linux/bcache.h +1 -1
index cf7399f03b71..dccd89756451 100644
@@ -288,7 +288,7 @@ BITMASK(CACHE_REPLACEMENT,		struct cache_sb, flags, 2, 3);
 #define CACHE_REPLACEMENT_FIFO		1U
 #define CACHE_REPLACEMENT_RANDOM	2U
 
-BITMASK(BDEV_CACHE_MODE,		struct cache_sb, flags, 0, 4);
+BITMASK(BDEV_CACHE_MODE,		struct cache_sb, flags, 5, 4);
 #define CACHE_MODE_WRITETHROUGH		0U
 #define CACHE_MODE_WRITEBACK		1U
 #define CACHE_MODE_WRITEAROUND		2U
Signature · 1 line
2.31.1
#1 lore raw
Quoted 10 lines · Lin Feng

Original codes:
BITMASK(CACHE_SYNC, struct cache_sb, flags, 0, 1);
BITMASK(CACHE_DISCARD, struct cache_sb, flags, 1, 1);
BITMASK(CACHE_REPLACEMENT, struct cache_sb, flags, 2, 3);
...
BITMASK(BDEV_CACHE_MODE, struct cache_sb, flags, 0, 4);

Should BDEV_CACHE_MODE bits start from bit-nr 5(2+3) else it overlaps with previous defined bit chunks, since we have 4 types of cache modes, BDEV_CACHE_MODE will overwrite CACHE_SYNC and CACHE_DISCARD bits.

The overlap won't happen, previous lines are for cache device, and what you modified is for backing device.

And your patch changes the on-disk format, which is unacceptable anyway.

Coly Li

Quoted 23 lines

This bug stays there since first upstream version of bcache, don't know why it lives so long, or am i missing something, please point me out if I'm wrong, thanks!

---
  include/uapi/linux/bcache.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
include/uapi/linux/bcache.h +1 -1
index cf7399f03b71..dccd89756451 100644
@@ -288,7 +288,7 @@ BITMASK(CACHE_REPLACEMENT,		struct cache_sb, flags, 2, 3);
  #define CACHE_REPLACEMENT_FIFO		1U
  #define CACHE_REPLACEMENT_RANDOM	2U
 
-BITMASK(BDEV_CACHE_MODE,		struct cache_sb, flags, 0, 4);
+BITMASK(BDEV_CACHE_MODE,		struct cache_sb, flags, 5, 4);
  #define CACHE_MODE_WRITETHROUGH		0U
  #define CACHE_MODE_WRITEBACK		1U
  #define CACHE_MODE_WRITEAROUND		2U
author
#2 lore raw
Quoted 8 lines · Coly Li
Quoted 7 lines · Lin Feng

Original codes:
BITMASK(CACHE_SYNC, struct cache_sb, flags, 0, 1);
BITMASK(CACHE_DISCARD, struct cache_sb, flags, 1, 1);
BITMASK(CACHE_REPLACEMENT, struct cache_sb, flags, 2, 3);
...
BITMASK(BDEV_CACHE_MODE, struct cache_sb, flags, 0, 4);

Quoted 9 lines
Quoted 3 lines

Should BDEV_CACHE_MODE bits start from bit-nr 5(2+3) else it overlaps with previous defined bit chunks, since we have 4 types of cache modes, BDEV_CACHE_MODE will overwrite CACHE_SYNC and CACHE_DISCARD bits.

The overlap won't happen, previous lines are for cache device, and what you modified is for backing device.

And your patch changes the on-disk format, which is unacceptable anyway.

Yes, you are right, this patch happened as I started reading bcache codes, I did not get a whole picture of bcache at that time, I'm sorry for making noise.

Thank you, Coly!

linfeng

Quoted 27 lines

Coly Li

Quoted 23 lines

This bug stays there since first upstream version of bcache, don't know why it lives so long, or am i missing something, please point me out if I'm wrong, thanks!

---
   include/uapi/linux/bcache.h | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
include/uapi/linux/bcache.h +1 -1
index cf7399f03b71..dccd89756451 100644
@@ -288,7 +288,7 @@ BITMASK(CACHE_REPLACEMENT,		struct cache_sb, flags, 2, 3);
   #define CACHE_REPLACEMENT_FIFO		1U
   #define CACHE_REPLACEMENT_RANDOM	2U
 
-BITMASK(BDEV_CACHE_MODE,		struct cache_sb, flags, 0, 4);
+BITMASK(BDEV_CACHE_MODE,		struct cache_sb, flags, 5, 4);
   #define CACHE_MODE_WRITETHROUGH		0U
   #define CACHE_MODE_WRITEBACK		1U
   #define CACHE_MODE_WRITEAROUND		2U