From 8b892544aeb93651eb58cc6af6560bfd4e276a3f Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Sat, 11 Jun 2016 15:12:51 -0700 Subject: [PATCH] hammer2 - Use B_IOISSUED * Get rid of the hokey B_IODEBUG use case H2 had before. * Integrate B_IOISSUED (which used to be B_IODEBUG) into hammer2 properly. * Remove the dio crc_good_mask hack. Now that hammer2_chain is more persistent we just use the flag in the cached hammer2_chain structure, clearing it if we determine that the kernel had to re-issue read I/O (at least in the full-block case). * Has approximately the same performance as the dio crc_good_mask hack had and is a bit safer w/regards to chain aliasing to the same physical block. --- sys/vfs/hammer2/hammer2.h | 1 - sys/vfs/hammer2/hammer2_chain.c | 36 +++++++++++++++++++++--------- sys/vfs/hammer2/hammer2_io.c | 5 ++++- sys/vfs/hammer2/hammer2_strategy.c | 25 +++++++++++++++++++-- 4 files changed, 52 insertions(+), 15 deletions(-) diff --git a/sys/vfs/hammer2/hammer2.h b/sys/vfs/hammer2/hammer2.h index e4b146642c..b59ae2e87d 100644 --- a/sys/vfs/hammer2/hammer2.h +++ b/sys/vfs/hammer2/hammer2.h @@ -288,7 +288,6 @@ struct hammer2_io { struct spinlock spin; struct hammer2_dev *hmp; struct buf *bp; - uint64_t crc_good_mask; off_t pbase; int psize; int refs; diff --git a/sys/vfs/hammer2/hammer2_chain.c b/sys/vfs/hammer2/hammer2_chain.c index 2e25ec0a85..baa02cb7a6 100644 --- a/sys/vfs/hammer2/hammer2_chain.c +++ b/sys/vfs/hammer2/hammer2_chain.c @@ -877,6 +877,7 @@ hammer2_chain_load_data(hammer2_chain_t *chain) { hammer2_blockref_t *bref; hammer2_dev_t *hmp; + hammer2_io_t *dio; char *bdata; int error; @@ -964,6 +965,28 @@ hammer2_chain_load_data(hammer2_chain_t *chain) } chain->error = 0; + /* + * This isn't perfect and can be ignored on OSs which do not have + * an indication as to whether a buffer is coming from cache or + * if I/O was actually issued for the read. TESTEDGOOD will work + * pretty well without the B_IOISSUED logic because chains are + * cached. + * + * If the underlying kernel buffer covers the entire chain we can + * use the B_IOISSUED indication to determine if we have to re-run + * the CRC on chain data for chains that managed to stay cached + * across the kernel disposal of the original buffer. + */ + if ((dio = chain->dio) != NULL && dio->bp) { + struct buf *bp = dio->bp; + + if (dio->psize == chain->bytes && + (bp->b_flags & B_IOISSUED)) { + chain->flags &= ~HAMMER2_CHAIN_TESTEDGOOD; + bp->b_flags &= ~B_IOISSUED; + } + } + /* * NOTE: A locked chain's data cannot be modified without first * calling hammer2_chain_modify(). @@ -974,6 +997,7 @@ hammer2_chain_load_data(hammer2_chain_t *chain) * been zero'd and marked dirty. */ bdata = hammer2_io_data(chain->dio, chain->bref.data_off); + if (chain->flags & HAMMER2_CHAIN_INITIAL) { atomic_clear_int(&chain->flags, HAMMER2_CHAIN_INITIAL); chain->bref.flags |= HAMMER2_BREF_FLAG_ZERO; @@ -985,12 +1009,8 @@ hammer2_chain_load_data(hammer2_chain_t *chain) * to calculate crc? or simple crc?). */ } else if ((chain->flags & HAMMER2_CHAIN_TESTEDGOOD) == 0) { - uint64_t mask; - TIMER(26); - if (hammer2_io_crc_good(chain, &mask)) { - atomic_set_int(&chain->flags, HAMMER2_CHAIN_TESTEDGOOD); - } else if (hammer2_chain_testcheck(chain, bdata) == 0) { + if (hammer2_chain_testcheck(chain, bdata) == 0) { kprintf("chain %016jx.%02x meth=%02x " "CHECK FAIL %08x (flags=%08x)\n", chain->bref.data_off, @@ -1000,12 +1020,6 @@ hammer2_chain_load_data(hammer2_chain_t *chain) chain->flags); chain->error = HAMMER2_ERROR_CHECK; } else { - hammer2_io_crc_setmask(chain->dio, mask); -#if 0 - kprintf("chain %02x %016jx %u\n", - chain->bref.type, chain->bref.key, - chain->bytes); -#endif atomic_set_int(&chain->flags, HAMMER2_CHAIN_TESTEDGOOD); } } diff --git a/sys/vfs/hammer2/hammer2_io.c b/sys/vfs/hammer2/hammer2_io.c index d589a391e5..3bea23e8b1 100644 --- a/sys/vfs/hammer2/hammer2_io.c +++ b/sys/vfs/hammer2/hammer2_io.c @@ -667,6 +667,7 @@ hammer2_io_data(hammer2_io_t *dio, off_t lbase) return(bp->b_data + off); } +#if 0 /* * Keep track of good CRCs in dio->good_crc_mask. XXX needs to be done * in the chain structure, but chain structure needs to be persistent as @@ -733,6 +734,7 @@ hammer2_io_crc_clrmask(hammer2_io_t *dio, uint64_t mask) } } } +#endif /* * Helpers for hammer2_io_new*() functions @@ -993,9 +995,10 @@ hammer2_io_setdirty(hammer2_io_t *dio) void hammer2_io_setinval(hammer2_io_t *dio, hammer2_off_t off, u_int bytes) { +#if 0 uint64_t mask = hammer2_io_mask(dio, off, bytes); - hammer2_io_crc_clrmask(dio, mask); +#endif if ((u_int)dio->psize == bytes) { dio->bp->b_flags |= B_INVAL | B_RELBUF; /* dio->bp->b_flags &= ~B_CACHE; not needed */ diff --git a/sys/vfs/hammer2/hammer2_strategy.c b/sys/vfs/hammer2/hammer2_strategy.c index 567ba6a742..3914f379c6 100644 --- a/sys/vfs/hammer2/hammer2_strategy.c +++ b/sys/vfs/hammer2/hammer2_strategy.c @@ -367,6 +367,15 @@ hammer2_strategy_xop_read(hammer2_xop_t *arg, int clindex) * Async operation has not completed and we now own the lock. * Determine if we can complete the operation by issuing the * frontend collection non-blocking. + * + * H2 double-buffers the data, setting B_NOTMETA on the logical + * buffer hints to the OS that the logical buffer should not be + * swapcached (since the device buffer can be). + * + * Also note that even for compressed data we would rather the + * kernel cache/swapcache device buffers more and (decompressed) + * logical buffers less, since that will significantly improve + * the amount of end-user data that can be cached. */ error = hammer2_xop_collect(&xop->head, HAMMER2_XOP_COLLECT_NOWAIT); TIMER(5); @@ -375,6 +384,7 @@ hammer2_strategy_xop_read(hammer2_xop_t *arg, int clindex) case 0: xop->finished = 1; hammer2_mtx_unlock(&xop->lock); + bp->b_flags |= B_NOTMETA; chain = xop->head.cluster.focus; hammer2_strategy_read_completion(chain, (char *)chain->data, xop->bio); @@ -384,6 +394,7 @@ hammer2_strategy_xop_read(hammer2_xop_t *arg, int clindex) case ENOENT: xop->finished = 1; hammer2_mtx_unlock(&xop->lock); + bp->b_flags |= B_NOTMETA; bp->b_resid = 0; bp->b_error = 0; bzero(bp->b_data, bp->b_bcount); @@ -426,9 +437,15 @@ hammer2_strategy_read_completion(hammer2_chain_t *chain, char *data, bp->b_error = 0; } else if (chain->bref.type == HAMMER2_BREF_TYPE_DATA) { /* - * Data is on-media, record for live dedup. + * Data is on-media, record for live dedup. Release the + * chain (try to free it) when done. The data is still + * cached by both the buffer cache in front and the + * block device behind us. This leaves more room in the + * LRU chain cache for meta-data chains which we really + * want to retain. */ hammer2_dedup_record(chain, data); + atomic_set_int(&chain->flags, HAMMER2_CHAIN_RELEASE); /* * Decompression and copy. @@ -451,7 +468,6 @@ hammer2_strategy_read_completion(hammer2_chain_t *chain, char *data, bzero(bp->b_data + chain->bytes, bp->b_bcount - chain->bytes); } - bp->b_flags |= B_NOTMETA; bp->b_resid = 0; bp->b_error = 0; break; @@ -579,6 +595,10 @@ hammer2_strategy_xop_write(hammer2_xop_t *arg, int clindex) * Async operation has not completed and we now own the lock. * Determine if we can complete the operation by issuing the * frontend collection non-blocking. + * + * H2 double-buffers the data, setting B_NOTMETA on the logical + * buffer hints to the OS that the logical buffer should not be + * swapcached (since the device buffer can be). */ error = hammer2_xop_collect(&xop->head, HAMMER2_XOP_COLLECT_NOWAIT); @@ -587,6 +607,7 @@ hammer2_strategy_xop_write(hammer2_xop_t *arg, int clindex) case 0: xop->finished = 1; hammer2_mtx_unlock(&xop->lock); + bp->b_flags |= B_NOTMETA; bp->b_resid = 0; bp->b_error = 0; biodone(bio); -- 2.41.0