All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ming Lei <ming.lei@redhat.com>
To: Kent Overstreet <kent.overstreet@gmail.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	Mike Snitzer <snitzer@redhat.com>,
	Eric Biggers <ebiggers@google.com>,
	linux-block@vger.kernel.org, dm-devel@redhat.com,
	Dmitry Monakhov <dmonakhov@openvz.org>
Subject: Re: [dm-devel] [PATCH 5.20 1/4] block: add bio_rewind() API
Date: Tue, 28 Jun 2022 15:49:28 +0800	[thread overview]
Message-ID: <YrqyiCcnvPCqsn8F@T590> (raw)
In-Reply-To: <20220628042610.wuittagsycyl4uwa@moria.home.lan>

On Tue, Jun 28, 2022 at 12:26:10AM -0400, Kent Overstreet wrote:
> On Mon, Jun 27, 2022 at 03:36:22PM +0800, Ming Lei wrote:
> > Not mention bio_iter, bvec_iter has been 32 bytes, which is too big to
> > hold in per-io data structure. With this patch, 8bytes is enough
> > to rewind one bio if the end sector is fixed.
> 
> And with rewind, you're making an assumption about the state the iterator is
> going to be in when the IO has completed.
> 
> What if the iterator was never advanced?

bio_rewind() works as expected if the iterator doesn't advance, since bytes
between the recorded position and the end position isn't changed, same
with the end position.

> 
> So say you check for that by saving some other part of the iterator - but that
> may have legitimately changed too, if the bio was redirected (bi_sector changes)
> or trimmed (bi_size changes)
> 
> I still think this is an inherently buggy interface, the way it's being proposed
> to be used.

The patch did mention that the interface should be for situation in which end
sector of bio won't change.


Thanks,
Ming
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


WARNING: multiple messages have this Message-ID (diff)
From: Ming Lei <ming.lei@redhat.com>
To: Kent Overstreet <kent.overstreet@gmail.com>
Cc: Jens Axboe <axboe@kernel.dk>, Mike Snitzer <snitzer@redhat.com>,
	linux-block@vger.kernel.org, dm-devel@redhat.com,
	Eric Biggers <ebiggers@google.com>,
	Dmitry Monakhov <dmonakhov@openvz.org>,
	"Martin K . Petersen" <martin.petersen@oracle.com>
Subject: Re: [PATCH 5.20 1/4] block: add bio_rewind() API
Date: Tue, 28 Jun 2022 15:49:28 +0800	[thread overview]
Message-ID: <YrqyiCcnvPCqsn8F@T590> (raw)
In-Reply-To: <20220628042610.wuittagsycyl4uwa@moria.home.lan>

On Tue, Jun 28, 2022 at 12:26:10AM -0400, Kent Overstreet wrote:
> On Mon, Jun 27, 2022 at 03:36:22PM +0800, Ming Lei wrote:
> > Not mention bio_iter, bvec_iter has been 32 bytes, which is too big to
> > hold in per-io data structure. With this patch, 8bytes is enough
> > to rewind one bio if the end sector is fixed.
> 
> And with rewind, you're making an assumption about the state the iterator is
> going to be in when the IO has completed.
> 
> What if the iterator was never advanced?

bio_rewind() works as expected if the iterator doesn't advance, since bytes
between the recorded position and the end position isn't changed, same
with the end position.

> 
> So say you check for that by saving some other part of the iterator - but that
> may have legitimately changed too, if the bio was redirected (bi_sector changes)
> or trimmed (bi_size changes)
> 
> I still think this is an inherently buggy interface, the way it's being proposed
> to be used.

The patch did mention that the interface should be for situation in which end
sector of bio won't change.


