mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-09-05 02:19:51 +00:00
ovl: initialize OVL_UPPERDATA in ovl_lookup()
Currently ovl_get_inode() initializes OVL_UPPERDATA flag and for that it has to call ovl_check_metacopy_xattr() and check if metacopy xattr is present or not. yangerkun reported sometimes underlying filesystem might return -EIO and in that case error handling path does not cleanup properly leading to various warnings. Run generic/461 with ext4 upper/lower layer sometimes may trigger the bug as below(linux 4.19): [ 551.001349] overlayfs: failed to get metacopy (-5) [ 551.003464] overlayfs: failed to get inode (-5) [ 551.004243] overlayfs: cleanup of 'd44/fd51' failed (-5) [ 551.004941] overlayfs: failed to get origin (-5) [ 551.005199] ------------[ cut here ]------------ [ 551.006697] WARNING: CPU: 3 PID: 24674 at fs/inode.c:1528 iput+0x33b/0x400 ... [ 551.027219] Call Trace: [ 551.027623] ovl_create_object+0x13f/0x170 [ 551.028268] ovl_create+0x27/0x30 [ 551.028799] path_openat+0x1a35/0x1ea0 [ 551.029377] do_filp_open+0xad/0x160 [ 551.029944] ? vfs_writev+0xe9/0x170 [ 551.030499] ? page_counter_try_charge+0x77/0x120 [ 551.031245] ? __alloc_fd+0x160/0x2a0 [ 551.031832] ? do_sys_open+0x189/0x340 [ 551.032417] ? get_unused_fd_flags+0x34/0x40 [ 551.033081] do_sys_open+0x189/0x340 [ 551.033632] __x64_sys_creat+0x24/0x30 [ 551.034219] do_syscall_64+0xd5/0x430 [ 551.034800] entry_SYSCALL_64_after_hwframe+0x44/0xa9 One solution is to improve error handling and call iget_failed() if error is encountered. Amir thinks that this path is little intricate and there is not real need to check and initialize OVL_UPPERDATA in ovl_get_inode(). Instead caller of ovl_get_inode() can initialize this state. And this will avoid double checking of metacopy xattr lookup in ovl_lookup() and ovl_get_inode(). OVL_UPPERDATA is inode flag. So I was little concerned that initializing it outside ovl_get_inode() might have some races. But this is one way transition. That is once a file has been fully copied up, it can't go back to metacopy file again. And that seems to help avoid races. So as of now I can't see any races w.r.t OVL_UPPERDATA being set wrongly. So move settingof OVL_UPPERDATA inside the callers of ovl_get_inode(). ovl_obtain_alias() already does it. So only two callers now left are ovl_lookup() and ovl_instantiate(). Reported-by: yangerkun <yangerkun@huawei.com> Suggested-by: Amir Goldstein <amir73il@gmail.com> Signed-off-by: Vivek Goyal <vgoyal@redhat.com> Reviewed-by: Amir Goldstein <amir73il@gmail.com> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
This commit is contained in:
parent
6815f479ca
commit
28166ab3c8
3 changed files with 5 additions and 10 deletions
|
@ -286,6 +286,8 @@ static int ovl_instantiate(struct dentry *dentry, struct inode *inode,
|
||||||
inode = ovl_get_inode(dentry->d_sb, &oip);
|
inode = ovl_get_inode(dentry->d_sb, &oip);
|
||||||
if (IS_ERR(inode))
|
if (IS_ERR(inode))
|
||||||
return PTR_ERR(inode);
|
return PTR_ERR(inode);
|
||||||
|
if (inode == oip.newinode)
|
||||||
|
ovl_set_flag(OVL_UPPERDATA, inode);
|
||||||
} else {
|
} else {
|
||||||
WARN_ON(ovl_inode_real(inode) != d_inode(newdentry));
|
WARN_ON(ovl_inode_real(inode) != d_inode(newdentry));
|
||||||
dput(newdentry);
|
dput(newdentry);
|
||||||
|
|
|
@ -957,7 +957,7 @@ struct inode *ovl_get_inode(struct super_block *sb,
|
||||||
bool bylower = ovl_hash_bylower(sb, upperdentry, lowerdentry,
|
bool bylower = ovl_hash_bylower(sb, upperdentry, lowerdentry,
|
||||||
oip->index);
|
oip->index);
|
||||||
int fsid = bylower ? lowerpath->layer->fsid : 0;
|
int fsid = bylower ? lowerpath->layer->fsid : 0;
|
||||||
bool is_dir, metacopy = false;
|
bool is_dir;
|
||||||
unsigned long ino = 0;
|
unsigned long ino = 0;
|
||||||
int err = oip->newinode ? -EEXIST : -ENOMEM;
|
int err = oip->newinode ? -EEXIST : -ENOMEM;
|
||||||
|
|
||||||
|
@ -1018,15 +1018,6 @@ struct inode *ovl_get_inode(struct super_block *sb,
|
||||||
if (oip->index)
|
if (oip->index)
|
||||||
ovl_set_flag(OVL_INDEX, inode);
|
ovl_set_flag(OVL_INDEX, inode);
|
||||||
|
|
||||||
if (upperdentry) {
|
|
||||||
err = ovl_check_metacopy_xattr(upperdentry);
|
|
||||||
if (err < 0)
|
|
||||||
goto out_err;
|
|
||||||
metacopy = err;
|
|
||||||
if (!metacopy)
|
|
||||||
ovl_set_flag(OVL_UPPERDATA, inode);
|
|
||||||
}
|
|
||||||
|
|
||||||
OVL_I(inode)->redirect = oip->redirect;
|
OVL_I(inode)->redirect = oip->redirect;
|
||||||
|
|
||||||
if (bylower)
|
if (bylower)
|
||||||
|
|
|
@ -1067,6 +1067,8 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
|
||||||
err = PTR_ERR(inode);
|
err = PTR_ERR(inode);
|
||||||
if (IS_ERR(inode))
|
if (IS_ERR(inode))
|
||||||
goto out_free_oe;
|
goto out_free_oe;
|
||||||
|
if (upperdentry && !uppermetacopy)
|
||||||
|
ovl_set_flag(OVL_UPPERDATA, inode);
|
||||||
}
|
}
|
||||||
|
|
||||||
ovl_dentry_update_reval(dentry, upperdentry,
|
ovl_dentry_update_reval(dentry, upperdentry,
|
||||||
|
|
Loading…
Reference in a new issue