* [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename
@ 2019-12-18 20:09 Jaegeuk Kim
2019-12-18 20:09 ` [f2fs-dev] [PATCH 2/4] f2fs: don't put new_page twice in f2fs_rename Jaegeuk Kim
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Jaegeuk Kim @ 2019-12-18 20:09 UTC (permalink / raw)
To: linux-kernel, linux-f2fs-devel; +Cc: Jaegeuk Kim
If we hit an error during rename, we'll get two dentries in different
directories.
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/f2fs.h | 1 +
fs/f2fs/inline.c | 30 ++++++++++++++++++++++++++++--
fs/f2fs/namei.c | 36 +++++++++++++-----------------------
3 files changed, 42 insertions(+), 25 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index dbc20d33d0e1..8d64525743cb 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3490,6 +3490,7 @@ void f2fs_truncate_inline_inode(struct inode *inode,
int f2fs_read_inline_data(struct inode *inode, struct page *page);
int f2fs_convert_inline_page(struct dnode_of_data *dn, struct page *page);
int f2fs_convert_inline_inode(struct inode *inode);
+int f2fs_convert_inline_dir(struct inode *dir);
int f2fs_write_inline_data(struct inode *inode, struct page *page);
bool f2fs_recover_inline_data(struct inode *inode, struct page *npage);
struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir,
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 52f85ed07a15..f82c3d9cf333 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -530,7 +530,7 @@ static int f2fs_move_rehashed_dirents(struct inode *dir, struct page *ipage,
return err;
}
-static int f2fs_convert_inline_dir(struct inode *dir, struct page *ipage,
+static int do_convert_inline_dir(struct inode *dir, struct page *ipage,
void *inline_dentry)
{
if (!F2FS_I(dir)->i_dir_level)
@@ -539,6 +539,32 @@ static int f2fs_convert_inline_dir(struct inode *dir, struct page *ipage,
return f2fs_move_rehashed_dirents(dir, ipage, inline_dentry);
}
+int f2fs_convert_inline_dir(struct inode *dir)
+{
+ struct f2fs_sb_info *sbi = F2FS_I_SB(dir);
+ struct page *ipage;
+ void *inline_dentry = NULL;
+ int err;
+
+ if (!f2fs_has_inline_dentry(dir))
+ return 0;
+
+ f2fs_lock_op(sbi);
+
+ ipage = f2fs_get_node_page(sbi, dir->i_ino);
+ if (IS_ERR(ipage))
+ return PTR_ERR(ipage);
+
+ inline_dentry = inline_data_addr(dir, ipage);
+
+ err = do_convert_inline_dir(dir, ipage, inline_dentry);
+ if (!err)
+ f2fs_put_page(ipage, 1);
+
+ f2fs_unlock_op(sbi);
+ return err;
+}
+
int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
const struct qstr *orig_name,
struct inode *inode, nid_t ino, umode_t mode)
@@ -562,7 +588,7 @@ int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
bit_pos = f2fs_room_for_filename(d.bitmap, slots, d.max);
if (bit_pos >= d.max) {
- err = f2fs_convert_inline_dir(dir, ipage, inline_dentry);
+ err = do_convert_inline_dir(dir, ipage, inline_dentry);
if (err)
return err;
err = -EAGAIN;
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 5d9584281935..61615ab466c2 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -855,7 +855,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
struct f2fs_dir_entry *old_dir_entry = NULL;
struct f2fs_dir_entry *old_entry;
struct f2fs_dir_entry *new_entry;
- bool is_old_inline = f2fs_has_inline_dentry(old_dir);
int err;
if (unlikely(f2fs_cp_error(sbi)))
@@ -868,6 +867,19 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
F2FS_I(old_dentry->d_inode)->i_projid)))
return -EXDEV;
+ /*
+ * old entry and new entry can locate in the same inline
+ * dentry in inode, when attaching new entry in inline dentry,
+ * it could force inline dentry conversion, after that,
+ * old_entry and old_page will point to wrong address, in
+ * order to avoid this, let's do the check and update here.
+ */
+ if (old_dir == new_dir && !new_inode) {
+ err = f2fs_convert_inline_dir(old_dir);
+ if (err)
+ return err;
+ }
+
if (flags & RENAME_WHITEOUT) {
err = f2fs_create_whiteout(old_dir, &whiteout);
if (err)
@@ -954,28 +966,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
if (old_dir_entry)
f2fs_i_links_write(new_dir, true);
-
- /*
- * old entry and new entry can locate in the same inline
- * dentry in inode, when attaching new entry in inline dentry,
- * it could force inline dentry conversion, after that,
- * old_entry and old_page will point to wrong address, in
- * order to avoid this, let's do the check and update here.
- */
- if (is_old_inline && !f2fs_has_inline_dentry(old_dir)) {
- f2fs_put_page(old_page, 0);
- old_page = NULL;
-
- old_entry = f2fs_find_entry(old_dir,
- &old_dentry->d_name, &old_page);
- if (!old_entry) {
- err = -ENOENT;
- if (IS_ERR(old_page))
- err = PTR_ERR(old_page);
- f2fs_unlock_op(sbi);
- goto out_dir;
- }
- }
}
down_write(&F2FS_I(old_inode)->i_sem);
--
2.24.0.525.g8f36a354ae-goog
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [f2fs-dev] [PATCH 2/4] f2fs: don't put new_page twice in f2fs_rename
2019-12-18 20:09 [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename Jaegeuk Kim
@ 2019-12-18 20:09 ` Jaegeuk Kim
2019-12-28 6:50 ` Chao Yu
2019-12-18 20:09 ` [f2fs-dev] [PATCH 3/4] f2fs: declare nested quota_sem and remove unnecessary sems Jaegeuk Kim
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2019-12-18 20:09 UTC (permalink / raw)
To: linux-kernel, linux-f2fs-devel; +Cc: Jaegeuk Kim
In f2fs_rename(), new_page is gone after f2fs_set_link(), but it tries
to put again when whiteout is failed and jumped to put_out_dir.
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/namei.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 61615ab466c2..91e7b8360d2a 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -850,7 +850,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
struct inode *old_inode = d_inode(old_dentry);
struct inode *new_inode = d_inode(new_dentry);
struct inode *whiteout = NULL;
- struct page *old_dir_page;
+ struct page *old_dir_page = NULL;
struct page *old_page, *new_page = NULL;
struct f2fs_dir_entry *old_dir_entry = NULL;
struct f2fs_dir_entry *old_entry;
@@ -941,6 +941,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
goto put_out_dir;
f2fs_set_link(new_dir, new_entry, new_page, old_inode);
+ new_page = NULL;
new_inode->i_ctime = current_time(new_inode);
down_write(&F2FS_I(new_inode)->i_sem);
@@ -980,6 +981,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
f2fs_mark_inode_dirty_sync(old_inode, false);
f2fs_delete_entry(old_entry, old_page, old_dir, NULL);
+ old_page = NULL;
if (whiteout) {
set_inode_flag(whiteout, FI_INC_LINK);
@@ -1015,8 +1017,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
put_out_dir:
f2fs_unlock_op(sbi);
- if (new_page)
- f2fs_put_page(new_page, 0);
+ f2fs_put_page(new_page, 0);
out_dir:
if (old_dir_entry)
f2fs_put_page(old_dir_page, 0);
--
2.24.0.525.g8f36a354ae-goog
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [f2fs-dev] [PATCH 3/4] f2fs: declare nested quota_sem and remove unnecessary sems
2019-12-18 20:09 [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename Jaegeuk Kim
2019-12-18 20:09 ` [f2fs-dev] [PATCH 2/4] f2fs: don't put new_page twice in f2fs_rename Jaegeuk Kim
@ 2019-12-18 20:09 ` Jaegeuk Kim
2019-12-18 20:09 ` [f2fs-dev] [PATCH 4/4] f2fs: free sysfs kobject Jaegeuk Kim
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Jaegeuk Kim @ 2019-12-18 20:09 UTC (permalink / raw)
To: linux-kernel, linux-f2fs-devel; +Cc: Jaegeuk Kim
1.
f2fs_quota_sync
-> down_read(&sbi->quota_sem)
-> dquot_writeback_dquots
-> f2fs_dquot_commit
-> down_read(&sbi->quota_sem)
2.
f2fs_quota_sync
-> down_read(&sbi->quota_sem)
-> f2fs_write_data_pages
-> f2fs_write_single_data_page
-> down_write(&F2FS_I(inode)->i_sem)
f2fs_mkdir
-> f2fs_do_add_link
-> down_write(&F2FS_I(inode)->i_sem)
-> f2fs_init_inode_metadata
-> f2fs_new_node_page
-> dquot_alloc_inode
-> f2fs_dquot_mark_dquot_dirty
-> down_read(&sbi->quota_sem)
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/super.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 9e85894d12c9..e6fb4b52eb91 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2158,7 +2158,7 @@ static int f2fs_dquot_commit(struct dquot *dquot)
struct f2fs_sb_info *sbi = F2FS_SB(dquot->dq_sb);
int ret;
- down_read(&sbi->quota_sem);
+ down_read_nested(&sbi->quota_sem, SINGLE_DEPTH_NESTING);
ret = dquot_commit(dquot);
if (ret < 0)
set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
@@ -2182,13 +2182,10 @@ static int f2fs_dquot_acquire(struct dquot *dquot)
static int f2fs_dquot_release(struct dquot *dquot)
{
struct f2fs_sb_info *sbi = F2FS_SB(dquot->dq_sb);
- int ret;
+ int ret = dquot_release(dquot);
- down_read(&sbi->quota_sem);
- ret = dquot_release(dquot);
if (ret < 0)
set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
- up_read(&sbi->quota_sem);
return ret;
}
@@ -2196,29 +2193,22 @@ static int f2fs_dquot_mark_dquot_dirty(struct dquot *dquot)
{
struct super_block *sb = dquot->dq_sb;
struct f2fs_sb_info *sbi = F2FS_SB(sb);
- int ret;
-
- down_read(&sbi->quota_sem);
- ret = dquot_mark_dquot_dirty(dquot);
+ int ret = dquot_mark_dquot_dirty(dquot);
/* if we are using journalled quota */
if (is_journalled_quota(sbi))
set_sbi_flag(sbi, SBI_QUOTA_NEED_FLUSH);
- up_read(&sbi->quota_sem);
return ret;
}
static int f2fs_dquot_commit_info(struct super_block *sb, int type)
{
struct f2fs_sb_info *sbi = F2FS_SB(sb);
- int ret;
+ int ret = dquot_commit_info(sb, type);
- down_read(&sbi->quota_sem);
- ret = dquot_commit_info(sb, type);
if (ret < 0)
set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
- up_read(&sbi->quota_sem);
return ret;
}
--
2.24.0.525.g8f36a354ae-goog
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [f2fs-dev] [PATCH 4/4] f2fs: free sysfs kobject
2019-12-18 20:09 [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename Jaegeuk Kim
2019-12-18 20:09 ` [f2fs-dev] [PATCH 2/4] f2fs: don't put new_page twice in f2fs_rename Jaegeuk Kim
2019-12-18 20:09 ` [f2fs-dev] [PATCH 3/4] f2fs: declare nested quota_sem and remove unnecessary sems Jaegeuk Kim
@ 2019-12-18 20:09 ` Jaegeuk Kim
2019-12-28 10:09 ` Chao Yu
2019-12-28 6:32 ` [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename Chao Yu
2020-01-02 18:36 ` [f2fs-dev] [PATCH 1/4 v2] " Jaegeuk Kim
4 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2019-12-18 20:09 UTC (permalink / raw)
To: linux-kernel, linux-f2fs-devel; +Cc: Jaegeuk Kim
Detected kmemleak.
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/sysfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 70945ceb9c0c..5963316f391a 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -786,4 +786,5 @@ void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi)
remove_proc_entry(sbi->sb->s_id, f2fs_proc_root);
}
kobject_del(&sbi->s_kobj);
+ kobject_put(&sbi->s_kobj);
}
--
2.24.0.525.g8f36a354ae-goog
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename
2019-12-18 20:09 [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename Jaegeuk Kim
` (2 preceding siblings ...)
2019-12-18 20:09 ` [f2fs-dev] [PATCH 4/4] f2fs: free sysfs kobject Jaegeuk Kim
@ 2019-12-28 6:32 ` Chao Yu
2019-12-31 0:06 ` Jaegeuk Kim
2020-01-02 18:36 ` [f2fs-dev] [PATCH 1/4 v2] " Jaegeuk Kim
4 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2019-12-28 6:32 UTC (permalink / raw)
To: Jaegeuk Kim, linux-kernel, linux-f2fs-devel
On 2019/12/19 4:09, Jaegeuk Kim wrote:
> If we hit an error during rename, we'll get two dentries in different
> directories.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
> fs/f2fs/f2fs.h | 1 +
> fs/f2fs/inline.c | 30 ++++++++++++++++++++++++++++--
> fs/f2fs/namei.c | 36 +++++++++++++-----------------------
> 3 files changed, 42 insertions(+), 25 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index dbc20d33d0e1..8d64525743cb 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3490,6 +3490,7 @@ void f2fs_truncate_inline_inode(struct inode *inode,
> int f2fs_read_inline_data(struct inode *inode, struct page *page);
> int f2fs_convert_inline_page(struct dnode_of_data *dn, struct page *page);
> int f2fs_convert_inline_inode(struct inode *inode);
> +int f2fs_convert_inline_dir(struct inode *dir);
> int f2fs_write_inline_data(struct inode *inode, struct page *page);
> bool f2fs_recover_inline_data(struct inode *inode, struct page *npage);
> struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir,
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index 52f85ed07a15..f82c3d9cf333 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -530,7 +530,7 @@ static int f2fs_move_rehashed_dirents(struct inode *dir, struct page *ipage,
> return err;
> }
>
> -static int f2fs_convert_inline_dir(struct inode *dir, struct page *ipage,
> +static int do_convert_inline_dir(struct inode *dir, struct page *ipage,
> void *inline_dentry)
> {
> if (!F2FS_I(dir)->i_dir_level)
> @@ -539,6 +539,32 @@ static int f2fs_convert_inline_dir(struct inode *dir, struct page *ipage,
> return f2fs_move_rehashed_dirents(dir, ipage, inline_dentry);
> }
>
> +int f2fs_convert_inline_dir(struct inode *dir)
> +{
> + struct f2fs_sb_info *sbi = F2FS_I_SB(dir);
> + struct page *ipage;
> + void *inline_dentry = NULL;
> + int err;
> +
> + if (!f2fs_has_inline_dentry(dir))
> + return 0;
> +
> + f2fs_lock_op(sbi);
> +
> + ipage = f2fs_get_node_page(sbi, dir->i_ino);
> + if (IS_ERR(ipage))
> + return PTR_ERR(ipage);
> +
> + inline_dentry = inline_data_addr(dir, ipage);
> +
> + err = do_convert_inline_dir(dir, ipage, inline_dentry);
> + if (!err)
> + f2fs_put_page(ipage, 1);
> +
> + f2fs_unlock_op(sbi);
> + return err;
> +}
> +
> int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
> const struct qstr *orig_name,
> struct inode *inode, nid_t ino, umode_t mode)
> @@ -562,7 +588,7 @@ int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
>
> bit_pos = f2fs_room_for_filename(d.bitmap, slots, d.max);
> if (bit_pos >= d.max) {
> - err = f2fs_convert_inline_dir(dir, ipage, inline_dentry);
> + err = do_convert_inline_dir(dir, ipage, inline_dentry);
> if (err)
> return err;
> err = -EAGAIN;
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 5d9584281935..61615ab466c2 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -855,7 +855,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
> struct f2fs_dir_entry *old_dir_entry = NULL;
> struct f2fs_dir_entry *old_entry;
> struct f2fs_dir_entry *new_entry;
> - bool is_old_inline = f2fs_has_inline_dentry(old_dir);
> int err;
>
> if (unlikely(f2fs_cp_error(sbi)))
> @@ -868,6 +867,19 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
> F2FS_I(old_dentry->d_inode)->i_projid)))
> return -EXDEV;
>
> + /*
> + * old entry and new entry can locate in the same inline
> + * dentry in inode, when attaching new entry in inline dentry,
> + * it could force inline dentry conversion, after that,
> + * old_entry and old_page will point to wrong address, in
> + * order to avoid this, let's do the check and update here.
> + */
The comment is out-of-update here...
If there is enough room for the new dentry in old directory's inline space,
how about just keeping inline dir state for old directory?
---
fs/f2fs/dir.c | 14 ++++++++++++++
fs/f2fs/f2fs.h | 4 +++-
fs/f2fs/inline.c | 22 +++++++++++++++++-----
fs/f2fs/namei.c | 2 +-
4 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index c967cacf979e..b56f6060c1a6 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -578,6 +578,20 @@ int f2fs_room_for_filename(const void *bitmap, int slots, int max_slots)
goto next;
}
+bool f2fs_has_enough_room(struct inode *dir, struct page *ipage,
+ struct fscrypt_name *fname)
+{
+ struct f2fs_dentry_ptr d;
+ unsigned int bit_pos;
+ int slots = GET_DENTRY_SLOTS(fname_len(fname));
+
+ make_dentry_ptr_inline(dir, &d, inline_data_addr(dir, ipage));
+
+ bit_pos = f2fs_room_for_filename(d.bitmap, slots, d.max);
+
+ return bit_pos < d.max;
+}
+
void f2fs_update_dentry(nid_t ino, umode_t mode, struct f2fs_dentry_ptr *d,
const struct qstr *name, f2fs_hash_t name_hash,
unsigned int bit_pos)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 5d55cef66410..ce4ce33a40ce 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3120,6 +3120,8 @@ ino_t f2fs_inode_by_name(struct inode *dir, const struct qstr *qstr,
struct page **page);
void f2fs_set_link(struct inode *dir, struct f2fs_dir_entry *de,
struct page *page, struct inode *inode);
+bool f2fs_has_enough_room(struct inode *dir, struct page *ipage,
+ struct fscrypt_name *fname);
void f2fs_update_dentry(nid_t ino, umode_t mode, struct f2fs_dentry_ptr *d,
const struct qstr *name, f2fs_hash_t name_hash,
unsigned int bit_pos);
@@ -3662,7 +3664,7 @@ void f2fs_truncate_inline_inode(struct inode *inode,
int f2fs_read_inline_data(struct inode *inode, struct page *page);
int f2fs_convert_inline_page(struct dnode_of_data *dn, struct page *page);
int f2fs_convert_inline_inode(struct inode *inode);
-int f2fs_convert_inline_dir(struct inode *dir);
+int f2fs_try_convert_inline_dir(struct inode *dir, struct dentry *dentry);
int f2fs_write_inline_data(struct inode *inode, struct page *page);
bool f2fs_recover_inline_data(struct inode *inode, struct page *npage);
struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir,
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index f82c3d9cf333..4167e5408151 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -539,28 +539,40 @@ static int do_convert_inline_dir(struct inode *dir, struct page *ipage,
return f2fs_move_rehashed_dirents(dir, ipage, inline_dentry);
}
-int f2fs_convert_inline_dir(struct inode *dir)
+int f2fs_try_convert_inline_dir(struct inode *dir, struct dentry *dentry)
{
struct f2fs_sb_info *sbi = F2FS_I_SB(dir);
struct page *ipage;
+ struct fscrypt_name fname;
void *inline_dentry = NULL;
- int err;
+ int err = 0;
if (!f2fs_has_inline_dentry(dir))
return 0;
f2fs_lock_op(sbi);
+ err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &fname);
+ if (err)
+ goto out;
+
ipage = f2fs_get_node_page(sbi, dir->i_ino);
- if (IS_ERR(ipage))
- return PTR_ERR(ipage);
+ if (IS_ERR(ipage)) {
+ err = PTR_ERR(ipage);
+ goto out;
+ }
+
+ if (f2fs_has_enough_room(dir, ipage, &fname)) {
+ f2fs_put_page(ipage, 1);
+ goto out;
+ }
inline_dentry = inline_data_addr(dir, ipage);
err = do_convert_inline_dir(dir, ipage, inline_dentry);
if (!err)
f2fs_put_page(ipage, 1);
-
+out:
f2fs_unlock_op(sbi);
return err;
}
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 1daa54c8d8d8..856f0f984549 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -927,7 +927,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
* order to avoid this, let's do the check and update here.
*/
if (old_dir == new_dir && !new_inode) {
- err = f2fs_convert_inline_dir(old_dir);
+ err = f2fs_try_convert_inline_dir(old_dir, new_dentry);
if (err)
return err;
}
--
2.18.0.rc1
Thanks,
> + if (old_dir == new_dir && !new_inode) {
> + err = f2fs_convert_inline_dir(old_dir);
> + if (err)
> + return err;
> + }
> +
> if (flags & RENAME_WHITEOUT) {
> err = f2fs_create_whiteout(old_dir, &whiteout);
> if (err)
> @@ -954,28 +966,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
>
> if (old_dir_entry)
> f2fs_i_links_write(new_dir, true);
> -
> - /*
> - * old entry and new entry can locate in the same inline
> - * dentry in inode, when attaching new entry in inline dentry,
> - * it could force inline dentry conversion, after that,
> - * old_entry and old_page will point to wrong address, in
> - * order to avoid this, let's do the check and update here.
> - */
> - if (is_old_inline && !f2fs_has_inline_dentry(old_dir)) {
> - f2fs_put_page(old_page, 0);
> - old_page = NULL;
> -
> - old_entry = f2fs_find_entry(old_dir,
> - &old_dentry->d_name, &old_page);
> - if (!old_entry) {
> - err = -ENOENT;
> - if (IS_ERR(old_page))
> - err = PTR_ERR(old_page);
> - f2fs_unlock_op(sbi);
> - goto out_dir;
> - }
> - }
> }
>
> down_write(&F2FS_I(old_inode)->i_sem);
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [f2fs-dev] [PATCH 2/4] f2fs: don't put new_page twice in f2fs_rename
2019-12-18 20:09 ` [f2fs-dev] [PATCH 2/4] f2fs: don't put new_page twice in f2fs_rename Jaegeuk Kim
@ 2019-12-28 6:50 ` Chao Yu
0 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2019-12-28 6:50 UTC (permalink / raw)
To: Jaegeuk Kim, linux-kernel, linux-f2fs-devel
On 2019/12/19 4:09, Jaegeuk Kim wrote:
> In f2fs_rename(), new_page is gone after f2fs_set_link(), but it tries
> to put again when whiteout is failed and jumped to put_out_dir.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Thanks,
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [f2fs-dev] [PATCH 4/4] f2fs: free sysfs kobject
2019-12-18 20:09 ` [f2fs-dev] [PATCH 4/4] f2fs: free sysfs kobject Jaegeuk Kim
@ 2019-12-28 10:09 ` Chao Yu
0 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2019-12-28 10:09 UTC (permalink / raw)
To: Jaegeuk Kim, linux-kernel, linux-f2fs-devel
On 2019/12/19 4:09, Jaegeuk Kim wrote:
> Detected kmemleak.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Thanks,
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename
2019-12-28 6:32 ` [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename Chao Yu
@ 2019-12-31 0:06 ` Jaegeuk Kim
2019-12-31 1:22 ` Chao Yu
0 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2019-12-31 0:06 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 12/28, Chao Yu wrote:
> On 2019/12/19 4:09, Jaegeuk Kim wrote:
> > If we hit an error during rename, we'll get two dentries in different
> > directories.
> >
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> > fs/f2fs/f2fs.h | 1 +
> > fs/f2fs/inline.c | 30 ++++++++++++++++++++++++++++--
> > fs/f2fs/namei.c | 36 +++++++++++++-----------------------
> > 3 files changed, 42 insertions(+), 25 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index dbc20d33d0e1..8d64525743cb 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -3490,6 +3490,7 @@ void f2fs_truncate_inline_inode(struct inode *inode,
> > int f2fs_read_inline_data(struct inode *inode, struct page *page);
> > int f2fs_convert_inline_page(struct dnode_of_data *dn, struct page *page);
> > int f2fs_convert_inline_inode(struct inode *inode);
> > +int f2fs_convert_inline_dir(struct inode *dir);
> > int f2fs_write_inline_data(struct inode *inode, struct page *page);
> > bool f2fs_recover_inline_data(struct inode *inode, struct page *npage);
> > struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir,
> > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> > index 52f85ed07a15..f82c3d9cf333 100644
> > --- a/fs/f2fs/inline.c
> > +++ b/fs/f2fs/inline.c
> > @@ -530,7 +530,7 @@ static int f2fs_move_rehashed_dirents(struct inode *dir, struct page *ipage,
> > return err;
> > }
> >
> > -static int f2fs_convert_inline_dir(struct inode *dir, struct page *ipage,
> > +static int do_convert_inline_dir(struct inode *dir, struct page *ipage,
> > void *inline_dentry)
> > {
> > if (!F2FS_I(dir)->i_dir_level)
> > @@ -539,6 +539,32 @@ static int f2fs_convert_inline_dir(struct inode *dir, struct page *ipage,
> > return f2fs_move_rehashed_dirents(dir, ipage, inline_dentry);
> > }
> >
> > +int f2fs_convert_inline_dir(struct inode *dir)
> > +{
> > + struct f2fs_sb_info *sbi = F2FS_I_SB(dir);
> > + struct page *ipage;
> > + void *inline_dentry = NULL;
> > + int err;
> > +
> > + if (!f2fs_has_inline_dentry(dir))
> > + return 0;
> > +
> > + f2fs_lock_op(sbi);
> > +
> > + ipage = f2fs_get_node_page(sbi, dir->i_ino);
> > + if (IS_ERR(ipage))
> > + return PTR_ERR(ipage);
> > +
> > + inline_dentry = inline_data_addr(dir, ipage);
> > +
> > + err = do_convert_inline_dir(dir, ipage, inline_dentry);
> > + if (!err)
> > + f2fs_put_page(ipage, 1);
> > +
> > + f2fs_unlock_op(sbi);
> > + return err;
> > +}
> > +
> > int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
> > const struct qstr *orig_name,
> > struct inode *inode, nid_t ino, umode_t mode)
> > @@ -562,7 +588,7 @@ int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
> >
> > bit_pos = f2fs_room_for_filename(d.bitmap, slots, d.max);
> > if (bit_pos >= d.max) {
> > - err = f2fs_convert_inline_dir(dir, ipage, inline_dentry);
> > + err = do_convert_inline_dir(dir, ipage, inline_dentry);
> > if (err)
> > return err;
> > err = -EAGAIN;
> > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> > index 5d9584281935..61615ab466c2 100644
> > --- a/fs/f2fs/namei.c
> > +++ b/fs/f2fs/namei.c
> > @@ -855,7 +855,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
> > struct f2fs_dir_entry *old_dir_entry = NULL;
> > struct f2fs_dir_entry *old_entry;
> > struct f2fs_dir_entry *new_entry;
> > - bool is_old_inline = f2fs_has_inline_dentry(old_dir);
> > int err;
> >
> > if (unlikely(f2fs_cp_error(sbi)))
> > @@ -868,6 +867,19 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
> > F2FS_I(old_dentry->d_inode)->i_projid)))
> > return -EXDEV;
> >
> > + /*
> > + * old entry and new entry can locate in the same inline
> > + * dentry in inode, when attaching new entry in inline dentry,
> > + * it could force inline dentry conversion, after that,
> > + * old_entry and old_page will point to wrong address, in
> > + * order to avoid this, let's do the check and update here.
> > + */
>
> The comment is out-of-update here...
>
> If there is enough room for the new dentry in old directory's inline space,
> how about just keeping inline dir state for old directory?
It'd work, but how can we deal with race conditions?
>
>
> ---
> fs/f2fs/dir.c | 14 ++++++++++++++
> fs/f2fs/f2fs.h | 4 +++-
> fs/f2fs/inline.c | 22 +++++++++++++++++-----
> fs/f2fs/namei.c | 2 +-
> 4 files changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index c967cacf979e..b56f6060c1a6 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -578,6 +578,20 @@ int f2fs_room_for_filename(const void *bitmap, int slots, int max_slots)
> goto next;
> }
>
> +bool f2fs_has_enough_room(struct inode *dir, struct page *ipage,
> + struct fscrypt_name *fname)
> +{
> + struct f2fs_dentry_ptr d;
> + unsigned int bit_pos;
> + int slots = GET_DENTRY_SLOTS(fname_len(fname));
> +
> + make_dentry_ptr_inline(dir, &d, inline_data_addr(dir, ipage));
> +
> + bit_pos = f2fs_room_for_filename(d.bitmap, slots, d.max);
> +
> + return bit_pos < d.max;
> +}
> +
> void f2fs_update_dentry(nid_t ino, umode_t mode, struct f2fs_dentry_ptr *d,
> const struct qstr *name, f2fs_hash_t name_hash,
> unsigned int bit_pos)
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 5d55cef66410..ce4ce33a40ce 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3120,6 +3120,8 @@ ino_t f2fs_inode_by_name(struct inode *dir, const struct qstr *qstr,
> struct page **page);
> void f2fs_set_link(struct inode *dir, struct f2fs_dir_entry *de,
> struct page *page, struct inode *inode);
> +bool f2fs_has_enough_room(struct inode *dir, struct page *ipage,
> + struct fscrypt_name *fname);
> void f2fs_update_dentry(nid_t ino, umode_t mode, struct f2fs_dentry_ptr *d,
> const struct qstr *name, f2fs_hash_t name_hash,
> unsigned int bit_pos);
> @@ -3662,7 +3664,7 @@ void f2fs_truncate_inline_inode(struct inode *inode,
> int f2fs_read_inline_data(struct inode *inode, struct page *page);
> int f2fs_convert_inline_page(struct dnode_of_data *dn, struct page *page);
> int f2fs_convert_inline_inode(struct inode *inode);
> -int f2fs_convert_inline_dir(struct inode *dir);
> +int f2fs_try_convert_inline_dir(struct inode *dir, struct dentry *dentry);
> int f2fs_write_inline_data(struct inode *inode, struct page *page);
> bool f2fs_recover_inline_data(struct inode *inode, struct page *npage);
> struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir,
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index f82c3d9cf333..4167e5408151 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -539,28 +539,40 @@ static int do_convert_inline_dir(struct inode *dir, struct page *ipage,
> return f2fs_move_rehashed_dirents(dir, ipage, inline_dentry);
> }
>
> -int f2fs_convert_inline_dir(struct inode *dir)
> +int f2fs_try_convert_inline_dir(struct inode *dir, struct dentry *dentry)
> {
> struct f2fs_sb_info *sbi = F2FS_I_SB(dir);
> struct page *ipage;
> + struct fscrypt_name fname;
> void *inline_dentry = NULL;
> - int err;
> + int err = 0;
>
> if (!f2fs_has_inline_dentry(dir))
> return 0;
>
> f2fs_lock_op(sbi);
>
> + err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &fname);
> + if (err)
> + goto out;
> +
> ipage = f2fs_get_node_page(sbi, dir->i_ino);
> - if (IS_ERR(ipage))
> - return PTR_ERR(ipage);
> + if (IS_ERR(ipage)) {
> + err = PTR_ERR(ipage);
> + goto out;
> + }
> +
> + if (f2fs_has_enough_room(dir, ipage, &fname)) {
> + f2fs_put_page(ipage, 1);
> + goto out;
> + }
>
> inline_dentry = inline_data_addr(dir, ipage);
>
> err = do_convert_inline_dir(dir, ipage, inline_dentry);
> if (!err)
> f2fs_put_page(ipage, 1);
> -
> +out:
> f2fs_unlock_op(sbi);
> return err;
> }
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 1daa54c8d8d8..856f0f984549 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -927,7 +927,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
> * order to avoid this, let's do the check and update here.
> */
> if (old_dir == new_dir && !new_inode) {
> - err = f2fs_convert_inline_dir(old_dir);
> + err = f2fs_try_convert_inline_dir(old_dir, new_dentry);
> if (err)
> return err;
> }
> --
> 2.18.0.rc1
>
> Thanks,
>
> > + if (old_dir == new_dir && !new_inode) {
> > + err = f2fs_convert_inline_dir(old_dir);
> > + if (err)
> > + return err;
> > + }
> > +
> > if (flags & RENAME_WHITEOUT) {
> > err = f2fs_create_whiteout(old_dir, &whiteout);
> > if (err)
> > @@ -954,28 +966,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
> >
> > if (old_dir_entry)
> > f2fs_i_links_write(new_dir, true);
> > -
> > - /*
> > - * old entry and new entry can locate in the same inline
> > - * dentry in inode, when attaching new entry in inline dentry,
> > - * it could force inline dentry conversion, after that,
> > - * old_entry and old_page will point to wrong address, in
> > - * order to avoid this, let's do the check and update here.
> > - */
> > - if (is_old_inline && !f2fs_has_inline_dentry(old_dir)) {
> > - f2fs_put_page(old_page, 0);
> > - old_page = NULL;
> > -
> > - old_entry = f2fs_find_entry(old_dir,
> > - &old_dentry->d_name, &old_page);
> > - if (!old_entry) {
> > - err = -ENOENT;
> > - if (IS_ERR(old_page))
> > - err = PTR_ERR(old_page);
> > - f2fs_unlock_op(sbi);
> > - goto out_dir;
> > - }
> > - }
> > }
> >
> > down_write(&F2FS_I(old_inode)->i_sem);
> >
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename
2019-12-31 0:06 ` Jaegeuk Kim
@ 2019-12-31 1:22 ` Chao Yu
0 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2019-12-31 1:22 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
On 2019/12/31 8:06, Jaegeuk Kim wrote:
> On 12/28, Chao Yu wrote:
>> On 2019/12/19 4:09, Jaegeuk Kim wrote:
>>> If we hit an error during rename, we'll get two dentries in different
>>> directories.
>>>
>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>> ---
>>> fs/f2fs/f2fs.h | 1 +
>>> fs/f2fs/inline.c | 30 ++++++++++++++++++++++++++++--
>>> fs/f2fs/namei.c | 36 +++++++++++++-----------------------
>>> 3 files changed, 42 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index dbc20d33d0e1..8d64525743cb 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -3490,6 +3490,7 @@ void f2fs_truncate_inline_inode(struct inode *inode,
>>> int f2fs_read_inline_data(struct inode *inode, struct page *page);
>>> int f2fs_convert_inline_page(struct dnode_of_data *dn, struct page *page);
>>> int f2fs_convert_inline_inode(struct inode *inode);
>>> +int f2fs_convert_inline_dir(struct inode *dir);
>>> int f2fs_write_inline_data(struct inode *inode, struct page *page);
>>> bool f2fs_recover_inline_data(struct inode *inode, struct page *npage);
>>> struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir,
>>> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
>>> index 52f85ed07a15..f82c3d9cf333 100644
>>> --- a/fs/f2fs/inline.c
>>> +++ b/fs/f2fs/inline.c
>>> @@ -530,7 +530,7 @@ static int f2fs_move_rehashed_dirents(struct inode *dir, struct page *ipage,
>>> return err;
>>> }
>>>
>>> -static int f2fs_convert_inline_dir(struct inode *dir, struct page *ipage,
>>> +static int do_convert_inline_dir(struct inode *dir, struct page *ipage,
>>> void *inline_dentry)
>>> {
>>> if (!F2FS_I(dir)->i_dir_level)
>>> @@ -539,6 +539,32 @@ static int f2fs_convert_inline_dir(struct inode *dir, struct page *ipage,
>>> return f2fs_move_rehashed_dirents(dir, ipage, inline_dentry);
>>> }
>>>
>>> +int f2fs_convert_inline_dir(struct inode *dir)
>>> +{
>>> + struct f2fs_sb_info *sbi = F2FS_I_SB(dir);
>>> + struct page *ipage;
>>> + void *inline_dentry = NULL;
>>> + int err;
>>> +
>>> + if (!f2fs_has_inline_dentry(dir))
>>> + return 0;
>>> +
>>> + f2fs_lock_op(sbi);
>>> +
>>> + ipage = f2fs_get_node_page(sbi, dir->i_ino);
>>> + if (IS_ERR(ipage))
>>> + return PTR_ERR(ipage);
>>> +
>>> + inline_dentry = inline_data_addr(dir, ipage);
>>> +
>>> + err = do_convert_inline_dir(dir, ipage, inline_dentry);
>>> + if (!err)
>>> + f2fs_put_page(ipage, 1);
>>> +
>>> + f2fs_unlock_op(sbi);
>>> + return err;
>>> +}
>>> +
>>> int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
>>> const struct qstr *orig_name,
>>> struct inode *inode, nid_t ino, umode_t mode)
>>> @@ -562,7 +588,7 @@ int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
>>>
>>> bit_pos = f2fs_room_for_filename(d.bitmap, slots, d.max);
>>> if (bit_pos >= d.max) {
>>> - err = f2fs_convert_inline_dir(dir, ipage, inline_dentry);
>>> + err = do_convert_inline_dir(dir, ipage, inline_dentry);
>>> if (err)
>>> return err;
>>> err = -EAGAIN;
>>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
>>> index 5d9584281935..61615ab466c2 100644
>>> --- a/fs/f2fs/namei.c
>>> +++ b/fs/f2fs/namei.c
>>> @@ -855,7 +855,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
>>> struct f2fs_dir_entry *old_dir_entry = NULL;
>>> struct f2fs_dir_entry *old_entry;
>>> struct f2fs_dir_entry *new_entry;
>>> - bool is_old_inline = f2fs_has_inline_dentry(old_dir);
>>> int err;
>>>
>>> if (unlikely(f2fs_cp_error(sbi)))
>>> @@ -868,6 +867,19 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
>>> F2FS_I(old_dentry->d_inode)->i_projid)))
>>> return -EXDEV;
>>>
>>> + /*
>>> + * old entry and new entry can locate in the same inline
>>> + * dentry in inode, when attaching new entry in inline dentry,
>>> + * it could force inline dentry conversion, after that,
>>> + * old_entry and old_page will point to wrong address, in
>>> + * order to avoid this, let's do the check and update here.
>>> + */
>>
>> The comment is out-of-update here...
>>
>> If there is enough room for the new dentry in old directory's inline space,
>> how about just keeping inline dir state for old directory?
>
> It'd work, but how can we deal with race conditions?
In rename() syscall, both src/dst's parent inode lock should has been hold to
prevent race condition.
Thanks,
>
>>
>>
>> ---
>> fs/f2fs/dir.c | 14 ++++++++++++++
>> fs/f2fs/f2fs.h | 4 +++-
>> fs/f2fs/inline.c | 22 +++++++++++++++++-----
>> fs/f2fs/namei.c | 2 +-
>> 4 files changed, 35 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
>> index c967cacf979e..b56f6060c1a6 100644
>> --- a/fs/f2fs/dir.c
>> +++ b/fs/f2fs/dir.c
>> @@ -578,6 +578,20 @@ int f2fs_room_for_filename(const void *bitmap, int slots, int max_slots)
>> goto next;
>> }
>>
>> +bool f2fs_has_enough_room(struct inode *dir, struct page *ipage,
>> + struct fscrypt_name *fname)
>> +{
>> + struct f2fs_dentry_ptr d;
>> + unsigned int bit_pos;
>> + int slots = GET_DENTRY_SLOTS(fname_len(fname));
>> +
>> + make_dentry_ptr_inline(dir, &d, inline_data_addr(dir, ipage));
>> +
>> + bit_pos = f2fs_room_for_filename(d.bitmap, slots, d.max);
>> +
>> + return bit_pos < d.max;
>> +}
>> +
>> void f2fs_update_dentry(nid_t ino, umode_t mode, struct f2fs_dentry_ptr *d,
>> const struct qstr *name, f2fs_hash_t name_hash,
>> unsigned int bit_pos)
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 5d55cef66410..ce4ce33a40ce 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -3120,6 +3120,8 @@ ino_t f2fs_inode_by_name(struct inode *dir, const struct qstr *qstr,
>> struct page **page);
>> void f2fs_set_link(struct inode *dir, struct f2fs_dir_entry *de,
>> struct page *page, struct inode *inode);
>> +bool f2fs_has_enough_room(struct inode *dir, struct page *ipage,
>> + struct fscrypt_name *fname);
>> void f2fs_update_dentry(nid_t ino, umode_t mode, struct f2fs_dentry_ptr *d,
>> const struct qstr *name, f2fs_hash_t name_hash,
>> unsigned int bit_pos);
>> @@ -3662,7 +3664,7 @@ void f2fs_truncate_inline_inode(struct inode *inode,
>> int f2fs_read_inline_data(struct inode *inode, struct page *page);
>> int f2fs_convert_inline_page(struct dnode_of_data *dn, struct page *page);
>> int f2fs_convert_inline_inode(struct inode *inode);
>> -int f2fs_convert_inline_dir(struct inode *dir);
>> +int f2fs_try_convert_inline_dir(struct inode *dir, struct dentry *dentry);
>> int f2fs_write_inline_data(struct inode *inode, struct page *page);
>> bool f2fs_recover_inline_data(struct inode *inode, struct page *npage);
>> struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir,
>> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
>> index f82c3d9cf333..4167e5408151 100644
>> --- a/fs/f2fs/inline.c
>> +++ b/fs/f2fs/inline.c
>> @@ -539,28 +539,40 @@ static int do_convert_inline_dir(struct inode *dir, struct page *ipage,
>> return f2fs_move_rehashed_dirents(dir, ipage, inline_dentry);
>> }
>>
>> -int f2fs_convert_inline_dir(struct inode *dir)
>> +int f2fs_try_convert_inline_dir(struct inode *dir, struct dentry *dentry)
>> {
>> struct f2fs_sb_info *sbi = F2FS_I_SB(dir);
>> struct page *ipage;
>> + struct fscrypt_name fname;
>> void *inline_dentry = NULL;
>> - int err;
>> + int err = 0;
>>
>> if (!f2fs_has_inline_dentry(dir))
>> return 0;
>>
>> f2fs_lock_op(sbi);
>>
>> + err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &fname);
>> + if (err)
>> + goto out;
>> +
>> ipage = f2fs_get_node_page(sbi, dir->i_ino);
>> - if (IS_ERR(ipage))
>> - return PTR_ERR(ipage);
>> + if (IS_ERR(ipage)) {
>> + err = PTR_ERR(ipage);
>> + goto out;
>> + }
>> +
>> + if (f2fs_has_enough_room(dir, ipage, &fname)) {
>> + f2fs_put_page(ipage, 1);
>> + goto out;
>> + }
>>
>> inline_dentry = inline_data_addr(dir, ipage);
>>
>> err = do_convert_inline_dir(dir, ipage, inline_dentry);
>> if (!err)
>> f2fs_put_page(ipage, 1);
>> -
>> +out:
>> f2fs_unlock_op(sbi);
>> return err;
>> }
>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
>> index 1daa54c8d8d8..856f0f984549 100644
>> --- a/fs/f2fs/namei.c
>> +++ b/fs/f2fs/namei.c
>> @@ -927,7 +927,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
>> * order to avoid this, let's do the check and update here.
>> */
>> if (old_dir == new_dir && !new_inode) {
>> - err = f2fs_convert_inline_dir(old_dir);
>> + err = f2fs_try_convert_inline_dir(old_dir, new_dentry);
>> if (err)
>> return err;
>> }
>> --
>> 2.18.0.rc1
>>
>> Thanks,
>>
>>> + if (old_dir == new_dir && !new_inode) {
>>> + err = f2fs_convert_inline_dir(old_dir);
>>> + if (err)
>>> + return err;
>>> + }
>>> +
>>> if (flags & RENAME_WHITEOUT) {
>>> err = f2fs_create_whiteout(old_dir, &whiteout);
>>> if (err)
>>> @@ -954,28 +966,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
>>>
>>> if (old_dir_entry)
>>> f2fs_i_links_write(new_dir, true);
>>> -
>>> - /*
>>> - * old entry and new entry can locate in the same inline
>>> - * dentry in inode, when attaching new entry in inline dentry,
>>> - * it could force inline dentry conversion, after that,
>>> - * old_entry and old_page will point to wrong address, in
>>> - * order to avoid this, let's do the check and update here.
>>> - */
>>> - if (is_old_inline && !f2fs_has_inline_dentry(old_dir)) {
>>> - f2fs_put_page(old_page, 0);
>>> - old_page = NULL;
>>> -
>>> - old_entry = f2fs_find_entry(old_dir,
>>> - &old_dentry->d_name, &old_page);
>>> - if (!old_entry) {
>>> - err = -ENOENT;
>>> - if (IS_ERR(old_page))
>>> - err = PTR_ERR(old_page);
>>> - f2fs_unlock_op(sbi);
>>> - goto out_dir;
>>> - }
>>> - }
>>> }
>>>
>>> down_write(&F2FS_I(old_inode)->i_sem);
>>>
> .
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [f2fs-dev] [PATCH 1/4 v2] f2fs: convert inline_dir early before starting rename
2019-12-18 20:09 [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename Jaegeuk Kim
` (3 preceding siblings ...)
2019-12-28 6:32 ` [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename Chao Yu
@ 2020-01-02 18:36 ` Jaegeuk Kim
4 siblings, 0 replies; 10+ messages in thread
From: Jaegeuk Kim @ 2020-01-02 18:36 UTC (permalink / raw)
To: linux-kernel, linux-f2fs-devel
If we hit an error during rename, we'll get two dentries in different
directories.
Chao adds to check the room in inline_dir which can avoid needless
inversion. This should be done by inode_lock(&old_dir).
Signed-off-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/dir.c | 14 ++++++++++++++
fs/f2fs/f2fs.h | 3 +++
fs/f2fs/inline.c | 42 ++++++++++++++++++++++++++++++++++++++++--
fs/f2fs/namei.c | 37 ++++++++++++++-----------------------
4 files changed, 71 insertions(+), 25 deletions(-)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index c967cacf979e..b56f6060c1a6 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -578,6 +578,20 @@ int f2fs_room_for_filename(const void *bitmap, int slots, int max_slots)
goto next;
}
+bool f2fs_has_enough_room(struct inode *dir, struct page *ipage,
+ struct fscrypt_name *fname)
+{
+ struct f2fs_dentry_ptr d;
+ unsigned int bit_pos;
+ int slots = GET_DENTRY_SLOTS(fname_len(fname));
+
+ make_dentry_ptr_inline(dir, &d, inline_data_addr(dir, ipage));
+
+ bit_pos = f2fs_room_for_filename(d.bitmap, slots, d.max);
+
+ return bit_pos < d.max;
+}
+
void f2fs_update_dentry(nid_t ino, umode_t mode, struct f2fs_dentry_ptr *d,
const struct qstr *name, f2fs_hash_t name_hash,
unsigned int bit_pos)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 740e4f11bd1f..0164c8279037 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3121,6 +3121,8 @@ ino_t f2fs_inode_by_name(struct inode *dir, const struct qstr *qstr,
struct page **page);
void f2fs_set_link(struct inode *dir, struct f2fs_dir_entry *de,
struct page *page, struct inode *inode);
+bool f2fs_has_enough_room(struct inode *dir, struct page *ipage,
+ struct fscrypt_name *fname);
void f2fs_update_dentry(nid_t ino, umode_t mode, struct f2fs_dentry_ptr *d,
const struct qstr *name, f2fs_hash_t name_hash,
unsigned int bit_pos);
@@ -3663,6 +3665,7 @@ void f2fs_truncate_inline_inode(struct inode *inode,
int f2fs_read_inline_data(struct inode *inode, struct page *page);
int f2fs_convert_inline_page(struct dnode_of_data *dn, struct page *page);
int f2fs_convert_inline_inode(struct inode *inode);
+int f2fs_try_convert_inline_dir(struct inode *dir, struct dentry *dentry);
int f2fs_write_inline_data(struct inode *inode, struct page *page);
bool f2fs_recover_inline_data(struct inode *inode, struct page *npage);
struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir,
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 52f85ed07a15..4167e5408151 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -530,7 +530,7 @@ static int f2fs_move_rehashed_dirents(struct inode *dir, struct page *ipage,
return err;
}
-static int f2fs_convert_inline_dir(struct inode *dir, struct page *ipage,
+static int do_convert_inline_dir(struct inode *dir, struct page *ipage,
void *inline_dentry)
{
if (!F2FS_I(dir)->i_dir_level)
@@ -539,6 +539,44 @@ static int f2fs_convert_inline_dir(struct inode *dir, struct page *ipage,
return f2fs_move_rehashed_dirents(dir, ipage, inline_dentry);
}
+int f2fs_try_convert_inline_dir(struct inode *dir, struct dentry *dentry)
+{
+ struct f2fs_sb_info *sbi = F2FS_I_SB(dir);
+ struct page *ipage;
+ struct fscrypt_name fname;
+ void *inline_dentry = NULL;
+ int err = 0;
+
+ if (!f2fs_has_inline_dentry(dir))
+ return 0;
+
+ f2fs_lock_op(sbi);
+
+ err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &fname);
+ if (err)
+ goto out;
+
+ ipage = f2fs_get_node_page(sbi, dir->i_ino);
+ if (IS_ERR(ipage)) {
+ err = PTR_ERR(ipage);
+ goto out;
+ }
+
+ if (f2fs_has_enough_room(dir, ipage, &fname)) {
+ f2fs_put_page(ipage, 1);
+ goto out;
+ }
+
+ inline_dentry = inline_data_addr(dir, ipage);
+
+ err = do_convert_inline_dir(dir, ipage, inline_dentry);
+ if (!err)
+ f2fs_put_page(ipage, 1);
+out:
+ f2fs_unlock_op(sbi);
+ return err;
+}
+
int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
const struct qstr *orig_name,
struct inode *inode, nid_t ino, umode_t mode)
@@ -562,7 +600,7 @@ int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
bit_pos = f2fs_room_for_filename(d.bitmap, slots, d.max);
if (bit_pos >= d.max) {
- err = f2fs_convert_inline_dir(dir, ipage, inline_dentry);
+ err = do_convert_inline_dir(dir, ipage, inline_dentry);
if (err)
return err;
err = -EAGAIN;
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index d479b91f9ca0..2aa035422c0f 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -906,7 +906,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
struct f2fs_dir_entry *old_dir_entry = NULL;
struct f2fs_dir_entry *old_entry;
struct f2fs_dir_entry *new_entry;
- bool is_old_inline = f2fs_has_inline_dentry(old_dir);
int err;
if (unlikely(f2fs_cp_error(sbi)))
@@ -919,6 +918,20 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
F2FS_I(old_dentry->d_inode)->i_projid)))
return -EXDEV;
+ /*
+ * If new_inode is null, the below renaming flow will
+ * add a link in old_dir which can conver inline_dir.
+ * After then, if we failed to get the entry due to other
+ * reasons like ENOMEM, we had to remove the new entry.
+ * Instead of adding such the error handling routine, let's
+ * simply convert first here.
+ */
+ if (old_dir == new_dir && !new_inode) {
+ err = f2fs_try_convert_inline_dir(old_dir, new_dentry);
+ if (err)
+ return err;
+ }
+
if (flags & RENAME_WHITEOUT) {
err = f2fs_create_whiteout(old_dir, &whiteout);
if (err)
@@ -1006,28 +1019,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
if (old_dir_entry)
f2fs_i_links_write(new_dir, true);
-
- /*
- * old entry and new entry can locate in the same inline
- * dentry in inode, when attaching new entry in inline dentry,
- * it could force inline dentry conversion, after that,
- * old_entry and old_page will point to wrong address, in
- * order to avoid this, let's do the check and update here.
- */
- if (is_old_inline && !f2fs_has_inline_dentry(old_dir)) {
- f2fs_put_page(old_page, 0);
- old_page = NULL;
-
- old_entry = f2fs_find_entry(old_dir,
- &old_dentry->d_name, &old_page);
- if (!old_entry) {
- err = -ENOENT;
- if (IS_ERR(old_page))
- err = PTR_ERR(old_page);
- f2fs_unlock_op(sbi);
- goto out_dir;
- }
- }
}
down_write(&F2FS_I(old_inode)->i_sem);
--
2.24.0.525.g8f36a354ae-goog
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-01-02 18:36 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-18 20:09 [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename Jaegeuk Kim
2019-12-18 20:09 ` [f2fs-dev] [PATCH 2/4] f2fs: don't put new_page twice in f2fs_rename Jaegeuk Kim
2019-12-28 6:50 ` Chao Yu
2019-12-18 20:09 ` [f2fs-dev] [PATCH 3/4] f2fs: declare nested quota_sem and remove unnecessary sems Jaegeuk Kim
2019-12-18 20:09 ` [f2fs-dev] [PATCH 4/4] f2fs: free sysfs kobject Jaegeuk Kim
2019-12-28 10:09 ` Chao Yu
2019-12-28 6:32 ` [f2fs-dev] [PATCH 1/4] f2fs: convert inline_dir early before starting rename Chao Yu
2019-12-31 0:06 ` Jaegeuk Kim
2019-12-31 1:22 ` Chao Yu
2020-01-02 18:36 ` [f2fs-dev] [PATCH 1/4 v2] " Jaegeuk Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).