Re: [PATCH] libceph: remove the dead dirty path in ceph_put_page_vector()

On 8/13/26 4:11 AM, Ilya Dryomov wrote:
> On Sun, Aug 9, 2026 at 5:04 PM Tal Zussman <[email protected]> wrote:
>>
>> The only caller of ceph_put_page_vector() passes dirty = false, so the
>> branch is dead code. Remove it along with the parameter. This removes a
>> call to the deprecated set_page_dirty_lock().
>>
>> Signed-off-by: Tal Zussman <[email protected]>
>> ---
>>  fs/ceph/addr.c               | 2 +-
>>  include/linux/ceph/libceph.h | 3 +--
>>  net/ceph/pagevec.c           | 7 ++-----
>>  3 files changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
>> index e2da3ab9f808..83752f3688fa 100644
>> --- a/fs/ceph/addr.c
>> +++ b/fs/ceph/addr.c
>> @@ -257,7 +257,7 @@ static void finish_netfs_read(struct ceph_osd_request *req)
>>         if (osd_data->type == CEPH_OSD_DATA_TYPE_PAGES) {
>>                 ceph_put_page_vector(osd_data->pages,
>>                                      calc_pages_for(osd_data->alignment,
>> -                                       osd_data->length), false);
>> +                                       osd_data->length));
>>         }
>>         if (err > 0) {
>>                 ceph_subvolume_metrics_record_io(fsc->mdsc, ceph_inode(inode),
>> diff --git a/include/linux/ceph/libceph.h b/include/linux/ceph/libceph.h
>> index 63e0e2aa1ce9..3098306e6173 100644
>> --- a/include/linux/ceph/libceph.h
>> +++ b/include/linux/ceph/libceph.h
>> @@ -313,8 +313,7 @@ int ceph_wait_for_latest_osdmap(struct ceph_client *client,
>>
>>  /* pagevec.c */
>>  extern void ceph_release_page_vector(struct page **pages, int num_pages);
>> -extern void ceph_put_page_vector(struct page **pages, int num_pages,
>> -                                bool dirty);
>> +void ceph_put_page_vector(struct page **pages, int num_pages);
>>  extern struct page **ceph_alloc_page_vector(int num_pages, gfp_t flags);
>>  extern void ceph_copy_from_page_vector(struct page **pages,
>>                                     void *data,
>> diff --git a/net/ceph/pagevec.c b/net/ceph/pagevec.c
>> index 858359873c4d..828b003aa2fc 100644
>> --- a/net/ceph/pagevec.c
>> +++ b/net/ceph/pagevec.c
>> @@ -10,15 +10,12 @@
>>
>>  #include <linux/ceph/libceph.h>
>>
>> -void ceph_put_page_vector(struct page **pages, int num_pages, bool dirty)
>> +void ceph_put_page_vector(struct page **pages, int num_pages)
>>  {
>>         int i;
>>
>> -       for (i = 0; i < num_pages; i++) {
>> -               if (dirty)
>> -                       set_page_dirty_lock(pages[i]);
>> +       for (i = 0; i < num_pages; i++)
>>                 put_page(pages[i]);
>> -       }
>>         kvfree(pages);
>>  }
>>  EXPORT_SYMBOL(ceph_put_page_vector);
>>
>> ---
>> base-commit: 30c0913a2b44a66dc17283a88489b3f3e12f4327
>> change-id: 20260809-ceph-put-page-vector-458bb3aa87f6
>>
>> Best regards,
>> --
>> Tal Zussman <[email protected]>
>>
> 
> Hi Tal,
> 
> I'd suggest getting rid of ceph_put_page_vector() altogether -- it's
> pretty confusing to use it to put something that wasn't allocated with
> some ceph_*_page_vector() helper.  It looks like ceph_put_page_vector()
> was paired with ceph_get_direct_page_vector() which is long gone.  The
> put_page() loop and kvfree() could be done in that single caller.
> 
> As I was looking into this, I think I spotted a related memory leak in
> ceph_netfs_issue_read().  On the fscrypt path, the "page vector" coming
> from iov_iter_get_pages_alloc2() isn't put in case of EIO caused by
> ceph_inc_osd_stopping_blocker().  Alex, please take note.
> 
> Thanks,
> 
>                  Ilya
> 

Hi Ilya,

Thanks for the review!

I'm happy to fix the ceph_netfs_issue_read() issue as well in a separate
patch by using ceph_put_page_vector() there (for backporting purposes) and
then inlining ceph_put_page_vector() in the (now two) callers. Does that
sound reasonable to you?

Thanks,
Tal