All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ming Lei <ming.lei@redhat.com>
To: Caleb Sander Mateos <csander@purestorage.com>
Cc: huang-jl <huang-jl@deepseek.com>,
	axboe@kernel.dk, io-uring@vger.kernel.org, nj.shetty@samsung.com
Subject: Re: [PATCH v6.20] io_uring/rsrc: refactor io_import_kbuf() to use single loop
Date: Wed, 24 Dec 2025 06:45:44 +0800	[thread overview]
Message-ID: <aUsbmGlQI9vC70IW@fedora> (raw)
In-Reply-To: <CADUfDZpXNcBuA0Z6+btpw1M+iiyQV2KK0xx6FvHAqoUEMxwO1g@mail.gmail.com>

On Tue, Dec 23, 2025 at 02:56:45PM -0500, Caleb Sander Mateos wrote:
> On Mon, Dec 22, 2025 at 9:30 PM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > On Mon, Dec 22, 2025 at 02:56:02PM -0500, Caleb Sander Mateos wrote:
> > > On Wed, Dec 17, 2025 at 9:28 PM Ming Lei <ming.lei@redhat.com> wrote:
> > > >
> > > > On Thu, Dec 18, 2025 at 09:42:43AM +0800, Ming Lei wrote:
> > > > > On Wed, Dec 17, 2025 at 11:16:47PM +0800, huang-jl wrote:
> > > > > > The code looks correct to me.
> > > > > >
> > > > > > > This simplifies the logic
> > > > > >
> > > > > > I'm not an expert in Linux development, but from my perspective, the
> > > > > > original version seems simpler and more readable. The semantics of
> > > > > > iov_iter_advance() are clear and well-understood.
> > > > > >
> > > > > > That said, I understand the appeal of merging them into a single loop.
> > > > > >
> > > > > > > and avoids the overhead of iov_iter_advance()
> > > > > >
> > > > > > Could you clarify what overhead you mean? If it's the function call
> > > > > > overhead, I think the compiler would inline it anyway. The actual
> > > > > > iteration work seems equivalent between both approaches.
> > > > >
> > > > > iov_iter_advance() is global function, and it can't be inline.
> > > > >
> > > > > Also single loop is more readable, cause ->iov_offset can be ignored easily.
> > > > >
> > > > > In theory, re-calculating nr_segs isn't necessary, it is just for avoiding
> > > > > potential split, however not get idea how it is triggered. Nitesh didn't
> > > > > mention the exact reason:
> > > > >
> > > > > https://lkml.org/lkml/2025/4/16/351
> > > > >
> > > > > I will look at the reason and see if it can be avoided.
> > > >
> > > > The reason is in both bio_iov_bvec_set() and bio_may_need_split().
> > >
> > > nr_segs is not just a performance optimization, it's part of the
> > > struct iov_iter API and used by iov_iter_extract_bvec_pages(), as
> > > huang-jl pointed out. I don't think it's a good idea to assume that
> > > nr_segs isn't going to be used and doesn't need to be calculated
> > > correctly.
> >
> > It doesn't have to be exact if the bytes covered by `count` won't cross
> > `nr_segs`.
> >
> > The `nr_segs` re-calculation is added only for fixing performance regression
> > in the following link:
> >
> > https://lkml.org/lkml/2025/4/16/351
> >
> > because bio_iov_bvec_set() takes iter->nr_segs for setting bio->bi_vcnt.
> 
> But iov_iter_extract_bvec_pages() appears to only use iter->nr_segs
> and not iter->count. I don't understand how it can get away with an
> overestimated iter->nr_segs.

iov_iter_extract_bvec_pages() does pass `max_size` for using iter->counter,
please see the only caller of iov_iter_extract_pages().

iov_iter has to respect both two, otherwise it is a iov_iter bug.


Thanks,
Ming


  reply	other threads:[~2025-12-23 22:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-17 12:31 [PATCH v6.20] io_uring/rsrc: refactor io_import_kbuf() to use single loop Ming Lei
2025-12-17 15:16 ` huang-jl
2025-12-18  1:42   ` Ming Lei
2025-12-18  2:27     ` Ming Lei
2025-12-18  3:19       ` huang-jl
2025-12-22 19:56       ` Caleb Sander Mateos
2025-12-23  2:30         ` Ming Lei
2025-12-23 19:56           ` Caleb Sander Mateos
2025-12-23 22:45             ` Ming Lei [this message]
2025-12-30 18:28               ` Caleb Sander Mateos

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=aUsbmGlQI9vC70IW@fedora \
    --to=ming.lei@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=csander@purestorage.com \
    --cc=huang-jl@deepseek.com \
    --cc=io-uring@vger.kernel.org \
    --cc=nj.shetty@samsung.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.