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.
next prev parent reply other threads:[~2026-08-19 5:44 UTC|newest]
Thread overview: 4+ 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]
2026-08-19 14:07 ` Tal Zussman
2026-08-19 14:26 ` Christoph Hellwig
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 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.