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 <konishi.ryusuke@gmail.com>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
---
fs/nilfs2/dir.c | 11 +----------
fs/nilfs2/namei.c | 13 +++++++------
fs/nilfs2/nilfs.h | 6 ++++++
3 files changed, 14 insertions(+), 16 deletions(-)
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 <konishi.ryusuke@gmail.com>
> Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Ah; I see. This makes it more like ext2, so I approve!
Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
@@ -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;
}
@@ -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;
@@ -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);