Thread (7 messages) 7 messages, 4 authors, 2017-08-06

Re: [PATCH] bio-integrity: revert "stop abusing bi_end_io"

flat view

From: Christoph Hellwig <hch@lst.de>
Date: 2017-08-05 13:44:18
Also in: dm-devel

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help