Whamcloud - gitweb
LU-1337 llite: kernel 3.2 protects inode->i_nlink
authorLiu Xuezhao <xuezhao.liu@emc.com>
Tue, 30 Oct 2012 09:01:51 +0000 (17:01 +0800)
committerOleg Drokin <green@whamcloud.com>
Tue, 20 Nov 2012 23:27:51 +0000 (18:27 -0500)
Kernel 3.2 protects inode->i_nlink from direct modification.
Filesystems can only read i_nlink directly and shall use the
(set|clear|inc|drop)_nlink for modification.
See kernel commit a78ef704a8dd430225955f0709b22d4a6ba21deb.

This patch adds LC_HAVE_PROTECT_I_NLINK checking and implements
set_nlink for old kernel, clear/inc/drop_nlink exists after 2.6.18

Signed-off-by: Liu Xuezhao <xuezhao.liu@emc.com>
Change-Id: Ie958cb308291ecc48d409a1282fed7ea3549a561
Reviewed-on: http://review.whamcloud.com/3577
Tested-by: Hudson
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Yang Sheng <yang.sheng@intel.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/autoconf/lustre-core.m4
lustre/include/linux/lustre_compat25.h
lustre/llite/dcache.c
lustre/llite/file.c
lustre/llite/llite_lib.c
lustre/osd-ldiskfs/osd_handler.c

index c066863..c284b8c 100644 (file)
@@ -1896,6 +1896,27 @@ LB_LINUX_TRY_COMPILE([
 ])
 
 #
