>>>>> "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.
John
Mikulas> Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
Mikulas> Cc: stable@xxxxxxxxxxxxxxx
Mikulas> Fixes: 7eada909bfd7 ("dm: add integrity target")
Mikulas> ---
Mikulas> drivers/md/dm-integrity.c | 7 ++++---
Mikulas> 1 file changed, 4 insertions(+), 3 deletions(-)
Mikulas> Index: linux-2.6/drivers/md/dm-integrity.c
Mikulas> ===================================================================
Mikulas> --- linux-2.6.orig/drivers/md/dm-integrity.c
Mikulas> +++ linux-2.6/drivers/md/dm-integrity.c
Mikulas> @@ -1589,14 +1589,14 @@ retry:
Mikulas> unsigned next_entry, i, pos;
Mikulas> unsigned ws, we;
Mikulas> - dio->range.n_sectors = min(dio->range.n_sectors, ic->free_sectors);
Mikulas> + dio->range.n_sectors = min(dio->range.n_sectors, ic->free_sectors << ic->sb->log2_sectors_per_block);
Mikulas> if (unlikely(!dio->range.n_sectors))
Mikulas> goto sleep;
Mikulas> - ic->free_sectors -= dio->range.n_sectors;
Mikulas> + ic->free_sectors -= dio->range.n_sectors >> ic->sb->log2_sectors_per_block;
Mikulas> journal_section = ic->free_section;
Mikulas> journal_entry = ic->free_section_entry;
Mikulas> - next_entry = ic->free_section_entry + dio->range.n_sectors;
Mikulas> + next_entry = ic->free_section_entry + (dio->range.n_sectors >> ic->sb->log2_sectors_per_block);
ic-> free_section_entry = next_entry % ic->journal_section_entries;
ic-> free_section += next_entry / ic->journal_section_entries;
ic-> n_uncommitted_sections += next_entry / ic->journal_section_entries;
Mikulas> @@ -1727,6 +1727,7 @@ static void pad_uncommitted(struct dm_in
Mikulas> wraparound_section(ic, &ic->free_section);
ic-> n_uncommitted_sections++;
Mikulas> }
Mikulas> + BUG_ON((ic->n_uncommitted_sections + ic->n_committed_sections) * ic->journal_section_entries + ic->free_sectors != ic->journal_sections * ic->journal_section_entries);
Mikulas> }
Mikulas> static void integrity_commit(struct work_struct *w)
Mikulas> --
Mikulas> dm-devel mailing list
Mikulas> dm-devel@xxxxxxxxxx
Mikulas> https://www.redhat.com/mailman/listinfo/dm-devel
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
