mbox series

[0/5] btrfs: prepare for larger folios support

Message ID cover.1740043233.git.wqu@suse.com (mailing list archive)
Headers show
Series btrfs: prepare for larger folios support | expand

Message

Qu Wenruo Feb. 20, 2025, 9:22 a.m. UTC
This means:

- Our subpage routine should check against the folio size other than
  PAGE_SIZE

- Make functions handling filemap folios to use folio_size() other than
  PAGE_SIZE

  The most common paths are:
  * Buffered reads/writes
  * Uncompressed folio writeback
    Already handled pretty well

  * Compressed read
  * Compressed write
    To take full advantage of larger folios, we should use folio_iter
    other than bvec_iter.
    This will be a dedicated patchset, and the existing bvec_iter can
    still handle larger folios.

  Internal usages can still use page sized folios, or even pages,
  including:
  * Encoded reads/writes
  * Compressed folios
  * RAID56 internal pages
  * Scrub internal pages

This patchset will handle the above mentioned points by:

- Prepare the subpage routine to handle larger folios
  This will introduce a small overhead, as all checks are against folio
  sizes, even on x86_64 we can no longer skip subpage completely.

  This is done in the first patch.

- Convert straightforward PAGE_SIZE users to use folio_size()
  This is done in the remaining patches.

Currently this patchset is not a exhaustive conversion, I'm pretty sure
there are other complex situations which can cause problems.
Those problems can only be exposed and fixed after switching on the
experimental larger folios support later.

Qu Wenruo (5):
  btrfs: prepare subpage.c for larger folios support
  btrfs: remove the PAGE_SIZE usage inside inline extent reads
  btrfs: prepare btrfs_launcher_folio() for larger folios support
  btrfs: prepare extent_io.c for future larger folio support
  btrfs: prepare btrfs_page_mkwrite() for larger folios

 fs/btrfs/extent_io.c | 50 +++++++++++++++++++++++++-------------------
 fs/btrfs/file.c      | 19 +++++++++--------
 fs/btrfs/inode.c     |  8 +++----
 fs/btrfs/subpage.c   | 36 +++++++++++++++----------------
 fs/btrfs/subpage.h   | 24 ++++++++-------------
 5 files changed, 69 insertions(+), 68 deletions(-)

Comments

Johannes Thumshirn Feb. 21, 2025, 11:23 a.m. UTC | #1
On 20.02.25 10:26, Qu Wenruo wrote:
> Qu Wenruo (5):
>    btrfs: prepare subpage.c for larger folios support
>    btrfs: remove the PAGE_SIZE usage inside inline extent reads
>    btrfs: prepare btrfs_launcher_folio() for larger folios support
>    btrfs: prepare extent_io.c for future larger folio support
>    btrfs: prepare btrfs_page_mkwrite() for larger folios
> 
>   fs/btrfs/extent_io.c | 50 +++++++++++++++++++++++++-------------------
>   fs/btrfs/file.c      | 19 +++++++++--------
>   fs/btrfs/inode.c     |  8 +++----
>   fs/btrfs/subpage.c   | 36 +++++++++++++++----------------
>   fs/btrfs/subpage.h   | 24 ++++++++-------------
>   5 files changed, 69 insertions(+), 68 deletions(-)
> 

Hi Qu,
What am I doing wrong?

Applying: btrfs: prepare subpage.c for larger folios support
Applying: btrfs: remove the PAGE_SIZE usage inside inline extent reads
In file included from ./include/linux/kernel.h:28,
                  from ./include/linux/cpumask.h:11,
                  from ./include/linux/smp.h:13,
                  from ./include/linux/lockdep.h:14,
                  from ./include/linux/spinlock.h:63,
                  from ./include/linux/swait.h:7,
                  from ./include/linux/completion.h:12,
                  from ./include/linux/crypto.h:15,
                  from ./include/crypto/hash.h:12,
                  from fs/btrfs/inode.c:6:
