From: Gao Xiang <gaoxian...@huawei.com>

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.gb29...@infradead.org/
[2] https://lore.kernel.org/r/20190829102426.ge20...@infradead.org/
Reported-by: Christoph Hellwig <h...@infradead.org>
Signed-off-by: Gao Xiang <gaoxian...@huawei.com>
---
 fs/erofs/inode.c    | 35 ++++++++++-------------------------
 fs/erofs/internal.h | 10 ----------
 fs/erofs/super.c    |  5 ++---
 3 files changed, 12 insertions(+), 38 deletions(-)

diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
index 24c94a7865f2..29a52138fa9d 100644
--- a/fs/erofs/inode.c
+++ b/fs/erofs/inode.c
@@ -109,28 +109,14 @@ static int read_inode(struct inode *inode, void *data)
        return -EFSCORRUPTED;
 }
 
-/*
- * try_lock can be required since locking order is:
- *   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)
+static int erofs_fill_symlink(struct inode *inode, void *data,
+                             unsigned int m_pofs)
 {
        struct erofs_inode *vi = EROFS_I(inode);
        struct erofs_sb_info *sbi = EROFS_I_SB(inode);
 
-       /* should be inode inline C */
-       if (!is_inode_flat_inline(inode))
-               return 0;
-
-       /* fast symlink */
-       if (S_ISLNK(inode->i_mode) && inode->i_size < PAGE_SIZE) {
+       /* if it can be handled with fast symlink scheme */
+       if (is_inode_flat_inline(inode) && inode->i_size < PAGE_SIZE) {
                char *lnk = erofs_kmalloc(sbi, inode->i_size + 1, GFP_KERNEL);
 
                if (!lnk)
@@ -151,7 +137,9 @@ static int fill_inline_data(struct inode *inode, void *data,
                lnk[inode->i_size] = '\0';
 
                inode->i_link = lnk;
-               set_inode_fast_symlink(inode);
+               inode->i_op = &erofs_fast_symlink_iops;
+       } else {
+               inode->i_op = &erofs_symlink_iops;
        }
        return 0;
 }
@@ -199,8 +187,9 @@ static int fill_inode(struct inode *inode, int isdir)
                        inode->i_fop = &erofs_dir_fops;
                        break;
                case S_IFLNK:
-                       /* by default, page_get_link is used for symlink */
-                       inode->i_op = &erofs_symlink_iops;
+                       err = erofs_fill_symlink(inode, data, ofs);
+                       if (err)
+                               goto out_unlock;
                        inode_nohighmem(inode);
                        break;
                case S_IFCHR:
@@ -219,11 +208,7 @@ static int fill_inode(struct inode *inode, int isdir)
                        err = z_erofs_fill_inode(inode);
                        goto out_unlock;
                }
-
                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:
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index e576650bd4f4..4442a6622504 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -480,16 +480,6 @@ extern const struct inode_operations erofs_generic_iops;
 extern const struct inode_operations erofs_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);
 int erofs_getattr(const struct path *path, struct kstat *stat,
                  u32 request_mask, unsigned int query_flags);
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index b0d318a8eb22..8c43af5d5e57 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -38,10 +38,9 @@ static void free_inode(struct inode *inode)
 {
        struct erofs_inode *vi = EROFS_I(inode);
 
-       /* be careful RCU symlink path (see ext4_inode_info->i_data)! */
-       if (is_inode_fast_symlink(inode))
+       /* be careful of RCU symlink path */
+       if (inode->i_op == &erofs_fast_symlink_iops)
                kfree(inode->i_link);
-
        kfree(vi->xattr_shared_xattrs);
 
        kmem_cache_free(erofs_inode_cachep, vi);
-- 
2.17.1

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to