Thanks,
Ming


  reply	other threads:[~2022-06-28  7:49 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-24 14:12 [dm-devel] [PATCH 5.20 0/4] block/dm: add bio_rewind for improving dm requeue Ming Lei
2022-06-24 14:12 ` Ming Lei
2022-06-24 14:12 ` [dm-devel] [PATCH 5.20 1/4] block: add bio_rewind() API Ming Lei
2022-06-24 14:12   ` Ming Lei
2022-06-26 20:14   ` [dm-devel] " Kent Overstreet
2022-06-26 20:14     ` Kent Overstreet
2022-06-27  7:36     ` [dm-devel] " Ming Lei
2022-06-27  7:36       ` Ming Lei
2022-06-28  4:20       ` [dm-devel] " Kent Overstreet
2022-06-28  4:20         ` Kent Overstreet
2022-06-28  7:42         ` [dm-devel] " Ming Lei
2022-06-28  7:42           ` Ming Lei
2022-06-28 16:16           ` [dm-devel] " Kent Overstreet
2022-06-28 16:16             ` Kent Overstreet
2022-06-28 18:13         ` [dm-devel] " Jens Axboe
2022-06-28 18:13           ` Jens Axboe
2022-06-28 18:32           ` [dm-devel] " Kent Overstreet
2022-06-28 18:32             ` Kent Overstreet
2022-06-29 17:16             ` [dm-devel] " Jens Axboe
2022-06-29 17:16               ` Jens Axboe
2022-06-29 18:40               ` [dm-devel] " Kent Overstreet
2022-06-29 18:40                 ` Kent Overstreet
2022-06-29 18:51                 ` [dm-devel] " Bart Van Assche
2022-06-29 18:51                   ` Bart Van Assche
2022-06-29 19:05                   ` [dm-devel] " Kent Overstreet
2022-06-29 19:05                     ` Kent Overstreet
2022-06-29 19:37                     ` [dm-devel] " Bart Van Assche
2022-06-29 19:37                       ` Bart Van Assche
2022-06-29 19:50                       ` [dm-devel] " Kent Overstreet
2022-06-29 19:50                         ` Kent Overstreet
2022-06-29 19:59                       ` [dm-devel] " Kent Overstreet
2022-06-29 19:59                         ` Kent Overstreet
2022-06-29 19:00                 ` [dm-devel] " Jens Axboe
2022-06-29 19:00                   ` Jens Axboe
2022-06-29 19:26                   ` [dm-devel] " Kent Overstreet
2022-06-29 19:26                     ` Kent Overstreet
2022-06-29 20:51                     ` [dm-devel] " Jens Axboe
2022-06-29 20:51                       ` Jens Axboe
2022-06-29  0:49           ` [dm-devel] " Ming Lei
2022-06-29  0:49             ` Ming Lei
2022-06-28  4:26       ` [dm-devel] " Kent Overstreet
2022-06-28  4:26         ` Kent Overstreet
2022-06-28  7:49         ` Ming Lei [this message]
2022-06-28  7:49           ` Ming Lei
2022-06-28 16:36           ` [dm-devel] " Kent Overstreet
2022-06-28 16:36             ` Kent Overstreet
2022-06-28 17:41             ` [dm-devel] " Mike Snitzer
2022-06-28 17:41               ` Mike Snitzer
2022-06-28 17:52               ` [dm-devel] " Kent Overstreet
2022-06-28 17:52                 ` Kent Overstreet
2022-06-29  6:07                 ` [dm-devel] " Mike Snitzer
2022-06-29  6:07                   ` Mike Snitzer
2022-06-29 18:11                   ` [dm-devel] " Kent Overstreet
2022-06-29 18:11                     ` Kent Overstreet
2022-06-30  0:47                     ` [dm-devel] " Ming Lei
2022-06-30  0:47                       ` Ming Lei
2022-06-30  0:58                       ` [dm-devel] " Kent Overstreet
2022-06-30  0:58                         ` Kent Overstreet
2022-06-30  1:14                       ` [dm-devel] " Kent Overstreet
2022-06-30  1:14                         ` Kent Overstreet
2022-07-01  3:58                         ` [dm-devel] " Ming Lei
2022-07-01  3:58                           ` Ming Lei
2022-07-01 21:09                           ` [dm-devel] " Kent Overstreet
2022-07-01 21:09                             ` Kent Overstreet
2022-06-29  1:02             ` [dm-devel] " Ming Lei
2022-06-29  1:02               ` Ming Lei
2022-06-26 21:37   ` [dm-devel] " Eric Biggers
2022-06-26 21:37     ` Eric Biggers
2022-06-27  7:37     ` Ming Lei
2022-06-27  7:37       ` Ming Lei
2022-06-24 14:12 ` [dm-devel] [PATCH 5.20 2/4] dm: add new helper for handling dm_io requeue Ming Lei
2022-06-24 14:12   ` Ming Lei
2022-06-24 14:12 ` [dm-devel] [PATCH 5.20 3/4] dm: improve handling for DM_REQUEUE and AGAIN Ming Lei
2022-06-24 14:12   ` Ming Lei
2022-06-24 14:12 ` [dm-devel] [PATCH 5.20 4/4] dm: add two stage requeue Ming Lei
2022-06-24 14:12   ` Ming Lei

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=YrqyiCcnvPCqsn8F@T590 \
    --to=ming.lei@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=dm-devel@redhat.com \
    --cc=dmonakhov@openvz.org \
    --cc=ebiggers@google.com \
    --cc=kent.overstreet@gmail.com \
    --cc=linux-block@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=snitzer@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.