fs/btrfs/inode.c: In function ‘uncompress_inline’:
fs/btrfs/inode.c:6807:41: error: ‘sectorsize’ undeclared (first use in 
this function); did you mean ‘sector_t’?
  6807 |         max_size = min_t(unsigned long, sectorsize, max_size);
       |                                         ^~~~~~~~~~
./include/linux/minmax.h:86:23: note: in definition of macro 
‘__cmp_once_unique’
    86 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
       |                       ^
./include/linux/minmax.h:161:27: note: in expansion of macro ‘__cmp_once’
   161 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
       |                           ^~~~~~~~~~
fs/btrfs/inode.c:6807:20: note: in expansion of macro ‘min_t’
  6807 |         max_size = min_t(unsigned long, sectorsize, max_size);
       |                    ^~~~~
fs/btrfs/inode.c:6807:41: note: each undeclared identifier is reported 
only once for each function it appears in
  6807 |         max_size = min_t(unsigned long, sectorsize, max_size);
       |                                         ^~~~~~~~~~
./include/linux/minmax.h:86:23: note: in definition of macro 
‘__cmp_once_unique’
    86 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
       |                       ^
./include/linux/minmax.h:161:27: note: in expansion of macro ‘__cmp_once’
   161 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
       |                           ^~~~~~~~~~
fs/btrfs/inode.c:6807:20: note: in expansion of macro ‘min_t’
  6807 |         max_size = min_t(unsigned long, sectorsize, max_size);
       |                    ^~~~~
