On Wed, Jul 19 2017 at 2:39pm -0400, John Stoffel <john@xxxxxxxxxxx> wrote: > >>>>> "Mikulas" == Mikulas Patocka <mpatocka@xxxxxxxxxx> writes: > > Mikulas> When using block size greater than 512 bytes, the dm-integrity target > Mikulas> allocates journal space inefficiently, it allocates one entry for each > Mikulas> 512-byte chunk of data, fills entries for each block of data and leaves > Mikulas> the remaining entries unused. This doesn't cause data corruption, but it > Mikulas> causes severe performance degradation. > > Mikulas> This patch fixes the journal allocation. As a safety it also > Mikulas> adds BUG that fires if the variables representing journal > Mikulas> usage get out of sync (it's better to crash than continue and > Mikulas> corrupt data, so BUG is justified). > > No, I don't agree. You should lock down the block device(s) using the > dm-integrity target, but you should NOT take down the entire system > because of this. Just return a failure up the stack that forces the > device into read only mode so that there's a chance to debug this > issue. > > Using a BUG_ON, especially for code that isn't proven, is just wrong. > Do a WARN_ONCE() and then return an error instead. I'll remove the BUG_ON from this stable@ fix. Mikulas, please have a look at handling the failure more gracefully (maybe like John suggests). In general, there are too many BUG_ON()s in the dm-integrity code. I let them slide for inclusion but it would probably be a good idea to look at eliminating them and putting the dm-integrity device into "fail mode" in the face of critical errors -- much like dm-thinp and dm-cache has. Mike -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