+# 3.2 protects inode->i_nlink from direct modification
+# see kernel commit a78ef704a8dd430225955f0709b22d4a6ba21deb
+# at the same time adds set_nlink(), so checks set_nlink() for it.
+#
+AC_DEFUN([LC_HAVE_PROTECT_I_NLINK],
+[AC_MSG_CHECKING([if inode->i_nlink is protected from direct modification])
+LB_LINUX_TRY_COMPILE([
+       #include <linux/fs.h>
+],[
+       struct inode i;
+       set_nlink(&i, 1);
+],[
+       AC_DEFINE(HAVE_PROTECT_I_NLINK, 1,
+                 [inode->i_nlink is protected from direct modification])
+       AC_MSG_RESULT([yes])
+],[
+       AC_MSG_RESULT([no])
+])
+])
+
+#
 # 3.3 introduces migrate_mode.h and migratepage has 4 args
 #
 AC_DEFUN([LC_HAVE_MIGRATE_HEADER],
@@ -2085,6 +2106,7 @@ AC_DEFUN([LC_PROG_LINUX],
 
         # 3.2
         LC_HAVE_VOID_MAKE_REQUEST_FN
+        LC_HAVE_PROTECT_I_NLINK
 
         # 3.3
         LC_HAVE_MIGRATE_HEADER
index 5c1467e..f872a1e 100644 (file)
@@ -714,4 +714,12 @@ static inline int ll_namei_to_lookup_intent_flag(int flag)
 # define LL_MRF_RETURN(rc) RETURN(rc)
 #endif
 
+#include <linux/fs.h>
+#ifndef HAVE_PROTECT_I_NLINK
+static inline void set_nlink(struct inode *inode, unsigned int nlink)
+{
+       inode->i_nlink = nlink;
+}
+#endif
+
 #endif /* _COMPAT25_H */
index 0fc60a4..cad36a4 100644 (file)
@@ -196,7 +196,7 @@ static int ll_ddelete(HAVE_D_DELETE_CONST struct dentry *de)
        /* if not ldlm lock for this inode, set i_nlink to 0 so that
         * this inode can be recycled later b=20433 */
        if (de->d_inode && !find_cbdata(de->d_inode))
-               de->d_inode->i_nlink = 0;
+               clear_nlink(de->d_inode);
 #endif
 
        if (d_lustre_invalid((struct dentry *)de))
@@ -694,10 +694,10 @@ out_it:
 
 void ll_d_iput(struct dentry *de, struct inode *inode)
 {
-        LASSERT(inode);
-        if (!find_cbdata(inode))
-                inode->i_nlink = 0;
-        iput(inode);
+       LASSERT(inode);
+       if (!find_cbdata(inode))
+               clear_nlink(inode);
+       iput(inode);
 }
 
 struct dentry_operations ll_d_ops = {
index b6d6f90..34c08cc 100644 (file)
@@ -2326,24 +2326,23 @@ ldlm_mode_t ll_take_md_lock(struct inode *inode, __u64 bits,
 }
 
 static int ll_inode_revalidate_fini(struct inode *inode, int rc) {
-        if (rc == -ENOENT) { /* Already unlinked. Just update nlink
-                              * and return success */
-                inode->i_nlink = 0;
-                /* This path cannot be hit for regular files unless in
-                 * case of obscure races, so no need to to validate
-                 * size. */
-                if (!S_ISREG(inode->i_mode) &&
-                    !S_ISDIR(inode->i_mode))
-                        return 0;
-        }
-
-        if (rc) {
-                CERROR("failure %d inode %lu\n", rc, inode->i_ino);
-                return -abs(rc);
+       if (rc == -ENOENT) { /* Already unlinked. Just update nlink
+                             * and return success */
+               clear_nlink(inode);
+               /* This path cannot be hit for regular files unless in
+                * case of obscure races, so no need to to validate
+                * size. */
+               if (!S_ISREG(inode->i_mode) &&
+                   !S_ISDIR(inode->i_mode))
+                       return 0;
+       }
 
-        }
+       if (rc) {
+               CERROR("failure %d inode %lu\n", rc, inode->i_ino);
+               return -abs(rc);
+       }
 
-        return 0;
+       return 0;
 }
 
 int __ll_inode_revalidate_it(struct dentry *dentry, struct lookup_intent *it,
index 2a0d6d6..c9ee262 100644 (file)
@@ -1239,24 +1239,24 @@ int ll_md_setattr(struct dentry *dentry, struct md_op_data *op_data,
 
         rc = md_setattr(sbi->ll_md_exp, op_data, NULL, 0, NULL, 0,
                         &request, mod);
-        if (rc) {
-                ptlrpc_req_finished(request);
-                if (rc == -ENOENT) {
-                        inode->i_nlink = 0;
-                        /* Unlinked special device node? Or just a race?
-                         * Pretend we done everything. */
-                        if (!S_ISREG(inode->i_mode) &&
-                            !S_ISDIR(inode->i_mode)) {
-                                ia_valid = op_data->op_attr.ia_valid;
-                                op_data->op_attr.ia_valid &= ~TIMES_SET_FLAGS;
-                                rc = simple_setattr(dentry, &op_data->op_attr);
-                                op_data->op_attr.ia_valid = ia_valid;
-                        }
-                } else if (rc != -EPERM && rc != -EACCES && rc != -ETXTBSY) {
-                        CERROR("md_setattr fails: rc = %d\n", rc);
-                }
-                RETURN(rc);
-        }
+       if (rc) {
+               ptlrpc_req_finished(request);
+               if (rc == -ENOENT) {
+                       clear_nlink(inode);
+                       /* Unlinked special device node? Or just a race?
+                        * Pretend we done everything. */
+                       if (!S_ISREG(inode->i_mode) &&
+                           !S_ISDIR(inode->i_mode)) {
+                               ia_valid = op_data->op_attr.ia_valid;
+                               op_data->op_attr.ia_valid &= ~TIMES_SET_FLAGS;
+                               rc = simple_setattr(dentry, &op_data->op_attr);
+                               op_data->op_attr.ia_valid = ia_valid;
+                       }
+               } else if (rc != -EPERM && rc != -EACCES && rc != -ETXTBSY) {
+                       CERROR("md_setattr fails: rc = %d\n", rc);
+               }
+               RETURN(rc);
+       }
 
         rc = md_get_lustre_md(sbi->ll_md_exp, request, sbi->ll_dt_exp,
                               sbi->ll_md_exp, &md);
@@ -1743,16 +1743,16 @@ void ll_update_inode(struct inode *inode, struct lustre_md *md)
         } else {
                 inode->i_blkbits = inode->i_sb->s_blocksize_bits;
         }
-        if (body->valid & OBD_MD_FLUID)
-                inode->i_uid = body->uid;
-        if (body->valid & OBD_MD_FLGID)
-                inode->i_gid = body->gid;
-        if (body->valid & OBD_MD_FLFLAGS)
-                inode->i_flags = ll_ext_to_inode_flags(body->flags);
-        if (body->valid & OBD_MD_FLNLINK)
-                inode->i_nlink = body->nlink;
-        if (body->valid & OBD_MD_FLRDEV)
-                inode->i_rdev = old_decode_dev(body->rdev);
+       if (body->valid & OBD_MD_FLUID)
+               inode->i_uid = body->uid;
+       if (body->valid & OBD_MD_FLGID)
+               inode->i_gid = body->gid;
+       if (body->valid & OBD_MD_FLFLAGS)
+               inode->i_flags = ll_ext_to_inode_flags(body->flags);
+       if (body->valid & OBD_MD_FLNLINK)
+               set_nlink(inode, body->nlink);
+       if (body->valid & OBD_MD_FLRDEV)
+               inode->i_rdev = old_decode_dev(body->rdev);
 
         if (body->valid & OBD_MD_FLID) {
                 /* FID shouldn't be changed! */
index 819d963..49258c6 100644 (file)
@@ -1543,7 +1543,7 @@ static int osd_inode_setattr(const struct lu_env *env,
         if (bits & LA_GID)
                 inode->i_gid    = attr->la_gid;
         if (bits & LA_NLINK)
-                inode->i_nlink  = attr->la_nlink;
+               set_nlink(inode, attr->la_nlink);
         if (bits & LA_RDEV)
                 inode->i_rdev   = attr->la_rdev;
 
@@ -2057,16 +2057,16 @@ static int osd_object_destroy(const struct lu_env *env,
        /* Parallel control for OI scrub. For most of cases, there is no
         * lock contention. So it will not affect unlink performance. */
        cfs_mutex_lock(&inode->i_mutex);
-        if (S_ISDIR(inode->i_mode)) {
-                LASSERT(osd_inode_unlinked(inode) ||
-                        inode->i_nlink == 1);
-                cfs_spin_lock(&obj->oo_guard);
-                inode->i_nlink = 0;
-                cfs_spin_unlock(&obj->oo_guard);
-                inode->i_sb->s_op->dirty_inode(inode);
-        } else {
-                LASSERT(osd_inode_unlinked(inode));
-        }
+       if (S_ISDIR(inode->i_mode)) {
+               LASSERT(osd_inode_unlinked(inode) ||
+                       inode->i_nlink == 1);
+               cfs_spin_lock(&obj->oo_guard);
+               clear_nlink(inode);
+               cfs_spin_unlock(&obj->oo_guard);
+               inode->i_sb->s_op->dirty_inode(inode);
+       } else {
+               LASSERT(osd_inode_unlinked(inode));
+       }
 
         OSD_EXEC_OP(th, destroy);
 
@@ -2257,29 +2257,33 @@ static int osd_object_ref_add(const struct lu_env *env,
 
         OSD_EXEC_OP(th, ref_add);
 
-        /*
-         * DIR_NLINK feature is set for compatibility reasons if:
-         * 1) nlinks > LDISKFS_LINK_MAX, or
-         * 2) nlinks == 2, since this indicates i_nlink was previously 1.
-         *
-         * It is easier to always set this flag (rather than check and set),
-         * since it has less overhead, and the superblock will be dirtied
-         * at some point. Both e2fsprogs and any Lustre-supported ldiskfs
-         * do not actually care whether this flag is set or not.
-         */
-        cfs_spin_lock(&obj->oo_guard);
-        inode->i_nlink++;
-        if (S_ISDIR(inode->i_mode) && inode->i_nlink > 1) {
-                if (inode->i_nlink >= LDISKFS_LINK_MAX ||
-                    inode->i_nlink == 2)
-                        inode->i_nlink = 1;
-        }
-        LASSERT(inode->i_nlink <= LDISKFS_LINK_MAX);
-        cfs_spin_unlock(&obj->oo_guard);
-        inode->i_sb->s_op->dirty_inode(inode);
-        LINVRNT(osd_invariant(obj));
+       /*
+        * DIR_NLINK feature is set for compatibility reasons if:
+        * 1) nlinks > LDISKFS_LINK_MAX, or
+        * 2) nlinks == 2, since this indicates i_nlink was previously 1.
+        *
+        * It is easier to always set this flag (rather than check and set),
+        * since it has less overhead, and the superblock will be dirtied
+        * at some point. Both e2fsprogs and any Lustre-supported ldiskfs
+        * do not actually care whether this flag is set or not.
+        */
+       cfs_spin_lock(&obj->oo_guard);
+       /* inc_nlink from 0 may cause WARN_ON */
+       if(inode->i_nlink == 0)
+               set_nlink(inode, 1);
+       else
+               inc_nlink(inode);
+       if (S_ISDIR(inode->i_mode) && inode->i_nlink > 1) {
+               if (inode->i_nlink >= LDISKFS_LINK_MAX ||
+                   inode->i_nlink == 2)
+                       set_nlink(inode, 1);
+       }
+       LASSERT(inode->i_nlink <= LDISKFS_LINK_MAX);
+       cfs_spin_unlock(&obj->oo_guard);
+       inode->i_sb->s_op->dirty_inode(inode);
+       LINVRNT(osd_invariant(obj));
 
-        return 0;
+       return 0;
 }
 
 static int osd_declare_object_ref_del(const struct lu_env *env,
@@ -2315,19 +2319,19 @@ static int osd_object_ref_del(const struct lu_env *env, struct dt_object *dt,
 
         OSD_EXEC_OP(th, ref_del);
 
-        cfs_spin_lock(&obj->oo_guard);
-        LASSERT(inode->i_nlink > 0);
-        inode->i_nlink--;
-        /* If this is/was a many-subdir directory (nlink > LDISKFS_LINK_MAX)
-         * then the nlink count is 1. Don't let it be set to 0 or the directory
-         * inode will be deleted incorrectly. */
-        if (S_ISDIR(inode->i_mode) && inode->i_nlink == 0)
-                inode->i_nlink++;
-        cfs_spin_unlock(&obj->oo_guard);
-        inode->i_sb->s_op->dirty_inode(inode);
-        LINVRNT(osd_invariant(obj));
+       cfs_spin_lock(&obj->oo_guard);
+       LASSERT(inode->i_nlink > 0);
+       drop_nlink(inode);
+       /* If this is/was a many-subdir directory (nlink > LDISKFS_LINK_MAX)
+        * then the nlink count is 1. Don't let it be set to 0 or the directory
+        * inode will be deleted incorrectly. */
+       if (S_ISDIR(inode->i_mode) && inode->i_nlink == 0)
+               set_nlink(inode, 1);
+       cfs_spin_unlock(&obj->oo_guard);
+       inode->i_sb->s_op->dirty_inode(inode);
+       LINVRNT(osd_invariant(obj));
 
-        return 0;
+       return 0;
 }
 
 /*