linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v3 1/3] iomap: resched ioend completion when in non-atomic context
Date: Tue, 18 May 2021 07:38:01 -0400	[thread overview]
Message-ID: <YKOnGSJ9NR+cSRRc@bfoster> (raw)
In-Reply-To: <YKKt2isZwu0qJK/C@casper.infradead.org>

On Mon, May 17, 2021 at 06:54:34PM +0100, Matthew Wilcox wrote:
> On Mon, May 17, 2021 at 01:17:20PM -0400, Brian Foster wrote:
> > @@ -1084,9 +1084,12 @@ iomap_finish_ioend(struct iomap_ioend *ioend, int error)
> >  			next = bio->bi_private;
> >  
> >  		/* walk each page on bio, ending page IO on them */
> > -		bio_for_each_segment_all(bv, bio, iter_all)
> > +		bio_for_each_segment_all(bv, bio, iter_all) {
> >  			iomap_finish_page_writeback(inode, bv->bv_page, error,
> >  					bv->bv_len);
> > +			if (!atomic)
> > +				cond_resched();
> > +		}
> 
> I don't know that it makes sense to check after _every_ page.  I might
> go for every segment.  Some users check after every thousand pages.
> 

The handful of examples I come across on a brief scan (including the
other iomap usage) have a similar pattern as used here. I don't doubt
there are others, but I think I'd prefer to have more reasoning behind
adding more code than might be necessary (i.e. do we expect additional
overhead to be measurable here?). As it is, the intent isn't so much to
check on every page as much as this just happens to be the common point
of the function to cover both long bio chains and single vector bios
with large numbers of pages.

Brian


  reply	other threads:[~2021-05-18 11:38 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-17 17:17 [PATCH v3 0/3] iomap: avoid soft lockup warnings on large ioends Brian Foster
2021-05-17 17:17 ` [PATCH v3 1/3] iomap: resched ioend completion when in non-atomic context Brian Foster
2021-05-17 17:54   ` Matthew Wilcox
2021-05-18 11:38     ` Brian Foster [this message]
2021-05-20 21:58       ` Darrick J. Wong
2021-05-24 11:57         ` Brian Foster
2021-05-24 16:53           ` Darrick J. Wong
2021-05-26  1:19             ` Darrick J. Wong
2021-05-22  7:45   ` Ming Lei
2021-05-24 11:57     ` Brian Foster
2021-05-24 14:11       ` Ming Lei
2021-05-17 17:17 ` [PATCH v3 2/3] xfs: kick large ioends to completion workqueue Brian Foster
2021-05-26  1:20   ` Darrick J. Wong
2021-05-17 17:17 ` [PATCH RFC v3 3/3] iomap: bound ioend size to 4096 pages Brian Foster
2021-05-19 13:28   ` Christoph Hellwig
2021-05-19 14:52     ` Brian Foster
2021-05-20 23:27   ` Darrick J. Wong
2021-05-24 12:02     ` Brian Foster
2021-05-25  4:20       ` Darrick J. Wong
2021-05-25  4:29         ` Damien Le Moal
2021-05-25  7:13         ` Dave Chinner
2021-05-25  9:07         ` Andreas Gruenbacher
2021-05-26  2:12         ` Matthew Wilcox
2021-05-26  3:32           ` Darrick J. Wong

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=YKOnGSJ9NR+cSRRc@bfoster \
    --to=bfoster@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=willy@infradead.org \
    /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).