Re: [PATCH 11/35] nilfs2: Convert nilfs_page_mkwrite() to use a folio

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 7, 2023 at 2:39 AM Matthew Wilcox (Oracle) wrote:
>
> Using the new folio APIs saves seven hidden calls to compound_head().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> ---
>  fs/nilfs2/file.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)

I'm still in the middle of reviewing this series, but I had one
question in a relevant part outside of this patch, so I'd like to ask
you a question.

In block_page_mkwrite() that nilfs_page_mkwrite() calls,
__block_write_begin_int() was called with the range using
folio_size(), as shown below:

        end = folio_size(folio);
        /* folio is wholly or partially inside EOF */
        if (folio_pos(folio) + end > size)
                end = size - folio_pos(folio);

        ret = __block_write_begin_int(folio, 0, end, get_block, NULL);
        ...

On the other hand, __block_write_begin_int() takes a folio as an
argument, but uses a PAGE_SIZE-based remainder calculation and BUG_ON
checks:

int __block_write_begin_int(struct folio *folio, loff_t pos, unsigned len,
                get_block_t *get_block, const struct iomap *iomap)
{
        unsigned from = pos & (PAGE_SIZE - 1);
        unsigned to = from + len;
        ...
        BUG_ON(from > PAGE_SIZE);
        BUG_ON(to > PAGE_SIZE);
        ...

So, it looks like this function causes a kernel BUG if it's called
from block_page_mkwrite() and folio_size() exceeds PAGE_SIZE.

Is this constraint intentional or temporary in folio conversions ?

Regards,
Ryusuke Konishi

>
> diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c
> index 740ce26d1e76..bec33b89a075 100644
> --- a/fs/nilfs2/file.c
> +++ b/fs/nilfs2/file.c
> @@ -45,34 +45,36 @@ int nilfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
>  static vm_fault_t nilfs_page_mkwrite(struct vm_fault *vmf)
>  {
>         struct vm_area_struct *vma = vmf->vma;
> -       struct page *page = vmf->page;
> +       struct folio *folio = page_folio(vmf->page);
>         struct inode *inode = file_inode(vma->vm_file);
>         struct nilfs_transaction_info ti;
> +       struct buffer_head *bh, *head;
>         int ret = 0;
>
>         if (unlikely(nilfs_near_disk_full(inode->i_sb->s_fs_info)))
>                 return VM_FAULT_SIGBUS; /* -ENOSPC */
>
>         sb_start_pagefault(inode->i_sb);
> -       lock_page(page);
> -       if (page->mapping != inode->i_mapping ||
> -           page_offset(page) >= i_size_read(inode) || !PageUptodate(page)) {
> -               unlock_page(page);
> +       folio_lock(folio);
> +       if (folio->mapping != inode->i_mapping ||
> +           folio_pos(folio) >= i_size_read(inode) ||
> +           !folio_test_uptodate(folio)) {
> +               folio_unlock(folio);
>                 ret = -EFAULT;  /* make the VM retry the fault */
>                 goto out;
>         }
>
>         /*
> -        * check to see if the page is mapped already (no holes)
> +        * check to see if the folio is mapped already (no holes)
>          */
> -       if (PageMappedToDisk(page))
> +       if (folio_test_mappedtodisk(folio))
>                 goto mapped;
>
> -       if (page_has_buffers(page)) {
> -               struct buffer_head *bh, *head;
> +       head = folio_buffers(folio);
> +       if (head) {
>                 int fully_mapped = 1;
>
> -               bh = head = page_buffers(page);
> +               bh = head;
>                 do {
>                         if (!buffer_mapped(bh)) {
>                                 fully_mapped = 0;
> @@ -81,11 +83,11 @@ static vm_fault_t nilfs_page_mkwrite(struct vm_fault *vmf)
>                 } while (bh = bh->b_this_page, bh != head);
>
>                 if (fully_mapped) {
> -                       SetPageMappedToDisk(page);
> +                       folio_set_mappedtodisk(folio);
>                         goto mapped;
>                 }
>         }
> -       unlock_page(page);
> +       folio_unlock(folio);
>
>         /*
>          * fill hole blocks
> @@ -105,7 +107,7 @@ static vm_fault_t nilfs_page_mkwrite(struct vm_fault *vmf)
>         nilfs_transaction_commit(inode->i_sb);
>
>   mapped:
> -       wait_for_stable_page(page);
> +       folio_wait_stable(folio);
>   out:
>         sb_end_pagefault(inode->i_sb);
>         return vmf_fs_error(ret);
> --
> 2.42.0
>





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux