Re: [PATCH] libceph: use iov_iter_extract_pages() in ceph_msg_data_iter_next()

On 8/19/26 1:44 AM, Christoph Hellwig wrote:
> 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 <[email protected]>
>> ---
>> 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.
> 

I'm perfectly happy to do that. I just want the PageWriteback() gone
and wanted to try to fix it properly rather than just delete the BUG()
or throw in a page_folio(). But seems like "properly" in this case may
be a deeper hole than I want to jump in to right now ;)