| Message ID | 20250221051004.2951759-1-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fscrypt: Change fscrypt_encrypt_pagecache_blocks() to take a folio | expand |
On Fri, Feb 21, 2025 at 05:10:01AM +0000, Matthew Wilcox (Oracle) wrote: > ext4 and ceph already have a folio to pass; f2fs needs to be properly > converted but this will do for now. This removes a reference > to page->index and page->mapping as well as removing a call to > compound_head(). > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> It's still assumed to be a small folio though, right? It still just allocates a "bounce page", not a "bounce folio". - Eric
On Thu, Feb 20, 2025 at 09:16:07PM -0800, Eric Biggers wrote: > On Fri, Feb 21, 2025 at 05:10:01AM +0000, Matthew Wilcox (Oracle) wrote: > > ext4 and ceph already have a folio to pass; f2fs needs to be properly > > converted but this will do for now. This removes a reference > > to page->index and page->mapping as well as removing a call to > > compound_head(). > > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > It's still assumed to be a small folio though, right? It still just allocates a > "bounce page", not a "bounce folio". Yup, I haven't figured out how to do large folio support, so any filesystem using fscrypt can't support large folios for now. I'm working on "separate folio and page" at the moment rather than "enable large folios everywhere". Maybe someone else will figure out how to support large folios in fscrypt and I won't have to ;-)
On Fri, Feb 21, 2025 at 05:35:09AM +0000, Matthew Wilcox wrote: > On Thu, Feb 20, 2025 at 09:16:07PM -0800, Eric Biggers wrote: > > On Fri, Feb 21, 2025 at 05:10:01AM +0000, Matthew Wilcox (Oracle) wrote: > > > ext4 and ceph already have a folio to pass; f2fs needs to be properly > > > converted but this will do for now. This removes a reference > > > to page->index and page->mapping as well as removing a call to > > > compound_head(). > > > > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > > > It's still assumed to be a small folio though, right? It still just allocates a > > "bounce page", not a "bounce folio". > > Yup, I haven't figured out how to do large folio support, so any > filesystem using fscrypt can't support large folios for now. I'm > working on "separate folio and page" at the moment rather than "enable > large folios everywhere". It might be a good idea to make the limitation to small folios clear in the function's kerneldoc and/or in a WARN_ON_ONCE(). > Maybe someone else will figure out how to > support large folios in fscrypt and I won't have to ;-) Decryption is easy and already done, but encryption is harder. We have to encrypt into bounce pages, since we can't overwrite the plaintext in the page cache. And when encrypting a large folio, I don't think we can rely on being able to allocate a large bounce folio of the same size; it may just not be available at the time. So we'd still need bounce pages, and the filesystem would have to keep track of potentially multiple bounce pages per folio. However, this is all specific to the original fs-layer file contents encryption path. The newer inline crypto one should just work, even without hardware support since block/blk-crypto-fallback.c would be used. (blk-crypto-fallback operates at the bio level, handles en/decryption, and manages bounce pages itself.) It actually might be time to remove the fs-layer file contents encryption path and just support the inline crypto one. - Eric
On Fri, Feb 21, 2025 at 09:09:38PM +0000, Eric Biggers wrote: > > Yup, I haven't figured out how to do large folio support, so any > > filesystem using fscrypt can't support large folios for now. I'm > > working on "separate folio and page" at the moment rather than "enable > > large folios everywhere". > > It might be a good idea to make the limitation to small folios clear in the > function's kerneldoc and/or in a WARN_ON_ONCE(). I can add a VM_BUG_ON like the other this-is-not-yet-ready-for-large-folios tests. > > Maybe someone else will figure out how to > > support large folios in fscrypt and I won't have to ;-) > > Decryption is easy and already done, but encryption is harder. We have to > encrypt into bounce pages, since we can't overwrite the plaintext in the page > cache. And when encrypting a large folio, I don't think we can rely on being > able to allocate a large bounce folio of the same size; it may just not be > available at the time. So we'd still need bounce pages, and the filesystem > would have to keep track of potentially multiple bounce pages per folio. > > However, this is all specific to the original fs-layer file contents encryption > path. The newer inline crypto one should just work, even without hardware > support since block/blk-crypto-fallback.c would be used. (blk-crypto-fallback > operates at the bio level, handles en/decryption, and manages bounce pages > itself.) It actually might be time to remove the fs-layer file contents > encryption path and just support the inline crypto one. That should be fine for f2fs and ext4, but ceph might be unhappy since it doesn't use bios. Would we need an analagous function for network filesystems?
On Fri, Feb 21, 2025 at 10:22:54PM +0000, Matthew Wilcox wrote: > On Fri, Feb 21, 2025 at 09:09:38PM +0000, Eric Biggers wrote: > > > Yup, I haven't figured out how to do large folio support, so any > > > filesystem using fscrypt can't support large folios for now. I'm > > > working on "separate folio and page" at the moment rather than "enable > > > large folios everywhere". > > > > It might be a good idea to make the limitation to small folios clear in the > > function's kerneldoc and/or in a WARN_ON_ONCE(). > > I can add a VM_BUG_ON like the other > this-is-not-yet-ready-for-large-folios tests. > > > > Maybe someone else will figure out how to > > > support large folios in fscrypt and I won't have to ;-) > > > > Decryption is easy and already done, but encryption is harder. We have to > > encrypt into bounce pages, since we can't overwrite the plaintext in the page > > cache. And when encrypting a large folio, I don't think we can rely on being > > able to allocate a large bounce folio of the same size; it may just not be > > available at the time. So we'd still need bounce pages, and the filesystem > > would have to keep track of potentially multiple bounce pages per folio. > > > > However, this is all specific to the original fs-layer file contents encryption > > path. The newer inline crypto one should just work, even without hardware > > support since block/blk-crypto-fallback.c would be used. (blk-crypto-fallback > > operates at the bio level, handles en/decryption, and manages bounce pages > > itself.) It actually might be time to remove the fs-layer file contents > > encryption path and just support the inline crypto one. > > That should be fine for f2fs and ext4, but ceph might be unhappy since > it doesn't use bios. Would we need an analagous function for network > filesystems? Oof, I forgot about ceph again. Something similar applies to ubifs (it's not block based), but ubifs uses its own bounce buffers, so it never calls fscrypt_encrypt_pagecache_blocks(). ceph does call it, though. So yes, while moving to "block based filesystems will always use inline crypto" would simplify some of fs/crypto/ and allow block-based filesystems to use large folios on encrypted files, ceph would need a different solution. I think it still has to involve encrypting the folio's data into a list of bounce pages, keeping track of them during the write, and freeing them at the end of the write. So we'd have to look at what is the easiest way to do that in the ceph case. - Eric
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index f5224a566b69..9261cd690181 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -753,7 +753,7 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc) ceph_fscache_write_to_cache(inode, page_off, len, caching); if (IS_ENCRYPTED(inode)) { - bounce_page = fscrypt_encrypt_pagecache_blocks(page, + bounce_page = fscrypt_encrypt_pagecache_blocks(folio, CEPH_FSCRYPT_BLOCK_SIZE, 0, GFP_NOFS); if (IS_ERR(bounce_page)) { @@ -1186,7 +1186,7 @@ static int ceph_writepages_start(struct address_space *mapping, if (IS_ENCRYPTED(inode)) { pages[locked_pages] = - fscrypt_encrypt_pagecache_blocks(page, + fscrypt_encrypt_pagecache_blocks(folio, PAGE_SIZE, 0, locked_pages ? GFP_NOWAIT : GFP_NOFS); if (IS_ERR(pages[locked_pages])) { diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c index 328470d40dec..1fbf523632fb 100644 --- a/fs/crypto/crypto.c +++ b/fs/crypto/crypto.c @@ -153,8 +153,8 @@ int fscrypt_crypt_data_unit(const struct fscrypt_inode_info *ci, } /** - * fscrypt_encrypt_pagecache_blocks() - Encrypt data from a pagecache page - * @page: the locked pagecache page containing the data to encrypt + * fscrypt_encrypt_pagecache_blocks() - Encrypt data from a pagecache folio + * @folio: the locked pagecache folio containing the data to encrypt * @len: size of the data to encrypt, in bytes * @offs: offset within @page of the data to encrypt, in bytes * @gfp_flags: memory allocation flags; see details below @@ -177,23 +177,20 @@ int fscrypt_crypt_data_unit(const struct fscrypt_inode_info *ci, * * Return: the new encrypted bounce page on success; an ERR_PTR() on failure */ -struct page *fscrypt_encrypt_pagecache_blocks(struct page *page, - unsigned int len, - unsigned int offs, - gfp_t gfp_flags) - +struct page *fscrypt_encrypt_pagecache_blocks(struct folio *folio, + size_t len, size_t offs, gfp_t gfp_flags) { - const struct inode *inode = page->mapping->host; + const struct inode *inode = folio->mapping->host; const struct fscrypt_inode_info *ci = inode->i_crypt_info; const unsigned int du_bits = ci->ci_data_unit_bits; const unsigned int du_size = 1U << du_bits; struct page *ciphertext_page; - u64 index = ((u64)page->index << (PAGE_SHIFT - du_bits)) + + u64 index = ((u64)folio->index << (PAGE_SHIFT - du_bits)) + (offs >> du_bits); unsigned int i; int err; - if (WARN_ON_ONCE(!PageLocked(page))) + if (WARN_ON_ONCE(!folio_test_locked(folio))) return ERR_PTR(-EINVAL); if (WARN_ON_ONCE(len <= 0 || !IS_ALIGNED(len | offs, du_size))) @@ -205,7 +202,7 @@ struct page *fscrypt_encrypt_pagecache_blocks(struct page *page, for (i = offs; i < offs + len; i += du_size, index++) { err = fscrypt_crypt_data_unit(ci, FS_ENCRYPT, index, - page, ciphertext_page, + &folio->page, ciphertext_page, du_size, i, gfp_flags); if (err) { fscrypt_free_bounce_page(ciphertext_page); @@ -213,7 +210,7 @@ struct page *fscrypt_encrypt_pagecache_blocks(struct page *page, } } SetPagePrivate(ciphertext_page); - set_page_private(ciphertext_page, (unsigned long)page); + set_page_private(ciphertext_page, (unsigned long)folio); return ciphertext_page; } EXPORT_SYMBOL(fscrypt_encrypt_pagecache_blocks); diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c index 69b8a7221a2b..37abee5016c3 100644 --- a/fs/ext4/page-io.c +++ b/fs/ext4/page-io.c @@ -522,7 +522,7 @@ int ext4_bio_write_folio(struct ext4_io_submit *io, struct folio *folio, if (io->io_bio) gfp_flags = GFP_NOWAIT | __GFP_NOWARN; retry_encrypt: - bounce_page = fscrypt_encrypt_pagecache_blocks(&folio->page, + bounce_page = fscrypt_encrypt_pagecache_blocks(folio, enc_bytes, 0, gfp_flags); if (IS_ERR(bounce_page)) { ret = PTR_ERR(bounce_page); diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index 24c5cb1f5ada..b6857b4a9787 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -2504,7 +2504,7 @@ int f2fs_encrypt_one_page(struct f2fs_io_info *fio) return 0; retry_encrypt: - fio->encrypted_page = fscrypt_encrypt_pagecache_blocks(page, + fio->encrypted_page = fscrypt_encrypt_pagecache_blocks(page_folio(page), PAGE_SIZE, 0, gfp_flags); if (IS_ERR(fio->encrypted_page)) { /* flush pending IOs and wait for a while in the ENOMEM case */ diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h index 18855cb44b1c..56fad33043d5 100644 --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -310,10 +310,8 @@ static inline void fscrypt_prepare_dentry(struct dentry *dentry, /* crypto.c */ void fscrypt_enqueue_decrypt_work(struct work_struct *); -struct page *fscrypt_encrypt_pagecache_blocks(struct page *page, - unsigned int len, - unsigned int offs, - gfp_t gfp_flags); +struct page *fscrypt_encrypt_pagecache_blocks(struct folio *folio, + size_t len, size_t offs, gfp_t gfp_flags); int fscrypt_encrypt_block_inplace(const struct inode *inode, struct page *page, unsigned int len, unsigned int offs, u64 lblk_num, gfp_t gfp_flags); @@ -480,10 +478,8 @@ static inline void fscrypt_enqueue_decrypt_work(struct work_struct *work) { } -static inline struct page *fscrypt_encrypt_pagecache_blocks(struct page *page, - unsigned int len, - unsigned int offs, - gfp_t gfp_flags) +static inline struct page *fscrypt_encrypt_pagecache_blocks(struct folio *folio, + size_t len, size_t offs, gfp_t gfp_flags) { return ERR_PTR(-EOPNOTSUPP); }
ext4 and ceph already have a folio to pass; f2fs needs to be properly converted but this will do for now. This removes a reference to page->index and page->mapping as well as removing a call to compound_head(). Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/ceph/addr.c | 4 ++-- fs/crypto/crypto.c | 21 +++++++++------------ fs/ext4/page-io.c | 2 +- fs/f2fs/data.c | 2 +- include/linux/fscrypt.h | 12 ++++-------- 5 files changed, 17 insertions(+), 24 deletions(-)