fs/btrfs/inode.c: In function ‘read_inline_extent’:
fs/btrfs/inode.c:6841:32: error: ‘sectorsize’ undeclared (first use in 
this function); did you mean ‘sector_t’?
  6841 |         copy_size = min_t(u64, sectorsize,
       |                                ^~~~~~~~~~
./include/linux/minmax.h:86:23: note: in definition of macro 
‘__cmp_once_unique’
    86 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
       |                       ^
./include/linux/minmax.h:161:27: note: in expansion of macro ‘__cmp_once’
   161 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
       |                           ^~~~~~~~~~
fs/btrfs/inode.c:6841:21: note: in expansion of macro ‘min_t’
  6841 |         copy_size = min_t(u64, sectorsize,
       |                     ^~~~~
make[4]: *** [scripts/Makefile.build:207: fs/btrfs/inode.o] Error 1
make[3]: *** [scripts/Makefile.build:465: fs/btrfs] Error 2
make[2]: *** [scripts/Makefile.build:465: fs] Error 2
make[1]: *** [/home/johannes/src/linux/Makefile:1989: .] Error 2
Filipe Manana Feb. 21, 2025, 12:34 p.m. UTC | #2
On Fri, Feb 21, 2025 at 11:23 AM Johannes Thumshirn
<Johannes.Thumshirn@wdc.com> wrote:
>
> On 20.02.25 10:26, Qu Wenruo wrote:
> > Qu Wenruo (5):
> >    btrfs: prepare subpage.c for larger folios support
> >    btrfs: remove the PAGE_SIZE usage inside inline extent reads
> >    btrfs: prepare btrfs_launcher_folio() for larger folios support
> >    btrfs: prepare extent_io.c for future larger folio support
> >    btrfs: prepare btrfs_page_mkwrite() for larger folios
> >
> >   fs/btrfs/extent_io.c | 50 +++++++++++++++++++++++++-------------------
> >   fs/btrfs/file.c      | 19 +++++++++--------
> >   fs/btrfs/inode.c     |  8 +++----
> >   fs/btrfs/subpage.c   | 36 +++++++++++++++----------------
> >   fs/btrfs/subpage.h   | 24 ++++++++-------------
> >   5 files changed, 69 insertions(+), 68 deletions(-)
> >
>
> Hi Qu,
> What am I doing wrong?
>
> Applying: btrfs: prepare subpage.c for larger folios support
> Applying: btrfs: remove the PAGE_SIZE usage inside inline extent reads
> In file included from ./include/linux/kernel.h:28,
>                   from ./include/linux/cpumask.h:11,
>                   from ./include/linux/smp.h:13,
>                   from ./include/linux/lockdep.h:14,
>                   from ./include/linux/spinlock.h:63,
>                   from ./include/linux/swait.h:7,
>                   from ./include/linux/completion.h:12,
>                   from ./include/linux/crypto.h:15,
>                   from ./include/crypto/hash.h:12,
>                   from fs/btrfs/inode.c:6:
> fs/btrfs/inode.c: In function ‘uncompress_inline’:
> fs/btrfs/inode.c:6807:41: error: ‘sectorsize’ undeclared (first use in
> this function); did you mean ‘sector_t’?

So this seems to depend on this patchset which introduces that variable:

https://lore.kernel.org/linux-btrfs/cover.1739608189.git.wqu@suse.com/

But that patset depends on other unmerged patches, so not easy to
follow and review.
I was just trying to review that patchset.

>   6807 |         max_size = min_t(unsigned long, sectorsize, max_size);
>        |                                         ^~~~~~~~~~
> ./include/linux/minmax.h:86:23: note: in definition of macro
> ‘__cmp_once_unique’
>     86 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
>        |                       ^
> ./include/linux/minmax.h:161:27: note: in expansion of macro ‘__cmp_once’
>    161 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
>        |                           ^~~~~~~~~~
> fs/btrfs/inode.c:6807:20: note: in expansion of macro ‘min_t’
>   6807 |         max_size = min_t(unsigned long, sectorsize, max_size);
>        |                    ^~~~~
> fs/btrfs/inode.c:6807:41: note: each undeclared identifier is reported
> only once for each function it appears in
>   6807 |         max_size = min_t(unsigned long, sectorsize, max_size);
>        |                                         ^~~~~~~~~~
> ./include/linux/minmax.h:86:23: note: in definition of macro
> ‘__cmp_once_unique’
>     86 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
>        |                       ^
> ./include/linux/minmax.h:161:27: note: in expansion of macro ‘__cmp_once’
>    161 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
>        |                           ^~~~~~~~~~
> fs/btrfs/inode.c:6807:20: note: in expansion of macro ‘min_t’
>   6807 |         max_size = min_t(unsigned long, sectorsize, max_size);
>        |                    ^~~~~
> fs/btrfs/inode.c: In function ‘read_inline_extent’:
> fs/btrfs/inode.c:6841:32: error: ‘sectorsize’ undeclared (first use in
> this function); did you mean ‘sector_t’?
>   6841 |         copy_size = min_t(u64, sectorsize,
>        |                                ^~~~~~~~~~
> ./include/linux/minmax.h:86:23: note: in definition of macro
> ‘__cmp_once_unique’
>     86 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
>        |                       ^
> ./include/linux/minmax.h:161:27: note: in expansion of macro ‘__cmp_once’
>    161 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
>        |                           ^~~~~~~~~~
> fs/btrfs/inode.c:6841:21: note: in expansion of macro ‘min_t’
>   6841 |         copy_size = min_t(u64, sectorsize,
>        |                     ^~~~~
> make[4]: *** [scripts/Makefile.build:207: fs/btrfs/inode.o] Error 1
> make[3]: *** [scripts/Makefile.build:465: fs/btrfs] Error 2
> make[2]: *** [scripts/Makefile.build:465: fs] Error 2
> make[1]: *** [/home/johannes/src/linux/Makefile:1989: .] Error 2
Qu Wenruo Feb. 21, 2025, 10:33 p.m. UTC | #3
在 2025/2/21 21:53, Johannes Thumshirn 写道:
> On 20.02.25 10:26, Qu Wenruo wrote:
>> Qu Wenruo (5):
>>     btrfs: prepare subpage.c for larger folios support
>>     btrfs: remove the PAGE_SIZE usage inside inline extent reads
>>     btrfs: prepare btrfs_launcher_folio() for larger folios support
>>     btrfs: prepare extent_io.c for future larger folio support
>>     btrfs: prepare btrfs_page_mkwrite() for larger folios
>>
>>    fs/btrfs/extent_io.c | 50 +++++++++++++++++++++++++-------------------
>>    fs/btrfs/file.c      | 19 +++++++++--------
>>    fs/btrfs/inode.c     |  8 +++----
>>    fs/btrfs/subpage.c   | 36 +++++++++++++++----------------
>>    fs/btrfs/subpage.h   | 24 ++++++++-------------
>>    5 files changed, 69 insertions(+), 68 deletions(-)
>>
>
> Hi Qu,
> What am I doing wrong?
>
> Applying: btrfs: prepare subpage.c for larger folios support
> Applying: btrfs: remove the PAGE_SIZE usage inside inline extent reads

I believe it's missing some dependency.

In fact it looks like it's missing the following patch:

https://lore.kernel.org/linux-btrfs/4e0368b2d4ab74e1a2cef76000ea75cb3198696a.1739608189.git.wqu@suse.com/

I'm sorry but there are too many pending subpage patches, thus it's
harder and harder to properly trace them...

Thanks,
Qu
> In file included from ./include/linux/kernel.h:28,
>                    from ./include/linux/cpumask.h:11,
>                    from ./include/linux/smp.h:13,
>                    from ./include/linux/lockdep.h:14,
>                    from ./include/linux/spinlock.h:63,
>                    from ./include/linux/swait.h:7,
>                    from ./include/linux/completion.h:12,
>                    from ./include/linux/crypto.h:15,
>                    from ./include/crypto/hash.h:12,
>                    from fs/btrfs/inode.c:6:
> fs/btrfs/inode.c: In function ‘uncompress_inline’:
> fs/btrfs/inode.c:6807:41: error: ‘sectorsize’ undeclared (first use in
> this function); did you mean ‘sector_t’?
>    6807 |         max_size = min_t(unsigned long, sectorsize, max_size);
>         |                                         ^~~~~~~~~~
> ./include/linux/minmax.h:86:23: note: in definition of macro
> ‘__cmp_once_unique’
>      86 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
>         |                       ^
> ./include/linux/minmax.h:161:27: note: in expansion of macro ‘__cmp_once’
>     161 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
>         |                           ^~~~~~~~~~
> fs/btrfs/inode.c:6807:20: note: in expansion of macro ‘min_t’
>    6807 |         max_size = min_t(unsigned long, sectorsize, max_size);
>         |                    ^~~~~
> fs/btrfs/inode.c:6807:41: note: each undeclared identifier is reported
> only once for each function it appears in
>    6807 |         max_size = min_t(unsigned long, sectorsize, max_size);
>         |                                         ^~~~~~~~~~
> ./include/linux/minmax.h:86:23: note: in definition of macro
> ‘__cmp_once_unique’
>      86 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
>         |                       ^
> ./include/linux/minmax.h:161:27: note: in expansion of macro ‘__cmp_once’
>     161 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
>         |                           ^~~~~~~~~~
> fs/btrfs/inode.c:6807:20: note: in expansion of macro ‘min_t’
>    6807 |         max_size = min_t(unsigned long, sectorsize, max_size);
>         |                    ^~~~~
> fs/btrfs/inode.c: In function ‘read_inline_extent’:
> fs/btrfs/inode.c:6841:32: error: ‘sectorsize’ undeclared (first use in
> this function); did you mean ‘sector_t’?
>    6841 |         copy_size = min_t(u64, sectorsize,
>         |                                ^~~~~~~~~~
> ./include/linux/minmax.h:86:23: note: in definition of macro
> ‘__cmp_once_unique’
>      86 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
>         |                       ^
> ./include/linux/minmax.h:161:27: note: in expansion of macro ‘__cmp_once’
>     161 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
>         |                           ^~~~~~~~~~
> fs/btrfs/inode.c:6841:21: note: in expansion of macro ‘min_t’
>    6841 |         copy_size = min_t(u64, sectorsize,
>         |                     ^~~~~
> make[4]: *** [scripts/Makefile.build:207: fs/btrfs/inode.o] Error 1
> make[3]: *** [scripts/Makefile.build:465: fs/btrfs] Error 2
> make[2]: *** [scripts/Makefile.build:465: fs] Error 2
> make[1]: *** [/home/johannes/src/linux/Makefile:1989: .] Error 2