Hi Andrew,
please add this series to your queue for the next merge window.
This series applies page->folio conversions to nilfs2 directory
operations. This reduces hidden compound_head() calls and also
converts deprecated kmap calls to kmap_local in the directory code.
Although nilfs2 does not yet support large folios, Matthew has done
his best here to include support for large folios, which will be
needed for devices with large block sizes.
This series corresponds to the second half of the original post [1],
but with two complementary patches inserted at the beginning and some
adjustments, to prevent a kmap_local constraint violation found during
testing with highmem mapping.
[1] https://lkml.kernel.org/r/[email protected]
I have reviewed all changes and tested this for regular and small
block sizes, both on machines with and without highmem mapping. No
issues found.
Thanks,
Ryusuke Konishi
Matthew Wilcox (Oracle) (15):
nilfs2: Remove page_address() from nilfs_set_link
nilfs2: Remove page_address() from nilfs_add_link
nilfs2: Remove page_address() from nilfs_delete_entry
nilfs2: Return the mapped address from nilfs_get_page()
nilfs2: Pass the mapped address to nilfs_check_page()
nilfs2: Switch to kmap_local for directory handling
nilfs2: Add nilfs_get_folio()
nilfs2: Convert nilfs_readdir to use a folio
nilfs2: Convert nilfs_find_entry to use a folio
nilfs2: Convert nilfs_rename() to use folios
nilfs2: Convert nilfs_add_link() to use a folio
nilfs2: Convert nilfs_empty_dir() to use a folio
nilfs2: Convert nilfs_make_empty() to use a folio
nilfs2: Convert nilfs_prepare_chunk() and nilfs_commit_chunk() to
folios
nilfs2: Convert nilfs_page_bug() to nilfs_folio_bug()
Ryusuke Konishi (2):
nilfs2: move page release outside of nilfs_delete_entry and
nilfs_set_link
nilfs2: eliminate staggered calls to kunmap in nilfs_rename
fs/nilfs2/btnode.c | 4 +-
fs/nilfs2/dir.c | 244 +++++++++++++++++++++------------------------
fs/nilfs2/namei.c | 38 +++----
fs/nilfs2/nilfs.h | 20 ++--
fs/nilfs2/page.c | 25 ++---
fs/nilfs2/page.h | 6 +-
6 files changed, 159 insertions(+), 178 deletions(-)
--
2.34.1
In nilfs_rename(), calls to nilfs_put_page() to release pages obtained
with nilfs_find_entry() or nilfs_dotdot() are alternated in the normal
path.
When replacing the kernel memory mapping method from kmap to
kmap_local_{page,folio}, this violates the constraint on the calling
order of kunmap_local().
Swap the order of nilfs_put_page calls where the kmap sections of
multiple pages overlap so that they are nested, allowing direct
replacement of nilfs_put_page() -> unmap_and_put_page().
Without this reordering, that replacement will cause a kernel WARNING
in kunmap_local_indexed() on architectures with high memory mapping.
Signed-off-by: Ryusuke Konishi <[email protected]>
Cc: Matthew Wilcox (Oracle) <[email protected]>
---
fs/nilfs2/namei.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
index 99255694cbe9..d179db8074c2 100644
--- a/fs/nilfs2/namei.c
+++ b/fs/nilfs2/namei.c
@@ -411,13 +411,14 @@ static int nilfs_rename(struct mnt_idmap *idmap,
inode_set_ctime_current(old_inode);
nilfs_delete_entry(old_de, old_page);
- nilfs_put_page(old_page);
if (dir_de) {
nilfs_set_link(old_inode, dir_de, dir_page, new_dir);
nilfs_put_page(dir_page);
drop_nlink(old_dir);
}
+ nilfs_put_page(old_page);
+
nilfs_mark_inode_dirty(old_dir);
nilfs_mark_inode_dirty(old_inode);
--
2.34.1
From: "Matthew Wilcox (Oracle)" <[email protected]>
Remove six calls to compound_head() by using the folio API.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
Signed-off-by: Ryusuke Konishi <[email protected]>
---
fs/nilfs2/dir.c | 31 ++++++++++++++-----------------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index 2a759598801b..8d8c42e34148 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -439,30 +439,28 @@ int nilfs_add_link(struct dentry *dentry, struct inode *inode)
unsigned int chunk_size = nilfs_chunk_size(dir);
unsigned int reclen = NILFS_DIR_REC_LEN(namelen);
unsigned short rec_len, name_len;
- struct page *page = NULL;
+ struct folio *folio = NULL;
struct nilfs_dir_entry *de;
unsigned long npages = dir_pages(dir);
unsigned long n;
- char *kaddr;
- unsigned int from, to;
+ size_t from, to;
int err;
/*
* We take care of directory expansion in the same loop.
- * This code plays outside i_size, so it locks the page
+ * This code plays outside i_size, so it locks the folio
* to protect that region.
*/
for (n = 0; n <= npages; n++) {
+ char *kaddr = nilfs_get_folio(dir, n, &folio);
char *dir_end;
- kaddr = nilfs_get_page(dir, n, &page);
- err = PTR_ERR(kaddr);
if (IS_ERR(kaddr))
- goto out;
- lock_page(page);
+ return PTR_ERR(kaddr);
+ folio_lock(folio);
dir_end = kaddr + nilfs_last_byte(dir, n);
de = (struct nilfs_dir_entry *)kaddr;
- kaddr += PAGE_SIZE - reclen;
+ kaddr += folio_size(folio) - reclen;
while ((char *)de <= kaddr) {
if ((char *)de == dir_end) {
/* We hit i_size */
@@ -489,16 +487,16 @@ int nilfs_add_link(struct dentry *dentry, struct inode *inode)
goto got_it;
de = (struct nilfs_dir_entry *)((char *)de + rec_len);
}
- unlock_page(page);
- unmap_and_put_page(page, kaddr);
+ folio_unlock(folio);
+ folio_release_kmap(folio, kaddr);
}
BUG();
return -EINVAL;
got_it:
- from = offset_in_page(de);
+ from = offset_in_folio(folio, de);
to = from + rec_len;
- err = nilfs_prepare_chunk(page, from, to);
+ err = nilfs_prepare_chunk(&folio->page, from, to);
if (err)
goto out_unlock;
if (de->inode) {
@@ -513,16 +511,15 @@ int nilfs_add_link(struct dentry *dentry, struct inode *inode)
memcpy(de->name, name, namelen);
de->inode = cpu_to_le64(inode->i_ino);
nilfs_set_de_type(de, inode);
- nilfs_commit_chunk(page, page->mapping, from, to);
+ nilfs_commit_chunk(&folio->page, folio->mapping, from, to);
inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
nilfs_mark_inode_dirty(dir);
/* OFFSET_CACHE */
out_put:
- unmap_and_put_page(page, de);
-out:
+ folio_release_kmap(folio, de);
return err;
out_unlock:
- unlock_page(page);
+ folio_unlock(folio);
goto out_put;
}
--
2.34.1
From: "Matthew Wilcox (Oracle)" <[email protected]>
In preparation for removing kmap from directory handling, use
offset_in_page() to calculate 'from'. Matches ext2.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
Signed-off-by: Ryusuke Konishi <[email protected]>
---
fs/nilfs2/dir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index 9c0513245a3b..73f135290288 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -493,7 +493,7 @@ int nilfs_add_link(struct dentry *dentry, struct inode *inode)
return -EINVAL;
got_it:
- from = (char *)de - (char *)page_address(page);
+ from = offset_in_page(de);
to = from + rec_len;
err = nilfs_prepare_chunk(page, from, to);
if (err)
--
2.34.1
From: "Matthew Wilcox (Oracle)" <[email protected]>
Use the new folio APIs to remove calls to compound_head().
[ konishi.ryusuke: resolved a conflict due to style warning correction ]
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
Signed-off-by: Ryusuke Konishi <[email protected]>
---
fs/nilfs2/dir.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index c7b046589877..a79726182867 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -339,7 +339,7 @@ nilfs_find_entry(struct inode *dir, const struct qstr *qstr,
unsigned int reclen = NILFS_DIR_REC_LEN(namelen);
unsigned long start, n;
unsigned long npages = dir_pages(dir);
- struct page *page = NULL;
+ struct folio *folio = NULL;
struct nilfs_inode_info *ei = NILFS_I(dir);
struct nilfs_dir_entry *de;
@@ -354,7 +354,7 @@ nilfs_find_entry(struct inode *dir, const struct qstr *qstr,
start = 0;
n = start;
do {
- char *kaddr = nilfs_get_page(dir, n, &page);
+ char *kaddr = nilfs_get_folio(dir, n, &folio);
if (!IS_ERR(kaddr)) {
de = (struct nilfs_dir_entry *)kaddr;
@@ -363,18 +363,18 @@ nilfs_find_entry(struct inode *dir, const struct qstr *qstr,
if (de->rec_len == 0) {
nilfs_error(dir->i_sb,
"zero-length directory entry");
- unmap_and_put_page(page, kaddr);
+ folio_release_kmap(folio, kaddr);
goto out;
}
if (nilfs_match(namelen, name, de))
goto found;
de = nilfs_next_entry(de);
}
- unmap_and_put_page(page, kaddr);
+ folio_release_kmap(folio, kaddr);
}
if (++n >= npages)
n = 0;
- /* next page is past the blocks we've got */
+ /* next folio is past the blocks we've got */
if (unlikely(n > (dir->i_blocks >> (PAGE_SHIFT - 9)))) {
nilfs_error(dir->i_sb,
"dir %lu size %lld exceeds block count %llu",
@@ -387,7 +387,7 @@ nilfs_find_entry(struct inode *dir, const struct qstr *qstr,
return NULL;
found:
- *res_page = page;
+ *res_page = &folio->page;
ei->i_dir_start_lookup = n;
return de;
}
--
2.34.1
From: "Matthew Wilcox (Oracle)" <[email protected]>
This involves converting nilfs_find_entry(), nilfs_dotdot(),
nilfs_set_link(), nilfs_delete_entry() and nilfs_do_unlink()
to use folios as well.
[ konishi.ryusuke: followed the change of page release helper call sites ]
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
Signed-off-by: Ryusuke Konishi <[email protected]>
---
fs/nilfs2/dir.c | 68 ++++++++++++++++++++++-------------------------
fs/nilfs2/namei.c | 36 ++++++++++++-------------
fs/nilfs2/nilfs.h | 20 +++++++-------
3 files changed, 60 insertions(+), 64 deletions(-)
diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index a79726182867..2a759598801b 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -323,38 +323,35 @@ static int nilfs_readdir(struct file *file, struct dir_context *ctx)
}
/*
- * nilfs_find_entry()
+ * nilfs_find_entry()
*
- * finds an entry in the specified directory with the wanted name. It
- * returns the page in which the entry was found, and the entry itself
- * (as a parameter - res_dir). Page is returned mapped and unlocked.
- * Entry is guaranteed to be valid.
+ * Finds an entry in the specified directory with the wanted name. It
+ * returns the folio in which the entry was found, and the entry itself.
+ * The folio is mapped and unlocked. When the caller is finished with
+ * the entry, it should call folio_release_kmap().
+ *
+ * On failure, returns NULL and the caller should ignore foliop.
*/
-struct nilfs_dir_entry *
-nilfs_find_entry(struct inode *dir, const struct qstr *qstr,
- struct page **res_page)
+struct nilfs_dir_entry *nilfs_find_entry(struct inode *dir,
+ const struct qstr *qstr, struct folio **foliop)
{
const unsigned char *name = qstr->name;
int namelen = qstr->len;
unsigned int reclen = NILFS_DIR_REC_LEN(namelen);
unsigned long start, n;
unsigned long npages = dir_pages(dir);
- struct folio *folio = NULL;
struct nilfs_inode_info *ei = NILFS_I(dir);
struct nilfs_dir_entry *de;
if (npages == 0)
goto out;
- /* OFFSET_CACHE */
- *res_page = NULL;
-
start = ei->i_dir_start_lookup;
if (start >= npages)
start = 0;
n = start;
do {
- char *kaddr = nilfs_get_folio(dir, n, &folio);
+ char *kaddr = nilfs_get_folio(dir, n, foliop);
if (!IS_ERR(kaddr)) {
de = (struct nilfs_dir_entry *)kaddr;
@@ -363,14 +360,14 @@ nilfs_find_entry(struct inode *dir, const struct qstr *qstr,
if (de->rec_len == 0) {
nilfs_error(dir->i_sb,
"zero-length directory entry");
- folio_release_kmap(folio, kaddr);
+ folio_release_kmap(*foliop, kaddr);
goto out;
}
if (nilfs_match(namelen, name, de))
goto found;
de = nilfs_next_entry(de);
}
- folio_release_kmap(folio, kaddr);
+ folio_release_kmap(*foliop, kaddr);
}
if (++n >= npages)
n = 0;
@@ -387,14 +384,13 @@ nilfs_find_entry(struct inode *dir, const struct qstr *qstr,
return NULL;
found:
- *res_page = &folio->page;
ei->i_dir_start_lookup = n;
return de;
}
-struct nilfs_dir_entry *nilfs_dotdot(struct inode *dir, struct page **p)
+struct nilfs_dir_entry *nilfs_dotdot(struct inode *dir, struct folio **foliop)
{
- struct nilfs_dir_entry *de = nilfs_get_page(dir, 0, p);
+ struct nilfs_dir_entry *de = nilfs_get_folio(dir, 0, foliop);
if (IS_ERR(de))
return NULL;
@@ -405,30 +401,30 @@ ino_t nilfs_inode_by_name(struct inode *dir, const struct qstr *qstr)
{
ino_t res = 0;
struct nilfs_dir_entry *de;
- struct page *page;
+ struct folio *folio;
- de = nilfs_find_entry(dir, qstr, &page);
+ de = nilfs_find_entry(dir, qstr, &folio);
if (de) {
res = le64_to_cpu(de->inode);
- unmap_and_put_page(page, de);
+ folio_release_kmap(folio, de);
}
return res;
}
void nilfs_set_link(struct inode *dir, struct nilfs_dir_entry *de,
- struct page *page, struct inode *inode)
+ struct folio *folio, struct inode *inode)
{
- unsigned int from = offset_in_page(de);
- unsigned int to = from + nilfs_rec_len_from_disk(de->rec_len);
- struct address_space *mapping = page->mapping;
+ size_t from = offset_in_folio(folio, de);
+ size_t to = from + nilfs_rec_len_from_disk(de->rec_len);
+ struct address_space *mapping = folio->mapping;
int err;
- lock_page(page);
- err = nilfs_prepare_chunk(page, from, to);
+ folio_lock(folio);
+ err = nilfs_prepare_chunk(&folio->page, from, to);
BUG_ON(err);
de->inode = cpu_to_le64(inode->i_ino);
nilfs_set_de_type(de, inode);
- nilfs_commit_chunk(page, mapping, from, to);
+ nilfs_commit_chunk(&folio->page, mapping, from, to);
inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
}
@@ -532,14 +528,14 @@ int nilfs_add_link(struct dentry *dentry, struct inode *inode)
/*
* nilfs_delete_entry deletes a directory entry by merging it with the
- * previous entry. Page is up-to-date.
+ * previous entry. Folio is up-to-date.
*/
-int nilfs_delete_entry(struct nilfs_dir_entry *dir, struct page *page)
+int nilfs_delete_entry(struct nilfs_dir_entry *dir, struct folio *folio)
{
- struct address_space *mapping = page->mapping;
+ struct address_space *mapping = folio->mapping;
struct inode *inode = mapping->host;
- char *kaddr = (char *)((unsigned long)dir & PAGE_MASK);
- unsigned int from, to;
+ char *kaddr = (char *)((unsigned long)dir & ~(folio_size(folio) - 1));
+ size_t from, to;
struct nilfs_dir_entry *de, *pde = NULL;
int err;
@@ -559,13 +555,13 @@ int nilfs_delete_entry(struct nilfs_dir_entry *dir, struct page *page)
}
if (pde)
from = (char *)pde - kaddr;
- lock_page(page);
- err = nilfs_prepare_chunk(page, from, to);
+ folio_lock(folio);
+ err = nilfs_prepare_chunk(&folio->page, from, to);
BUG_ON(err);
if (pde)
pde->rec_len = nilfs_rec_len_to_disk(to - from);
dir->inode = 0;
- nilfs_commit_chunk(page, mapping, from, to);
+ nilfs_commit_chunk(&folio->page, mapping, from, to);
inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
out:
return err;
diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
index c08b1bf9fa7b..959bd9fb3d81 100644
--- a/fs/nilfs2/namei.c
+++ b/fs/nilfs2/namei.c
@@ -260,11 +260,11 @@ static int nilfs_do_unlink(struct inode *dir, struct dentry *dentry)
{
struct inode *inode;
struct nilfs_dir_entry *de;
- struct page *page;
+ struct folio *folio;
int err;
err = -ENOENT;
- de = nilfs_find_entry(dir, &dentry->d_name, &page);
+ de = nilfs_find_entry(dir, &dentry->d_name, &folio);
if (!de)
goto out;
@@ -279,8 +279,8 @@ static int nilfs_do_unlink(struct inode *dir, struct dentry *dentry)
inode->i_ino, inode->i_nlink);
set_nlink(inode, 1);
}
- err = nilfs_delete_entry(de, page);
- unmap_and_put_page(page, de);
+ err = nilfs_delete_entry(de, folio);
+ folio_release_kmap(folio, de);
if (err)
goto out;
@@ -348,9 +348,9 @@ static int nilfs_rename(struct mnt_idmap *idmap,
{
struct inode *old_inode = d_inode(old_dentry);
struct inode *new_inode = d_inode(new_dentry);
- struct page *dir_page = NULL;
+ struct folio *dir_folio = NULL;
struct nilfs_dir_entry *dir_de = NULL;
- struct page *old_page;
+ struct folio *old_folio;
struct nilfs_dir_entry *old_de;
struct nilfs_transaction_info ti;
int err;
@@ -363,19 +363,19 @@ static int nilfs_rename(struct mnt_idmap *idmap,
return err;
err = -ENOENT;
- old_de = nilfs_find_entry(old_dir, &old_dentry->d_name, &old_page);
+ old_de = nilfs_find_entry(old_dir, &old_dentry->d_name, &old_folio);
if (!old_de)
goto out;
if (S_ISDIR(old_inode->i_mode)) {
err = -EIO;
- dir_de = nilfs_dotdot(old_inode, &dir_page);
+ dir_de = nilfs_dotdot(old_inode, &dir_folio);
if (!dir_de)
goto out_old;
}
if (new_inode) {
- struct page *new_page;
+ struct folio *new_folio;
struct nilfs_dir_entry *new_de;
err = -ENOTEMPTY;
@@ -383,11 +383,11 @@ static int nilfs_rename(struct mnt_idmap *idmap,
goto out_dir;
err = -ENOENT;
- new_de = nilfs_find_entry(new_dir, &new_dentry->d_name, &new_page);
+ new_de = nilfs_find_entry(new_dir, &new_dentry->d_name, &new_folio);
if (!new_de)
goto out_dir;
- nilfs_set_link(new_dir, new_de, new_page, old_inode);
- unmap_and_put_page(new_page, new_de);
+ nilfs_set_link(new_dir, new_de, new_folio, old_inode);
+ folio_release_kmap(new_folio, new_de);
nilfs_mark_inode_dirty(new_dir);
inode_set_ctime_current(new_inode);
if (dir_de)
@@ -410,14 +410,14 @@ static int nilfs_rename(struct mnt_idmap *idmap,
*/
inode_set_ctime_current(old_inode);
- nilfs_delete_entry(old_de, old_page);
+ nilfs_delete_entry(old_de, old_folio);
if (dir_de) {
- nilfs_set_link(old_inode, dir_de, dir_page, new_dir);
- unmap_and_put_page(dir_page, dir_de);
+ nilfs_set_link(old_inode, dir_de, dir_folio, new_dir);
+ folio_release_kmap(dir_folio, dir_de);
drop_nlink(old_dir);
}
- unmap_and_put_page(old_page, old_de);
+ folio_release_kmap(old_folio, old_de);
nilfs_mark_inode_dirty(old_dir);
nilfs_mark_inode_dirty(old_inode);
@@ -427,9 +427,9 @@ static int nilfs_rename(struct mnt_idmap *idmap,
out_dir:
if (dir_de)
- unmap_and_put_page(dir_page, dir_de);
+ folio_release_kmap(dir_folio, dir_de);
out_old:
- unmap_and_put_page(old_page, old_de);
+ folio_release_kmap(old_folio, old_de);
out:
nilfs_transaction_abort(old_dir->i_sb);
return err;
diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index 8046490cd7fe..98cffaf0ac12 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -226,16 +226,16 @@ static inline __u32 nilfs_mask_flags(umode_t mode, __u32 flags)
}
/* dir.c */
-extern int nilfs_add_link(struct dentry *, struct inode *);
-extern ino_t nilfs_inode_by_name(struct inode *, const struct qstr *);
-extern int nilfs_make_empty(struct inode *, struct inode *);
-extern struct nilfs_dir_entry *
-nilfs_find_entry(struct inode *, const struct qstr *, struct page **);
-extern int nilfs_delete_entry(struct nilfs_dir_entry *, struct page *);
-extern int nilfs_empty_dir(struct inode *);
-extern struct nilfs_dir_entry *nilfs_dotdot(struct inode *, struct page **);
-extern void nilfs_set_link(struct inode *, struct nilfs_dir_entry *,
- struct page *, struct inode *);
+int nilfs_add_link(struct dentry *, struct inode *);
+ino_t nilfs_inode_by_name(struct inode *, const struct qstr *);
+int nilfs_make_empty(struct inode *, struct inode *);
+struct nilfs_dir_entry *nilfs_find_entry(struct inode *, const struct qstr *,
+ struct folio **);
+int nilfs_delete_entry(struct nilfs_dir_entry *, struct folio *);
+int nilfs_empty_dir(struct inode *);
+struct nilfs_dir_entry *nilfs_dotdot(struct inode *, struct folio **);
+void nilfs_set_link(struct inode *, struct nilfs_dir_entry *,
+ struct folio *, struct inode *);
/* file.c */
extern int nilfs_sync_file(struct file *, loff_t, loff_t, int);
--
2.34.1
In a few directory operations, the call to nilfs_put_page() for a page
obtained using nilfs_find_entry() or nilfs_dotdot() is hidden in
nilfs_set_link() and nilfs_delete_entry(), making it difficult to track
page release and preventing change of its call position.
By moving nilfs_put_page() out of these functions, this makes the page
get/put correspondence clearer and makes it easier to swap
nilfs_put_page() calls (and kunmap calls within them) when modifying
multiple directory entries simultaneously in nilfs_rename().
Also, update comments for nilfs_set_link() and nilfs_delete_entry() to
reflect changes in their behavior.
To make nilfs_put_page() visible from namei.c, this moves its definition
to nilfs.h and replaces existing equivalents to use it, but the exposure
of that definition is temporary and will be removed on a later
kmap -> kmap_local conversion.
Signed-off-by: Ryusuke Konishi <[email protected]>
Cc: Matthew Wilcox (Oracle) <[email protected]>
---
fs/nilfs2/dir.c | 11 +----------
fs/nilfs2/namei.c | 13 +++++++------
fs/nilfs2/nilfs.h | 6 ++++++
3 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index de2073c47651..b9f13bdf8fba 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -64,12 +64,6 @@ static inline unsigned int nilfs_chunk_size(struct inode *inode)
return inode->i_sb->s_blocksize;
}
-static inline void nilfs_put_page(struct page *page)
-{
- kunmap(page);
- put_page(page);
-}
-
/*
* Return the offset into page `page_nr' of the last valid
* byte in that page, plus one.
@@ -413,7 +407,6 @@ ino_t nilfs_inode_by_name(struct inode *dir, const struct qstr *qstr)
return res;
}
-/* Releases the page */
void nilfs_set_link(struct inode *dir, struct nilfs_dir_entry *de,
struct page *page, struct inode *inode)
{
@@ -428,7 +421,6 @@ void nilfs_set_link(struct inode *dir, struct nilfs_dir_entry *de,
de->inode = cpu_to_le64(inode->i_ino);
nilfs_set_de_type(de, inode);
nilfs_commit_chunk(page, mapping, from, to);
- nilfs_put_page(page);
inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
}
@@ -533,7 +525,7 @@ int nilfs_add_link(struct dentry *dentry, struct inode *inode)
/*
* nilfs_delete_entry deletes a directory entry by merging it with the
- * previous entry. Page is up-to-date. Releases the page.
+ * previous entry. Page is up-to-date.
*/
int nilfs_delete_entry(struct nilfs_dir_entry *dir, struct page *page)
{
@@ -569,7 +561,6 @@ int nilfs_delete_entry(struct nilfs_dir_entry *dir, struct page *page)
nilfs_commit_chunk(page, mapping, from, to);
inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
out:
- nilfs_put_page(page);
return err;
}
diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
index 2a4e7f4a8102..99255694cbe9 100644
--- a/fs/nilfs2/namei.c
+++ b/fs/nilfs2/namei.c
@@ -280,6 +280,7 @@ static int nilfs_do_unlink(struct inode *dir, struct dentry *dentry)
set_nlink(inode, 1);
}
err = nilfs_delete_entry(de, page);
+ nilfs_put_page(page);
if (err)
goto out;
@@ -386,6 +387,7 @@ static int nilfs_rename(struct mnt_idmap *idmap,
if (!new_de)
goto out_dir;
nilfs_set_link(new_dir, new_de, new_page, old_inode);
+ nilfs_put_page(new_page);
nilfs_mark_inode_dirty(new_dir);
inode_set_ctime_current(new_inode);
if (dir_de)
@@ -409,9 +411,11 @@ static int nilfs_rename(struct mnt_idmap *idmap,
inode_set_ctime_current(old_inode);
nilfs_delete_entry(old_de, old_page);
+ nilfs_put_page(old_page);
if (dir_de) {
nilfs_set_link(old_inode, dir_de, dir_page, new_dir);
+ nilfs_put_page(dir_page);
drop_nlink(old_dir);
}
nilfs_mark_inode_dirty(old_dir);
@@ -421,13 +425,10 @@ static int nilfs_rename(struct mnt_idmap *idmap,
return err;
out_dir:
- if (dir_de) {
- kunmap(dir_page);
- put_page(dir_page);
- }
+ if (dir_de)
+ nilfs_put_page(dir_page);
out_old:
- kunmap(old_page);
- put_page(old_page);
+ nilfs_put_page(old_page);
out:
nilfs_transaction_abort(old_dir->i_sb);
return err;
diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index 8046490cd7fe..afd700f5dc4e 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -237,6 +237,12 @@ extern struct nilfs_dir_entry *nilfs_dotdot(struct inode *, struct page **);
extern void nilfs_set_link(struct inode *, struct nilfs_dir_entry *,
struct page *, struct inode *);
+static inline void nilfs_put_page(struct page *page)
+{
+ kunmap(page);
+ put_page(page);
+}
+
/* file.c */
extern int nilfs_sync_file(struct file *, loff_t, loff_t, int);
--
2.34.1
From: "Matthew Wilcox (Oracle)" <[email protected]>
Remove two calls to compound_head() and switch from kmap_atomic to
kmap_local.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
Signed-off-by: Ryusuke Konishi <[email protected]>
---
fs/nilfs2/dir.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index 919936d9ec27..ff0a009a292f 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -559,21 +559,21 @@ int nilfs_delete_entry(struct nilfs_dir_entry *dir, struct folio *folio)
int nilfs_make_empty(struct inode *inode, struct inode *parent)
{
struct address_space *mapping = inode->i_mapping;
- struct page *page = grab_cache_page(mapping, 0);
+ struct folio *folio = filemap_grab_folio(mapping, 0);
unsigned int chunk_size = nilfs_chunk_size(inode);
struct nilfs_dir_entry *de;
int err;
void *kaddr;
- if (!page)
- return -ENOMEM;
+ if (IS_ERR(folio))
+ return PTR_ERR(folio);
- err = nilfs_prepare_chunk(page, 0, chunk_size);
+ err = nilfs_prepare_chunk(&folio->page, 0, chunk_size);
if (unlikely(err)) {
- unlock_page(page);
+ folio_unlock(folio);
goto fail;
}
- kaddr = kmap_atomic(page);
+ kaddr = kmap_local_folio(folio, 0);
memset(kaddr, 0, chunk_size);
de = (struct nilfs_dir_entry *)kaddr;
de->name_len = 1;
@@ -588,10 +588,10 @@ int nilfs_make_empty(struct inode *inode, struct inode *parent)
de->inode = cpu_to_le64(parent->i_ino);
memcpy(de->name, "..\0", 4);
nilfs_set_de_type(de, inode);
- kunmap_atomic(kaddr);
- nilfs_commit_chunk(page, mapping, 0, chunk_size);
+ kunmap_local(kaddr);
+ nilfs_commit_chunk(&folio->page, mapping, 0, chunk_size);
fail:
- put_page(page);
+ folio_put(folio);
return err;
}
--
2.34.1
From: "Matthew Wilcox (Oracle)" <[email protected]>
All callers now have a folio, so convert these two functions.
Saves one call to compound_head() in unlock_page().
[ konishi.ryusuke: resolved conflicts in nilfs_{set_link,delete_entry} ]
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
Signed-off-by: Ryusuke Konishi <[email protected]>
---
fs/nilfs2/dir.c | 39 +++++++++++++++++++--------------------
1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index ff0a009a292f..bc846b904b68 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -78,33 +78,32 @@ static unsigned int nilfs_last_byte(struct inode *inode, unsigned long page_nr)
return last_byte;
}
-static int nilfs_prepare_chunk(struct page *page, unsigned int from,
+static int nilfs_prepare_chunk(struct folio *folio, unsigned int from,
unsigned int to)
{
- loff_t pos = page_offset(page) + from;
+ loff_t pos = folio_pos(folio) + from;
- return __block_write_begin(page, pos, to - from, nilfs_get_block);
+ return __block_write_begin(&folio->page, pos, to - from, nilfs_get_block);
}
-static void nilfs_commit_chunk(struct page *page,
- struct address_space *mapping,
- unsigned int from, unsigned int to)
+static void nilfs_commit_chunk(struct folio *folio,
+ struct address_space *mapping, size_t from, size_t to)
{
struct inode *dir = mapping->host;
- loff_t pos = page_offset(page) + from;
- unsigned int len = to - from;
- unsigned int nr_dirty, copied;
+ loff_t pos = folio_pos(folio) + from;
+ size_t copied, len = to - from;
+ unsigned int nr_dirty;
int err;
- nr_dirty = nilfs_page_count_clean_buffers(page, from, to);
- copied = block_write_end(NULL, mapping, pos, len, len, page, NULL);
+ nr_dirty = nilfs_page_count_clean_buffers(&folio->page, from, to);
+ copied = block_write_end(NULL, mapping, pos, len, len, &folio->page, NULL);
if (pos + copied > dir->i_size)
i_size_write(dir, pos + copied);
if (IS_DIRSYNC(dir))
nilfs_set_transaction_flag(NILFS_TI_SYNC);
err = nilfs_set_file_dirty(dir, nr_dirty);
WARN_ON(err); /* do not happen */
- unlock_page(page);
+ folio_unlock(folio);
}
static bool nilfs_check_folio(struct folio *folio, char *kaddr)
@@ -409,11 +408,11 @@ void nilfs_set_link(struct inode *dir, struct nilfs_dir_entry *de,
int err;
folio_lock(folio);
- err = nilfs_prepare_chunk(&folio->page, from, to);
+ err = nilfs_prepare_chunk(folio, from, to);
BUG_ON(err);
de->inode = cpu_to_le64(inode->i_ino);
nilfs_set_de_type(de, inode);
- nilfs_commit_chunk(&folio->page, mapping, from, to);
+ nilfs_commit_chunk(folio, mapping, from, to);
inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
}
@@ -485,7 +484,7 @@ int nilfs_add_link(struct dentry *dentry, struct inode *inode)
got_it:
from = offset_in_folio(folio, de);
to = from + rec_len;
- err = nilfs_prepare_chunk(&folio->page, from, to);
+ err = nilfs_prepare_chunk(folio, from, to);
if (err)
goto out_unlock;
if (de->inode) {
@@ -500,7 +499,7 @@ int nilfs_add_link(struct dentry *dentry, struct inode *inode)
memcpy(de->name, name, namelen);
de->inode = cpu_to_le64(inode->i_ino);
nilfs_set_de_type(de, inode);
- nilfs_commit_chunk(&folio->page, folio->mapping, from, to);
+ nilfs_commit_chunk(folio, folio->mapping, from, to);
inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
nilfs_mark_inode_dirty(dir);
/* OFFSET_CACHE */
@@ -542,12 +541,12 @@ int nilfs_delete_entry(struct nilfs_dir_entry *dir, struct folio *folio)
if (pde)
from = (char *)pde - kaddr;
folio_lock(folio);
- err = nilfs_prepare_chunk(&folio->page, from, to);
+ err = nilfs_prepare_chunk(folio, from, to);
BUG_ON(err);
if (pde)
pde->rec_len = nilfs_rec_len_to_disk(to - from);
dir->inode = 0;
- nilfs_commit_chunk(&folio->page, mapping, from, to);
+ nilfs_commit_chunk(folio, mapping, from, to);
inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
out:
return err;
@@ -568,7 +567,7 @@ int nilfs_make_empty(struct inode *inode, struct inode *parent)
if (IS_ERR(folio))
return PTR_ERR(folio);
- err = nilfs_prepare_chunk(&folio->page, 0, chunk_size);
+ err = nilfs_prepare_chunk(folio, 0, chunk_size);
if (unlikely(err)) {
folio_unlock(folio);
goto fail;
@@ -589,7 +588,7 @@ int nilfs_make_empty(struct inode *inode, struct inode *parent)
memcpy(de->name, "..\0", 4);
nilfs_set_de_type(de, inode);
kunmap_local(kaddr);
- nilfs_commit_chunk(&folio->page, mapping, 0, chunk_size);
+ nilfs_commit_chunk(folio, mapping, 0, chunk_size);
fail:
folio_put(folio);
return err;
--
2.34.1
From: "Matthew Wilcox (Oracle)" <[email protected]>
All callers have a folio now, so convert it.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
Signed-off-by: Ryusuke Konishi <[email protected]>
---
fs/nilfs2/btnode.c | 4 ++--
fs/nilfs2/page.c | 25 +++++++++++++------------
fs/nilfs2/page.h | 6 +++---
3 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/fs/nilfs2/btnode.c b/fs/nilfs2/btnode.c
index 1204dd06ead8..0131d83b912d 100644
--- a/fs/nilfs2/btnode.c
+++ b/fs/nilfs2/btnode.c
@@ -190,7 +190,7 @@ int nilfs_btnode_prepare_change_key(struct address_space *btnc,
retry:
/* BUG_ON(oldkey != obh->b_folio->index); */
if (unlikely(oldkey != ofolio->index))
- NILFS_PAGE_BUG(&ofolio->page,
+ NILFS_FOLIO_BUG(ofolio,
"invalid oldkey %lld (newkey=%lld)",
(unsigned long long)oldkey,
(unsigned long long)newkey);
@@ -246,7 +246,7 @@ void nilfs_btnode_commit_change_key(struct address_space *btnc,
if (nbh == NULL) { /* blocksize == pagesize */
ofolio = obh->b_folio;
if (unlikely(oldkey != ofolio->index))
- NILFS_PAGE_BUG(&ofolio->page,
+ NILFS_FOLIO_BUG(ofolio,
"invalid oldkey %lld (newkey=%lld)",
(unsigned long long)oldkey,
(unsigned long long)newkey);
diff --git a/fs/nilfs2/page.c b/fs/nilfs2/page.c
index 94e11bcee05b..5c2eba1987bd 100644
--- a/fs/nilfs2/page.c
+++ b/fs/nilfs2/page.c
@@ -150,29 +150,30 @@ bool nilfs_folio_buffers_clean(struct folio *folio)
return true;
}
-void nilfs_page_bug(struct page *page)
+void nilfs_folio_bug(struct folio *folio)
{
+ struct buffer_head *bh, *head;
struct address_space *m;
unsigned long ino;
- if (unlikely(!page)) {
- printk(KERN_CRIT "NILFS_PAGE_BUG(NULL)\n");
+ if (unlikely(!folio)) {
+ printk(KERN_CRIT "NILFS_FOLIO_BUG(NULL)\n");
return;
}
- m = page->mapping;
+ m = folio->mapping;
ino = m ? m->host->i_ino : 0;
- printk(KERN_CRIT "NILFS_PAGE_BUG(%p): cnt=%d index#=%llu flags=0x%lx "
+ printk(KERN_CRIT "NILFS_FOLIO_BUG(%p): cnt=%d index#=%llu flags=0x%lx "
"mapping=%p ino=%lu\n",
- page, page_ref_count(page),
- (unsigned long long)page->index, page->flags, m, ino);
+ folio, folio_ref_count(folio),
+ (unsigned long long)folio->index, folio->flags, m, ino);
- if (page_has_buffers(page)) {
- struct buffer_head *bh, *head;
+ head = folio_buffers(folio);
+ if (head) {
int i = 0;
- bh = head = page_buffers(page);
+ bh = head;
do {
printk(KERN_CRIT
" BH[%d] %p: cnt=%d block#=%llu state=0x%lx\n",
@@ -258,7 +259,7 @@ int nilfs_copy_dirty_pages(struct address_space *dmap,
folio_lock(folio);
if (unlikely(!folio_test_dirty(folio)))
- NILFS_PAGE_BUG(&folio->page, "inconsistent dirty state");
+ NILFS_FOLIO_BUG(folio, "inconsistent dirty state");
dfolio = filemap_grab_folio(dmap, folio->index);
if (unlikely(IS_ERR(dfolio))) {
@@ -268,7 +269,7 @@ int nilfs_copy_dirty_pages(struct address_space *dmap,
break;
}
if (unlikely(!folio_buffers(folio)))
- NILFS_PAGE_BUG(&folio->page,
+ NILFS_FOLIO_BUG(folio,
"found empty page in dat page cache");
nilfs_copy_folio(dfolio, folio, true);
diff --git a/fs/nilfs2/page.h b/fs/nilfs2/page.h
index 968b311d265b..7e1a2c455a10 100644
--- a/fs/nilfs2/page.h
+++ b/fs/nilfs2/page.h
@@ -37,7 +37,7 @@ struct buffer_head *nilfs_grab_buffer(struct inode *, struct address_space *,
void nilfs_forget_buffer(struct buffer_head *);
void nilfs_copy_buffer(struct buffer_head *, struct buffer_head *);
bool nilfs_folio_buffers_clean(struct folio *);
-void nilfs_page_bug(struct page *);
+void nilfs_folio_bug(struct folio *);
int nilfs_copy_dirty_pages(struct address_space *, struct address_space *);
void nilfs_copy_back_pages(struct address_space *, struct address_space *);
@@ -49,7 +49,7 @@ unsigned long nilfs_find_uncommitted_extent(struct inode *inode,
sector_t start_blk,
sector_t *blkoff);
-#define NILFS_PAGE_BUG(page, m, a...) \
- do { nilfs_page_bug(page); BUG(); } while (0)
+#define NILFS_FOLIO_BUG(folio, m, a...) \
+ do { nilfs_folio_bug(folio); BUG(); } while (0)
#endif /* _NILFS_PAGE_H */
--
2.34.1
From: "Matthew Wilcox (Oracle)" <[email protected]>
Remove three calls to compound_head() by using the folio API.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
Signed-off-by: Ryusuke Konishi <[email protected]>
---
fs/nilfs2/dir.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index 8d8c42e34148..919936d9ec27 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -203,17 +203,6 @@ static void *nilfs_get_folio(struct inode *dir, unsigned long n,
return ERR_PTR(-EIO);
}
-static void *nilfs_get_page(struct inode *dir, unsigned long n,
- struct page **pagep)
-{
- struct folio *folio;
- void *kaddr = nilfs_get_folio(dir, n, &folio);
-
- if (!IS_ERR(kaddr))
- *pagep = &folio->page;
- return kaddr;
-}
-
/*
* NOTE! unlike strncmp, nilfs_match returns 1 for success, 0 for failure.
*
@@ -611,14 +600,14 @@ int nilfs_make_empty(struct inode *inode, struct inode *parent)
*/
int nilfs_empty_dir(struct inode *inode)
{
- struct page *page = NULL;
+ struct folio *folio = NULL;
char *kaddr;
unsigned long i, npages = dir_pages(inode);
for (i = 0; i < npages; i++) {
struct nilfs_dir_entry *de;
- kaddr = nilfs_get_page(inode, i, &page);
+ kaddr = nilfs_get_folio(inode, i, &folio);
if (IS_ERR(kaddr))
continue;
@@ -647,12 +636,12 @@ int nilfs_empty_dir(struct inode *inode)
}
de = nilfs_next_entry(de);
}
- unmap_and_put_page(page, kaddr);
+ folio_release_kmap(folio, kaddr);
}
return 1;
not_empty:
- unmap_and_put_page(page, kaddr);
+ folio_release_kmap(folio, kaddr);
return 0;
}
--
2.34.1
On Mon, Nov 27, 2023 at 11:30:20PM +0900, Ryusuke Konishi wrote:
> In a few directory operations, the call to nilfs_put_page() for a page
> obtained using nilfs_find_entry() or nilfs_dotdot() is hidden in
> nilfs_set_link() and nilfs_delete_entry(), making it difficult to track
> page release and preventing change of its call position.
>
> By moving nilfs_put_page() out of these functions, this makes the page
> get/put correspondence clearer and makes it easier to swap
> nilfs_put_page() calls (and kunmap calls within them) when modifying
> multiple directory entries simultaneously in nilfs_rename().
>
> Also, update comments for nilfs_set_link() and nilfs_delete_entry() to
> reflect changes in their behavior.
>
> To make nilfs_put_page() visible from namei.c, this moves its definition
> to nilfs.h and replaces existing equivalents to use it, but the exposure
> of that definition is temporary and will be removed on a later
> kmap -> kmap_local conversion.
>
> Signed-off-by: Ryusuke Konishi <[email protected]>
> Cc: Matthew Wilcox (Oracle) <[email protected]>
Ah; I see. This makes it more like ext2, so I approve!
Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
On Mon, Nov 27, 2023 at 11:30:21PM +0900, Ryusuke Konishi wrote:
> In nilfs_rename(), calls to nilfs_put_page() to release pages obtained
> with nilfs_find_entry() or nilfs_dotdot() are alternated in the normal
> path.
>
> When replacing the kernel memory mapping method from kmap to
> kmap_local_{page,folio}, this violates the constraint on the calling
> order of kunmap_local().
>
> Swap the order of nilfs_put_page calls where the kmap sections of
> multiple pages overlap so that they are nested, allowing direct
> replacement of nilfs_put_page() -> unmap_and_put_page().
>
> Without this reordering, that replacement will cause a kernel WARNING
> in kunmap_local_indexed() on architectures with high memory mapping.
>
> Signed-off-by: Ryusuke Konishi <[email protected]>
> Cc: Matthew Wilcox (Oracle) <[email protected]>
Thanks for catching this!
Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>