From 7d7f8e45b77ce91f3cdd0f7d9b012fe22ce2130e Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Thu, 26 Apr 2018 15:06:46 -0700 Subject: [PATCH] scoutfs: more carefully manage private bh bits The management of _checked and _valid_crc private bits in the buffer_head wasn't quite right. _checked indicates that the block has been checked and that the expensive crc verification doesn't need to be recalculated. _valid_crc then indicates the result of the crc verification. _checked is read without locks. First, we didn't make sure that _valid_crc was stored before _checked. Multiple tasks could race to see _checked before _valid_crc. So we add some memory barriers. Then we didn't clear _checked when re-reading a stale block. This meant that the moment the block was read its private flags could still indicate that it had a valid crc. We clear the private bits before we read so that we'll recalculate the crc. Signed-off-by: Zach Brown --- kmod/src/btree.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/kmod/src/btree.c b/kmod/src/btree.c index d8ff6d51..aead4756 100644 --- a/kmod/src/btree.c +++ b/kmod/src/btree.c @@ -611,6 +611,7 @@ static bool valid_referenced_block(struct scoutfs_super_block *super, __le32 existing; u32 calc; + smp_rmb(); /* load checked before crc */ if (!buffer_scoutfs_checked(bh)) { lock_buffer(bh); if (!buffer_scoutfs_checked(bh)) { @@ -619,11 +620,13 @@ static bool valid_referenced_block(struct scoutfs_super_block *super, calc = crc32c(~0, bt, SCOUTFS_BLOCK_SIZE); bt->crc = existing; - set_buffer_scoutfs_checked(bh); if (calc == le32_to_cpu(existing)) set_buffer_scoutfs_valid_crc(bh); else clear_buffer_scoutfs_valid_crc(bh); + + smp_wmb(); /* store crc before checked */ + set_buffer_scoutfs_checked(bh); } unlock_buffer(bh); } @@ -683,6 +686,9 @@ retry: lock_buffer(bh); clear_buffer_uptodate(bh); + clear_buffer_scoutfs_valid_crc(bh); + smp_wmb(); /* store crc before checked */ + clear_buffer_scoutfs_checked(bh); unlock_buffer(bh); put_bh(bh); bt = NULL;