On Thu, Aug 03, 2017 at 10:10:55AM -0400, Mikulas Patocka wrote:
> That dm-crypt commit that uses bio integrity payload came 3 months before 
> 7c20f11680a441df09de7235206f70115fbf6290 and it was already present in 
> 4.12.

And on it's own that isn't an argument if your usage is indeed wrong,
and that's why we are having this discussion.

> The patch 7c20f11680a441df09de7235206f70115fbf6290 changes it so that 
> bio->bi_end_io is not changed and bio_integrity_endio is called directly 
> from bio_endio if the bio has integrity payload. The unintended 
> consequence of this change is that when a request with integrity payload 
> is finished and bio_endio is called for each cloned bio, the verification 
> routine bio_integrity_verify_fn is called for every level in the stack (it 
> used to be called only for the lowest level in 4.12). At different levels 
> in the stack, the bio may have different bi_sector value, so the repeated 
> verification can't succeed.

We can simply add another bio flag to get back to the previous
behavior.  That being said thing to do in the end is to verify it
at the top of the stack, and not the bottom eventuall.  I can cook
up a patch for that.

> I think the patch 7c20f11680a441df09de7235206f70115fbf6290 should be 
> reverted and it should be reworked for the next merge window, so that it 
> calls bio_integrity_verify_fn only for the lowest level.

And with this you will just reintroduce the memory leak for
on-stack bios we've fixed.

I'll take a look at not calling the verify function for every level,
which is wrong, and in the meantime we can discuss the uses and abuses
of the bio integrity code by dm in the separate subthread.

Reply via email to