From: Christoph Hellwig <hch@lst.de>
To: Tal Zussman <tz2294@columbia.edu>
Cc: Ilya Dryomov <idryomov@gmail.com>,
Alex Markuze <amarkuze@redhat.com>,
Viacheslav Dubeyko <slava@dubeyko.com>,
David Howells <dhowells@redhat.com>,
"Matthew Wilcox (Oracle)" <willy@infradead.org>,
Christoph Hellwig <hch@lst.de>,
ceph-devel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] libceph: use iov_iter_extract_pages() in ceph_msg_data_iter_next()
Date: Wed, 19 Aug 2026 07:44:13 +0200 [thread overview]
Message-ID: <20260819054413.GA32590@lst.de> (raw)
In-Reply-To: <20260817-ceph-msgr-writeback-v1-1-38f3754034b9@columbia.edu>
On Mon, Aug 17, 2026 at 06:16:56PM -0400, Tal Zussman wrote:
> ceph_msg_data_iter_next() gets a page reference from
> iov_iter_get_pages2() only to immediately drop it, asserting that the
> page is pinned some other way. The FIXME here predates
> iov_iter_extract_pages(), which takes no reference for kernel-backed
> iterators.
>
> CEPH_MSG_DATA_ITER data only comes from osd_req_op_extent_osd_iter(),
> whose only caller passes the netfs read iterator, which is always
> kernel-backed. Use iov_iter_extract_pages() and remove the put and the
> assertion. The messenger still relies on the upper layers to keep the
> pages alive while it uses them, as it did before. Extracting from a
> user-backed iterator would pin pages that nothing unpins, so add a
> precautionary warn in ceph_msg_data_add_iter().
Yikes, this goes through like three layers of pointless abstraction
for a single user :(
But trying to unwind those it comes from ceph_netfs_issue_read, which
is the netfs issue_read method, which is used for all kinds of
reads, but it does seem like for direct reads it uses the kinda
interesting netfs_extract_user_iter helper to turn the user iov
into a kernel one, which makes all of this such a freakin' mess.
So yeah, the analysis is right, at the same time using
iov_iter_extract_pages is just as weird as the old version. Someone
really needs to clean up all the mess in both netfs and ceph :(
> This removes the last caller of PageWriteback(), allowing the page
> flag accessors to be removed in a future patch.
>
> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> ---
> The assertion is the last caller of PageWriteback() in the tree. The
> removal of the PG_writeback page flag accessors will be sent
> separately.
What about just killing that assert and leaving the rest of this
mess in place until it is sorted out properly? iov_iter_get_pages2
is a pretty good marker for that, and it would be sad to loose that.
prev parent reply other threads:[~2026-08-19 5:44 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-17 22:16 [PATCH] libceph: use iov_iter_extract_pages() in ceph_msg_data_iter_next() Tal Zussman
2026-08-19 5:44 ` Christoph Hellwig [this message]
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=20260819054413.GA32590@lst.de \
--to=hch@lst.de \
--cc=amarkuze@redhat.com \
--cc=ceph-devel@vger.kernel.org \
--cc=dhowells@redhat.com \
--cc=idryomov@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=slava@dubeyko.com \
--cc=tz2294@columbia.edu \
--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