bcache: add REQ_FUA to avoid data lost in writeback mode
PATCH 1/1 bcache: add REQ_FUA to avoid data lost in writeback mode
data may lost when in the follow scene of writeback mode:
1. client write data1 to bcache
2. client fdatasync
3. bcache flush cache set and backing device
if now data1 was not writed back to backing, it was only guaranteed safe in cache. 4.then cache writeback data1 to backing with only REQ_OP_WRITE
So data1 was not guaranteed in non-volatile storage, it may lost if power interruption
--- drivers/md/bcache/writeback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/md/bcache/writeback.c +1 -1
@@ -357,7 +357,7 @@ static void write_dirty(struct closure *cl)*/ if (KEY_DIRTY (&w->key)) {dirty_init (w); -bio_set_op_attrs (&io->bio,REQ_OP_WRITE ,0 ); +bio_set_op_attrs (&io->bio,REQ_OP_WRITE |REQ_FUA ,0 ); io->bio.bi_iter.bi_sector =KEY_START (&w->key);bio_set_dev (&io->bio, io->dc->bdev); io->bio.bi_end_io = dirty_endio;
Signature · 1 line
2.17.1
Quoted 8 lines · kungf
data may lost when in the follow scene of writeback mode:
1. client write data1 to bcache
2. client fdatasync
3. bcache flush cache set and backing device
if now data1 was not writed back to backing, it was only guaranteed safe in cache. 4.then cache writeback data1 to backing with onlyREQ_OP_WRITE
So data1 was not guaranteed in non-volatile storage, it may lost if power interruption
Hi,
Do you encounter such problem in real work load ? With bcache journal, I don't see the possibility of data lost with your description.
Correct me if I am wrong.
Coly Li
Quoted 19 lines
Signed-off-by kungf <[email protected]>--- drivers/md/bcache/writeback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/md/bcache/writeback.c +1
-1@@ -357,7 +357,7 @@ static void write_dirty(struct closure *cl)*/ if (KEY_DIRTY (&w->key)) {dirty_init (w); -bio_set_op_attrs (&io->bio,REQ_OP_WRITE ,0 ); +bio_set_op_attrs (&io->bio,REQ_OP_WRITE |REQ_FUA ,0 ); io->bio.bi_iter.bi_sector =KEY_START (&w->key);bio_set_dev (&io->bio, io->dc->bdev); io->bio.bi_end_io = dirty_endio;
Quoted 10 lines · Coly Li
Quoted 8 lines · kungf
data may lost when in the follow scene of writeback mode:
1. client write data1 to bcache
2. client fdatasync
3. bcache flush cache set and backing device
if now data1 was not writed back to backing, it was only guaranteed safe in cache. 4.then cache writeback data1 to backing with onlyREQ_OP_WRITE
So data1 was not guaranteed in non-volatile storage, it may lost if power interruption
Quoted 8 lines
Hi,
Do you encounter such problem in real work load ? With bcache journal, I don't see the possibility of data lost with your description.
Correct me if I am wrong.
Coly Li
If this does become necessary, then we should have a sysfs or superblock flag to disable FUA for those with RAID BBUs.
--
Eric Wheeler
Quoted 21 lines
Quoted 19 lines
Signed-off-by kungf <[email protected]>--- drivers/md/bcache/writeback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/md/bcache/writeback.c +1
-1@@ -357,7 +357,7 @@ static void write_dirty(struct closure *cl)*/ if (KEY_DIRTY (&w->key)) {dirty_init (w); -bio_set_op_attrs (&io->bio,REQ_OP_WRITE ,0 ); +bio_set_op_attrs (&io->bio,REQ_OP_WRITE |REQ_FUA ,0 ); io->bio.bi_iter.bi_sector =KEY_START (&w->key);bio_set_dev (&io->bio, io->dc->bdev); io->bio.bi_end_io = dirty_endio;
Quoted 16 lines · Eric Wheeler
Quoted 15 lines · Coly Li
Quoted 8 lines · kungf
data may lost when in the follow scene of writeback mode:
1. client write data1 to bcache
2. client fdatasync
3. bcache flush cache set and backing device
if now data1 was not writed back to backing, it was only guaranteed safe in cache. 4.then cache writeback data1 to backing with onlyREQ_OP_WRITE
So data1 was not guaranteed in non-volatile storage, it may lost if power interruptionHi,
Do you encounter such problem in real work load ? With bcache journal, I don't see the possibility of data lost with your description.
Quoted 6 lines
Quoted 3 lines
Correct me if I am wrong.
Coly Li
If this does become necessary, then we should have a sysfs or superblock flag to disable FUA for those with RAID BBUs.
Hi Eric,
I doubt it is necessary to add FUA tag for all writeback bios, it is unnecessary. If power failure happens after dirty data written to backing device and the bkey turns into clean, a following read request will go to cache device because the LBA can be indexed no matter it is dirty or clean. Unless the bkey is invalidated from the B+tree, read will always go to cache device firstly in writeback mode. If a power failure happens before the cached bkey turns from dirty to clean, just an extra writeback bio flushed from cache device to backing device with identical data. Comparing the FUA tag for all writeback bios (it will be really slow), the extra writeback IOs after a power failure is more acceptable to me.
Coly Li
Quoted 21 lines
Quoted 21 lines
Quoted 20 lines
Signed-off-by kungf <[email protected]>--- drivers/md/bcache/writeback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/md/bcache/writeback.c +1
-1@@ -357,7 +357,7 @@ static void write_dirty(struct closure *cl)*/ if (KEY_DIRTY (&w->key)) {dirty_init (w); -bio_set_op_attrs (&io->bio,REQ_OP_WRITE ,0 ); +bio_set_op_attrs (&io->bio,REQ_OP_WRITE |REQ_FUA ,0 ); io->bio.bi_iter.bi_sector =KEY_START (&w->key);bio_set_dev (&io->bio, io->dc->bdev); io->bio.bi_end_io = dirty_endio;
Quoted 26 lines · Coly Li
Quoted 16 lines · Eric Wheeler
Quoted 15 lines · Coly Li
Quoted 8 lines · kungf
data may lost when in the follow scene of writeback mode:
1. client write data1 to bcache
2. client fdatasync
3. bcache flush cache set and backing device
if now data1 was not writed back to backing, it was only guaranteed safe in cache. 4.then cache writeback data1 to backing with onlyREQ_OP_WRITE
So data1 was not guaranteed in non-volatile storage, it may lost if power interruptionHi,
Do you encounter such problem in real work load ? With bcache journal, I don't see the possibility of data lost with your description.
Quoted 6 lines
Quoted 3 lines
Correct me if I am wrong.
Coly Li
If this does become necessary, then we should have a sysfs or superblock flag to disable FUA for those with RAID BBUs.
Hi Eric,
Quoted 11 lines
I doubt it is necessary to add FUA tag for all writeback bios, it is unnecessary. If power failure happens after dirty data written to backing device and the bkey turns into clean, a following read request will go to cache device because the LBA can be indexed no matter it is dirty or clean. Unless the bkey is invalidated from the B+tree, read will always go to cache device firstly in writeback mode. If a power failure happens before the cached bkey turns from dirty to clean, just an extra writeback bio flushed from cache device to backing device with identical data. Comparing the FUA tag for all writeback bios (it will be really slow), the extra writeback IOs after a power failure is more acceptable to me.
Hi Eric,
I come to realize what the problem is. Yes there is potential possibility.With FUA the writeback performance will be very low, it is quite tricky....
Coly Li
Quoted 26 lines · Coly Li
Quoted 16 lines · Eric Wheeler
Quoted 15 lines · Coly Li
Quoted 8 lines · kungf
data may lost when in the follow scene of writeback mode:
1. client write data1 to bcache
2. client fdatasync
3. bcache flush cache set and backing device
if now data1 was not writed back to backing, it was only guaranteed safe in cache. 4.then cache writeback data1 to backing with onlyREQ_OP_WRITE
So data1 was not guaranteed in non-volatile storage, it may lost if power interruptionHi,
Do you encounter such problem in real work load ? With bcache journal, I don't see the possibility of data lost with your description.
Quoted 6 lines
Quoted 3 lines
Correct me if I am wrong.
Coly Li
If this does become necessary, then we should have a sysfs or superblock flag to disable FUA for those with RAID BBUs.
Hi Eric,
Quoted 11 lines
I doubt it is necessary to add FUA tag for all writeback bios, it is unnecessary. If power failure happens after dirty data written to backing device and the bkey turns into clean, a following read request will go to cache device because the LBA can be indexed no matter it is dirty or clean. Unless the bkey is invalidated from the B+tree, read will always go to cache device firstly in writeback mode. If a power failure happens before the cached bkey turns from dirty to clean, just an extra writeback bio flushed from cache device to backing device with identical data. Comparing the FUA tag for all writeback bios (it will be really slow), the extra writeback IOs after a power failure is more acceptable to me.
I agree. FWIW, I just learned about /sys/block/sdX/queue/write_cache from Nikos Tsironis <[email protected]>. Thus, my flag request for a FUA bypass isn't necessary anyway, even if you did want an FUA there, because FUAs are stripped when a blockdev is set to "write back" (QUEUE_FLAG_WC).
----------------------------------------------------------------------
This happens in generic_make_request_checks():
/*
* Filter flush bio's early so that make_request based
* drivers without flush support don't have to worry
* about them.
*/
if (op_is_flush(bio->bi_opf) &&
!test_bit(QUEUE_FLAG_WC, &q->queue_flags)) {
bio->bi_opf &= ~(REQ_PREFLUSH | REQ_FUA);
if (!nr_sectors) {
status = BLK_STS_OK;
goto end_io;
}
}----------------------------------------------------------------------
-Eric
Quoted 25 lines
Coly Li
Quoted 21 lines
Quoted 21 lines
Quoted 20 lines
Signed-off-by kungf <[email protected]>--- drivers/md/bcache/writeback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/md/bcache/writeback.c +1
-1@@ -357,7 +357,7 @@ static void write_dirty(struct closure *cl)*/ if (KEY_DIRTY (&w->key)) {dirty_init (w); -bio_set_op_attrs (&io->bio,REQ_OP_WRITE ,0 ); +bio_set_op_attrs (&io->bio,REQ_OP_WRITE |REQ_FUA ,0 ); io->bio.bi_iter.bi_sector =KEY_START (&w->key);bio_set_dev (&io->bio, io->dc->bdev); io->bio.bi_end_io = dirty_endio;
Quoted 10 lines · On Mon, 2 Dec 2019 19:08:59 +0800 Coly Li <[email protected]> wrote:
Quoted 8 lines · kungf
data may lost when in the follow scene of writeback mode:
1. client write data1 to bcache
2. client fdatasync
3. bcache flush cache set and backing device
if now data1 was not writed back to backing, it was only guaranteed safe in cache. 4.then cache writeback data1 to backing with onlyREQ_OP_WRITE
So data1 was not guaranteed in non-volatile storage, it may lost if power interruption
Quoted 8 lines
Hi,
Do you encounter such problem in real work load ? With bcache journal, I don't see the possibility of data lost with your description.
Correct me if I am wrong.
Coly Li
Hi Coly?
Sorry to confuse you. As i known now, write_dirty function write dirty to backing without FUA, and write_dirty_finish make dirty key clean, it means the data indexed by the key will not be writeback again, am i wrong?
I only find that the backing device will be flushed when bcache get an PREFLUSH bio, any other place it will be flushed in journal?
I made a test that write bcache with dd, and then detach it, blktrace the cache and backing device at the same time.
1. close writeback
# echo 0 > /sys/block/bcache0/bcache/writeback_running 2. write data with a fdatasync #dd if=/dev/zero of=/dev/bcache0 bs=16k count=1 oflag=direct 3. detach and trigger writeback #echo b1f40ca5-37a3-4852-9abf-6abed96d71db >/sys/block/bcache0/bcache/detach
the blow text is blkparse result.
from cache blktrace blow, we can see 16k data write to cache set, and then flush with op FWFSM(PREFLUSH|WRITE|FUA|SYNC|META)
``` 8,160 33 1 0.000000000 222844 A W 630609920 + 32 <- (8,167) 1464320 8,167 33 2 0.000000478 222844 Q W 630609920 + 32 [dd] 8,167 33 3 0.000006167 222844 G W 630609920 + 32 [dd] 8,167 33 5 0.000011385 222844 I W 630609920 + 32 [dd] 8,167 33 6 0.000023890 948 D W 630609920 + 32 [kworker/33:1H] 8,167 33 7 0.000111203 0 C W 630609920 + 32 [0] 8,160 34 1 0.000167029 215616 A FWFSM 629153808 + 8 <- (8,167) 8208 8,167 34 2 0.000167490 215616 Q FWFSM 629153808 + 8 [kworker/34:2] 8,167 34 3 0.000169061 215616 G FWFSM 629153808 + 8 [kworker/34:2] 8,167 34 4 0.000301308 949 D WFSM 629153808 + 8 [kworker/34:1H] 8,167 34 5 0.000348832 0 C WFSM 629153808 + 8 [0] 8,167 34 6 0.000349612 0 C WFSM 629153808 [0] ```
from backing blktrace blow, the backing device first get flush op FWS(PERFLUSH|WRITE|SYNC) because of we stop writeback, then get W op after detach, the 16k data was writeback to backing device, and after this, the backing device never get flush op, it means that the 16k data we write it's not safe in backing device, even we had write with fdatasync.
``` 8,144 33 1 0.000000000 222844 Q WSM 8 + 8 [dd] 8,144 33 2 0.000016609 222844 G WSM 8 + 8 [dd] 8,144 33 5 0.000020710 222844 I WSM 8 + 8 [dd] 8,144 33 6 0.000031967 948 D WSM 8 + 8 [kworker/33:1H] 8,144 33 7 0.000152945 88631 C WS 16 + 32 [0] 8,144 34 1 0.000186127 215616 Q FWS [kworker/34:2] 8,144 34 2 0.000187006 215616 G FWS [kworker/34:2] 8,144 33 8 0.000326761 0 C WSM 8 + 8 [0] 8,144 34 3 0.020195027 0 C WS 16 [0] 8,144 34 4 0.020195904 0 C FWS 16 [0] 8,144 23 1 19.415130395 215884 Q W 16 + 32 [kworker/23:2] 8,144 23 2 19.415132072 215884 G W 16 + 32 [kworker/23:2] 8,144 23 3 19.415133134 215884 I W 16 + 32 [kworker/23:2] 8,144 23 4 19.415137776 1215 D W 16 + 32 [kworker/23:1H] 8,144 23 5 19.416607260 0 C W 16 + 32 [0] 8,144 24 1 19.416640754 222593 Q WSM 8 + 8 [bcache_writebac] 8,144 24 2 19.416642698 222593 G WSM 8 + 8 [bcache_writebac] 8,144 24 3 19.416643505 222593 I WSM 8 + 8 [bcache_writebac] 8,144 24 4 19.416650589 1107 D WSM 8 + 8 [kworker/24:1H] 8,144 24 5 19.416865258 0 C WSM 8 + 8 [0] 8,144 24 6 19.416871350 221889 Q WSM 8 + 8 [kworker/24:1] 8,144 24 7 19.416872201 221889 G WSM 8 + 8 [kworker/24:1] 8,144 24 8 19.416872542 221889 I WSM 8 + 8 [kworker/24:1] 8,144 24 9 19.416875458 1107 D WSM 8 + 8 [kworker/24:1H] 8,144 24 10 19.417076935 0 C WSM 8 + 8 [0] ```
Quoted 21 lines
Quoted 20 lines
Signed-off-by kungf <[email protected]>--- drivers/md/bcache/writeback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/md/bcache/writeback.c +1
-1@@ -357,7 +357,7 @@ static void write_dirty(struct closure *cl)*/ if (KEY_DIRTY (&w->key)) {dirty_init (w); -bio_set_op_attrs (&io->bio,REQ_OP_WRITE ,0 ); +bio_set_op_attrs (&io->bio,REQ_OP_WRITE |REQ_FUA ,0 ); io->bio.bi_iter.bi_sector =KEY_START (&w->key);bio_set_dev (&io->bio, io->dc->bdev); io->bio.bi_end_io = dirty_endio;
Signature · 1 line
kungf <[email protected]>
A message that is not in the archive (sent off-list or not yet received): CAMo46+q-usqgwHWhk2mcKMvK9yMY2kfN-t10wyqm+pv8L0HqYA@mail.gmail.com
Quoted 24 lines · kungf
Quoted 7 lines · On Mon, 2 Dec 2019 at 19:09, Coly Li <[email protected] <mailto:[email protected]>>…
Quoted 5 lines · kungf
data may lost when in the follow scene of writeback mode:
1. client write data1 to bcache
2. client fdatasync
3. bcache flush cache set and backing device
if now data1 was not writed back to backing, it was only guaranteedsafe in cache.
Quoted 2 lines
Quoted 2 lines
4.then cache writeback data1 to backing with only
REQ_OP_WRITESo data1 was not guaranteed in non-volatile storage, it may lost ifpower interruption
Quoted 9 lines
Quoted 1 line
Hi,
Do you encounter such problem in real work load ? With bcache journal, I don't see the possibility of data lost with your description.
Correct me if I am wrong.
Quoted 8 lines
Quoted 2 lines
Coly Li
Hi Coly,
Sorry to confuse you. As i known now,
write_dirtyfunction write dirty to backing without FUA，andwrite_dirty_finishmake dirty key clean, it means the data indexed by the key will not be writeback again, am i wrong?
Yes, you are right. This is the behavior as design. We don't guarantee the data will be on always on platter, this is what most storage systems do.
Quoted 3 lines
I only find that the backing device will be flushed when bcache get an PREFLUSH bio, any other place it will be flushed in journal?
Storage system flushes its buffer when upper layer requires, that means if the application wants to make its writing data flushed on platter, it should explicitly issue a flush request.
What you observe and test are all as designed IMHO. The I/O stack does not guarantee any data persistent on storage media unless an explicit flush request received from upper layer and returned to upper layer.
Coly Li
Quoted 116 lines
I made a test that write bcache with dd，and then detach it, blktrace the cache and backing device at the same time.
1. close writeback# echo 0 > /sys/block/bcache0/bcache/writeback_running 2. write data with a fdatasync #dd if=/dev/zero of=/dev/bcache0 bs=16k count=1 oflag=direct 3. detach and trigger writeback #echo b1f40ca5-37a3-4852-9abf-6abed96d71db >/sys/block/bcache0/bcache/detachthe blow text is blkparse result.
from cache blktrace blow, we can see 16k data write to cache set, and then flush with op FWFSM (PREFLUSH| WRITE| FUA|SYNC|META )``` 8,160 33 1 0.000000000 222844 A W 630609920 + 32 <- (8,167) 1464320 8,167 33 2 0.000000478 222844 Q W 630609920 + 32 [dd] 8,167 33 3 0.000006167 222844 G W 630609920 + 32 [dd] 8,167 33 5 0.000011385 222844 I W 630609920 + 32 [dd] 8,167 33 6 0.000023890 948 D W 630609920 + 32 [kworker/33:1H] 8,167 33 7 0.000111203 0 C W 630609920 + 32 [0] 8,160 34 1 0.000167029 215616 A FWFSM 629153808 + 8 <- (8,167) 8208 8,167 34 2 0.000167490 215616 Q FWFSM 629153808 + 8 [kworker/34:2] 8,167 34 3 0.000169061 215616 G FWFSM 629153808 + 8 [kworker/34:2] 8,167 34 4 0.000301308 949 D WFSM 629153808 + 8 [kworker/34:1H] 8,167 34 5 0.000348832 0 C WFSM 629153808 + 8 [0] 8,167 34 6 0.000349612 0 C WFSM 629153808 [0] ```from backing blktrace blow, the backing device first get flush op FWS (PERFLUSH|WRITE|SYNC) because of we stop writeback, then get W op after detach,
the 16k data was writeback to backing device, and after this, the backing device never get flush op, */it means that the 16k data we write it's not safe in backing/*
*/device, even we dd write with fdatasync./*``` 8,144 33 1 0.000000000 222844 Q WSM 8 + 8 [dd] 8,144 33 2 0.000016609 222844 G WSM 8 + 8 [dd] 8,144 33 5 0.000020710 222844 I WSM 8 + 8 [dd] 8,144 33 6 0.000031967 948 D WSM 8 + 8 [kworker/33:1H] 8,144 33 7 0.000152945 88631 C WS 16 + 32 [0] 8,144 34 1 0.000186127 215616 Q FWS [kworker/34:2] 8,144 34 2 0.000187006 215616 G FWS [kworker/34:2] 8,144 33 8 0.000326761 0 C WSM 8 + 8 [0] 8,144 34 3 0.020195027 0 C WS 16 [0] 8,144 34 4 0.020195904 0 C FWS 16 [0] 8,144 23 1 19.415130395 215884 Q W 16 + 32 [kworker/23:2] 8,144 23 2 19.415132072 215884 G W 16 + 32 [kworker/23:2] 8,144 23 3 19.415133134 215884 I W 16 + 32 [kworker/23:2] 8,144 23 4 19.415137776 1215 D W 16 + 32 [kworker/23:1H] 8,144 23 5 19.416607260 0 C W 16 + 32 [0] 8,144 24 1 19.416640754 222593 Q WSM 8 + 8 [bcache_writebac] 8,144 24 2 19.416642698 222593 G WSM 8 + 8 [bcache_writebac] 8,144 24 3 19.416643505 222593 I WSM 8 + 8 [bcache_writebac] 8,144 24 4 19.416650589 1107 D WSM 8 + 8 [kworker/24:1H] 8,144 24 5 19.416865258 0 C WSM 8 + 8 [0] 8,144 24 6 19.416871350 221889 Q WSM 8 + 8 [kworker/24:1] 8,144 24 7 19.416872201 221889 G WSM 8 + 8 [kworker/24:1] 8,144 24 8 19.416872542 221889 I WSM 8 + 8 [kworker/24:1] 8,144 24 9 19.416875458 1107 D WSM 8 + 8 [kworker/24:1H] 8,144 24 10 19.417076935 0 C WSM 8 + 8 [0] ```On Mon, 2 Dec 2019 at 19:09, Coly Li <[email protected] <mailto:[email protected]>> wrote:
On 2019/12/2 6:24 下午, kungf wrote:
> data may lost when in the follow scene of writeback mode:
> 1. client write data1 to bcache
> 2. client fdatasync
> 3. bcache flush cache set and backing device
> if now data1 was not writed back to backing, it was only guaranteed safe in cache.
> 4.then cache writeback data1 to backing with onlyREQ_OP_WRITE> So data1 was not guaranteed in non-volatile storage, it may lost if power interruption
>Hi,
Do you encounter such problem in real work load ? With bcache journal, I don't see the possibility of data lost with your description.
Correct me if I am wrong.
Coly Li
> Signed-off-by: kungf <[email protected] <mailto:[email protected]>>
> ---
>drivers/md/bcache/writeback.c| 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --gita/drivers/md/bcache/writeback.cb/drivers/md/bcache/writeback.c
> index4a40f9eadeaf..e5cecb60569e100644
> ---a/drivers/md/bcache/writeback.c
> +++b/drivers/md/bcache/writeback.c
> @@ -357,7 +357,7 @@ static voidwrite_dirty(struct closure *cl)> */
> if (KEY_DIRTY(&w->key)) {
> dirty_init(w);
> - bio_set_op_attrs(&io->bio,REQ_OP_WRITE, 0); > + bio_set_op_attrs(&io->bio,REQ_OP_WRITE|REQ_FUA, 0); > io->bio.bi_iter.bi_sector =KEY_START(&w->key); > bio_set_dev(&io->bio,io->dc->bdev);
> io->bio.bi_end_io =dirty_endio;
>