linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>, Milan Broz <gmazyland@gmail.com>,
	linux-block@vger.kernel.org, axboe@kernel.dk,
	dm-devel@redhat.com,
	"Martin K. Petersen" <martin.petersen@oracle.com>
Subject: Re: [PATCH] bio-integrity: revert "stop abusing bi_end_io"
Date: Sat, 5 Aug 2017 15:44:18 +0200	[thread overview]
Message-ID: <20170805134418.GB17694@lst.de> (raw)
In-Reply-To: <alpine.LRH.2.02.1708030938110.21391@file01.intranet.prod.int.rdu2.redhat.com>


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	other threads:[~2017-08-05 13:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-02 12:27 [RFC PATCH] bio-integrity: Fix regression if profile verify_fn is NULL Milan Broz
2017-08-02 12:55 ` Christoph Hellwig
2017-08-02 13:15   ` Milan Broz
2017-08-02 14:11     ` Martin K. Petersen
2017-08-02 15:24       ` Milan Broz
2017-08-03 14:10   ` [PATCH] bio-integrity: revert "stop abusing bi_end_io" Mikulas Patocka
2017-08-05 13:44     ` Christoph Hellwig [this message]
2017-08-05 14:46       ` Mikulas Patocka
2017-08-05 20:25         ` Martin K. Petersen
2017-08-06 18:49           ` Mikulas Patocka
2017-08-07 15:48             ` Martin K. Petersen
2017-08-08  6:47               ` Milan Broz
2017-08-05 20:19       ` Martin K. Petersen
2017-08-09 14:07         ` Christoph Hellwig
2017-08-06 13:30 ` [RFC PATCH] bio-integrity: Fix regression if profile verify_fn is NULL Thorsten Leemhuis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170805134418.GB17694@lst.de \
    --to=hch@lst.de \
    --cc=axboe@kernel.dk \
    --cc=dm-devel@redhat.com \
    --cc=gmazyland@gmail.com \
    --cc=linux-block@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=mpatocka@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).