Commit a2c75c81 authored by Gao Xiang's avatar Gao Xiang Committed by Greg Kroah-Hartman

erofs: better erofs symlink stuffs

Fix as Christoph suggested [1] [2], "remove is_inode_fast_symlink
and just opencode it in the few places using it"

and
"Please just set the ops directly instead of obsfucating that in
a single caller, single line inline function.  And please set it
instead of the normal symlink iops in the same place where you
also set those."

[1] https://lore.kernel.org/r/20190830163910.GB29603@infradead.org/
[2] https://lore.kernel.org/r/20190829102426.GE20598@infradead.org/Reported-by: default avatarChristoph Hellwig <hch@infradead.org>
Signed-off-by: default avatarGao Xiang <gaoxiang25@huawei.com>
Link: https://lore.kernel.org/r/20190904020912.63925-13-gaoxiang25@huawei.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 2d78c209
...@@ -127,50 +127,39 @@ static int read_inode(struct inode *inode, void *data) ...@@ -127,50 +127,39 @@ static int read_inode(struct inode *inode, void *data)
return -EFSCORRUPTED; return -EFSCORRUPTED;
} }
/* static int erofs_fill_symlink(struct inode *inode, void *data,
* try_lock can be required since locking order is: unsigned int m_pofs)
* file data(fs_inode)
* meta(bd_inode)
* but the majority of the callers is "iget",
* in that case we are pretty sure no deadlock since
* no data operations exist. However I tend to
* try_lock since it takes no much overhead and
* will success immediately.
*/
static int fill_inline_data(struct inode *inode, void *data,
unsigned int m_pofs)
{ {
struct erofs_inode *vi = EROFS_I(inode); struct erofs_inode *vi = EROFS_I(inode);
struct erofs_sb_info *sbi = EROFS_I_SB(inode); struct erofs_sb_info *sbi = EROFS_I_SB(inode);
char *lnk;
/* should be tail-packing data inline */ /* if it cannot be handled with fast symlink scheme */
if (vi->datalayout != EROFS_INODE_FLAT_INLINE) if (vi->datalayout != EROFS_INODE_FLAT_INLINE ||
inode->i_size >= PAGE_SIZE) {
inode->i_op = &erofs_symlink_iops;
return 0; return 0;
}
/* fast symlink */ lnk = erofs_kmalloc(sbi, inode->i_size + 1, GFP_KERNEL);
if (S_ISLNK(inode->i_mode) && inode->i_size < PAGE_SIZE) { if (!lnk)
char *lnk = erofs_kmalloc(sbi, inode->i_size + 1, GFP_KERNEL); return -ENOMEM;
if (!lnk)
return -ENOMEM;
m_pofs += vi->inode_isize + vi->xattr_isize;
/* inline symlink data shouldn't cross page boundary as well */ m_pofs += vi->inode_isize + vi->xattr_isize;
if (m_pofs + inode->i_size > PAGE_SIZE) { /* inline symlink data shouldn't cross page boundary as well */
kfree(lnk); if (m_pofs + inode->i_size > PAGE_SIZE) {
errln("inline data cross block boundary @ nid %llu", kfree(lnk);
vi->nid); errln("inline data cross block boundary @ nid %llu",
DBG_BUGON(1); vi->nid);
return -EFSCORRUPTED; DBG_BUGON(1);
} return -EFSCORRUPTED;
}
memcpy(lnk, data + m_pofs, inode->i_size); memcpy(lnk, data + m_pofs, inode->i_size);
lnk[inode->i_size] = '\0'; lnk[inode->i_size] = '\0';
inode->i_link = lnk; inode->i_link = lnk;
set_inode_fast_symlink(inode); inode->i_op = &erofs_fast_symlink_iops;
}
return 0; return 0;
} }
...@@ -217,8 +206,9 @@ static int fill_inode(struct inode *inode, int isdir) ...@@ -217,8 +206,9 @@ static int fill_inode(struct inode *inode, int isdir)
inode->i_fop = &erofs_dir_fops; inode->i_fop = &erofs_dir_fops;
break; break;
case S_IFLNK: case S_IFLNK:
/* by default, page_get_link is used for symlink */ err = erofs_fill_symlink(inode, data, ofs);
inode->i_op = &erofs_symlink_iops; if (err)
goto out_unlock;
inode_nohighmem(inode); inode_nohighmem(inode);
break; break;
case S_IFCHR: case S_IFCHR:
...@@ -237,11 +227,7 @@ static int fill_inode(struct inode *inode, int isdir) ...@@ -237,11 +227,7 @@ static int fill_inode(struct inode *inode, int isdir)
err = z_erofs_fill_inode(inode); err = z_erofs_fill_inode(inode);
goto out_unlock; goto out_unlock;
} }
inode->i_mapping->a_ops = &erofs_raw_access_aops; inode->i_mapping->a_ops = &erofs_raw_access_aops;
/* fill last page if inline data is available */
err = fill_inline_data(inode, data, ofs);
} }
out_unlock: out_unlock:
......
...@@ -479,16 +479,6 @@ extern const struct inode_operations erofs_generic_iops; ...@@ -479,16 +479,6 @@ extern const struct inode_operations erofs_generic_iops;
extern const struct inode_operations erofs_symlink_iops; extern const struct inode_operations erofs_symlink_iops;
extern const struct inode_operations erofs_fast_symlink_iops; extern const struct inode_operations erofs_fast_symlink_iops;
static inline void set_inode_fast_symlink(struct inode *inode)
{
inode->i_op = &erofs_fast_symlink_iops;
}
static inline bool is_inode_fast_symlink(struct inode *inode)
{
return inode->i_op == &erofs_fast_symlink_iops;
}
struct inode *erofs_iget(struct super_block *sb, erofs_nid_t nid, bool dir); struct inode *erofs_iget(struct super_block *sb, erofs_nid_t nid, bool dir);
int erofs_getattr(const struct path *path, struct kstat *stat, int erofs_getattr(const struct path *path, struct kstat *stat,
u32 request_mask, unsigned int query_flags); u32 request_mask, unsigned int query_flags);
......
...@@ -40,10 +40,9 @@ static void free_inode(struct inode *inode) ...@@ -40,10 +40,9 @@ static void free_inode(struct inode *inode)
{ {
struct erofs_inode *vi = EROFS_I(inode); struct erofs_inode *vi = EROFS_I(inode);
/* be careful RCU symlink path (see ext4_inode_info->i_data)! */ /* be careful of RCU symlink path */
if (is_inode_fast_symlink(inode)) if (inode->i_op == &erofs_fast_symlink_iops)
kfree(inode->i_link); kfree(inode->i_link);
kfree(vi->xattr_shared_xattrs); kfree(vi->xattr_shared_xattrs);
kmem_cache_free(erofs_inode_cachep, vi); kmem_cache_free(erofs_inode_cachep, vi);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment