Linux-BTRFS Archive on
 help / color / Atom feed
From: Filipe Manana <>
To: Matthew Wilcox <>
Cc: linux-btrfs <>
Subject: Re: Missing unread page handling in readpages
Date: Sat, 15 Feb 2020 11:16:46 +0000
Message-ID: <> (raw)
In-Reply-To: <>

On Sat, Feb 15, 2020 at 5:22 AM Matthew Wilcox <> wrote:
> As part of my rewrite of the readahead code, I think I spotted a hole
> in btrfs' handling of errors in the current readpages code.
> btrfs_readpages() calls extent_readpages() calls (a number of things)
> then finishes up by calling submit_one_bio().  If submit_one_bio()
> returns an error, I believe btrfs never unlocks the pages which were in
> that bio.

So, the pages are unlocked at end_bio_extent_readpage().

The bio created by extent_readpages() (more specifically at
__do_readpage()) has its ->bi_end_io set to end_bio_extent_readpage().
Then submit_one_bio() calls btrfs_submit_bio_hook() or
btree_submit_bio_hook(). When these functions return an error, they
set the bio's ->bi_status to an error and then call bio_endio(), which
results in end_bio_extent_readpage() being called, and that will
unlock the pages, call SetPageError(), and do all necessary error


>  Certainly the VFS does not; the filesystem is presumed to
> unlock those pages when IO finishes.  But AFAICT, returning an error
> there means btrfs will never start IO on those pages submit_one_bio()
> is a btrfs function.  It calls tree->ops->submit_bio_hook() so that's
> either btree_submit_bio_hook() or btrfs_submit_bio_hook(), which certainly
> seem like they might be able to return errors.
> So do we need to do something to complete the bio with an error in
> order to unlock those pages?  Or have I failed to notice that already
> happening somewhere?

Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

  reply index

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-15  5:15 Matthew Wilcox
2020-02-15 11:16 ` Filipe Manana [this message]
2020-02-15 16:48   ` Matthew Wilcox

Reply instructions:

You may reply publically 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:

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

  git send-email \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-BTRFS Archive on

Archives are clonable:
	git clone --mirror linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ \
	public-inbox-index linux-btrfs

Example config snippet for mirrors

Newsgroup available over NNTP:

AGPL code for this site: git clone