Whamcloud - gitweb
- fixed using of few deprected functions:
authoryury <yury>
Fri, 6 May 2005 18:32:05 +0000 (18:32 +0000)
committeryury <yury>
Fri, 6 May 2005 18:32:05 +0000 (18:32 +0000)
  - sleep_on() is known to be racy replacing it with OBD_SLEEP_ON()
  - added OBD_SLEEP_ON() which is inspired by wait_event() macro.
  - inter_module_{get|put}() replaced with symbol_{get|put}()

- fixes in GNS stuff:
  - ll_revalidate_it() calls missed ll_intent_release() in the case
    GNS dentry is found (fix for #6176)

  - added ll_gns_send_signal() which is needed to cause syscall restarting in the
    case of -ERESTARTSYS is returned.

  - cleanups in ll_gns_mount_object()

  - ll_gns_mount_object() does not wait unconditionaly for userspace upcall,
    instead it give it some time to complete (10s) and returns error if it
    did not mount dentry. This fixes the case when upcall script runs endless
    loop and never returns. This also fixes possible deadlock if another thread
    with wait on not finished mount (this schema is disabled by and -ERESTARTSYS
    is used instead. Both, waiting aproach and -ERESTARTSYS one need more
    investigation).

  - handling result of inode_setattr() call. Printing error message on error.
    This fixes wanings that result of calling this function should be checked.

  - fixed debug message about GNS inabilty to mount dentry (missed arg to CDEBUG
    format string)

  - cleanups in obdfilter_init()
  - fixed using "mount" command in sanity-gns.sh

lustre/include/linux/obd_support.h
lustre/ldlm/ldlm_lib.c
lustre/llite/dcache.c
lustre/llite/llite_gns.c
lustre/llite/llite_internal.h
lustre/llite/llite_lib.c
lustre/llite/namei.c
lustre/llite/super25.c
lustre/lvfs/lvfs_linux.c
lustre/obdfilter/filter.c
lustre/tests/sanity-gns.sh

index fe95a50..f00483c 100644 (file)
@@ -212,6 +212,19 @@ do {                                                                         \
 } while(0)
 
 #ifdef __KERNEL__
+
+/*
+ * sleep_on() is known to be racy, using wait_event() interface instead as
+ * recommended. --umka
+ */
+#define OBD_SLEEP_ON(wq)                                                     \
+do {                                                                         \
+        DEFINE_WAIT(__wait);                                                 \
+        prepare_to_wait(&wq, &__wait, TASK_UNINTERRUPTIBLE);                 \
+        schedule();                                                          \
+        finish_wait(&wq, &__wait);                                           \
+} while (0)
+
 /* The idea here is to synchronise two threads to force a race. The
  * first thread that calls this with a matching fail_loc is put to
  * sleep. The next thread that calls with the same fail_loc wakes up
@@ -220,7 +233,7 @@ do {                                                                         \
 do {                                                            \
         if  (OBD_FAIL_CHECK_ONCE(id)) {                         \
                 CERROR("obd_race id %x sleeping\n", (id));      \
-                sleep_on(&obd_race_waitq);                      \
+                OBD_SLEEP_ON(obd_race_waitq);                   \
                 CERROR("obd_fail_race id %x awake\n", (id));    \
         } else if ((obd_fail_loc & OBD_FAIL_MASK_LOC) ==        \
                     ((id) & OBD_FAIL_MASK_LOC)) {               \
@@ -320,6 +333,7 @@ struct mem_track {
         int m_size;
 };
 
+void lvfs_memdbg_show(void);
 void lvfs_memdbg_insert(struct mem_track *mt);
 void lvfs_memdbg_remove(struct mem_track *mt);
 struct mem_track *lvfs_memdbg_find(void *ptr);
index ed82823..d45f1e4 100644 (file)
@@ -318,14 +318,14 @@ int client_obd_setup(struct obd_device *obddev, obd_count len, void *buf)
                         GOTO(err_import, rc = -ENOSYS);
                 }
 
-                register_f = inter_module_get("mgmtcli_register_for_events");
+                register_f = (mgmtcli_register_for_events_t)symbol_get("mgmtcli_register_for_events");
                 if (!register_f) {
                         CERROR("can't i_m_g mgmtcli_register_for_events\n");
                         GOTO(err_import, rc = -ENOSYS);
                 }
 
                 rc = register_f(mgmt_obd, obddev, &imp->imp_target_uuid);
-                inter_module_put("mgmtcli_register_for_events");
+                symbol_put("mgmtcli_register_for_events");
 
                 if (!rc)
                         cli->cl_mgmtcli_obd = mgmt_obd;
@@ -352,9 +352,9 @@ int client_obd_cleanup(struct obd_device *obddev, int flags)
         if (cli->cl_mgmtcli_obd) {
                 mgmtcli_deregister_for_events_t dereg_f;
 
-                dereg_f = inter_module_get("mgmtcli_deregister_for_events");
+                dereg_f = symbol_get("mgmtcli_deregister_for_events");
                 dereg_f(cli->cl_mgmtcli_obd, obddev);
-                inter_module_put("mgmtcli_deregister_for_events");
+                symbol_put("mgmtcli_deregister_for_events");
         }
 
         /* Here we try to drop the security structure after destroy import,
index 096d91e..2f31473 100644 (file)
@@ -139,15 +139,6 @@ void ll_intent_release(struct lookup_intent *it)
         EXIT;
 }
 
-void ll_intent_free(struct lookup_intent *it)
-{
-        if (it->d.fs_data) {
-                OBD_SLAB_FREE(it->d.fs_data, ll_intent_slab,
-                               sizeof(struct lustre_intent_data));
-                it->d.fs_data = NULL;
-        }
-}
-
 void ll_unhash_aliases(struct inode *inode)
 {
         struct list_head *tmp, *head;
@@ -277,7 +268,7 @@ int ll_intent_alloc(struct lookup_intent *it)
                 return 0;
         }
         OBD_SLAB_ALLOC(it->d.fs_data, ll_intent_slab, SLAB_KERNEL,
-                        sizeof(struct lustre_intent_data));
+                       sizeof(struct lustre_intent_data));
         if (!it->d.fs_data) {
                 CERROR("Failed to allocate memory for lustre specific intent "
                        "data\n");
@@ -285,10 +276,18 @@ int ll_intent_alloc(struct lookup_intent *it)
         }
 
         it->it_op_release = ll_intent_release;
-
         return 0;
 }
 
+void ll_intent_free(struct lookup_intent *it)
+{
+        if (it->d.fs_data) {
+                OBD_SLAB_FREE(it->d.fs_data, ll_intent_slab,
+                              sizeof(struct lustre_intent_data));
+                it->d.fs_data = NULL;
+        }
+}
+
 static inline int 
 ll_special_name(struct dentry *de)
 {
@@ -516,16 +515,19 @@ out:
          * lookup control path, which is always made with parent's i_sem taken.
          * --umka
          */
-        if (!((de->d_inode->i_mode & S_ISUID) && S_ISDIR(de->d_inode->i_mode)) ||
-            !(flags & LOOKUP_CONTINUE || (orig_it & (IT_CHDIR | IT_OPEN))))
+        if (((de->d_inode->i_mode & S_ISUID) && S_ISDIR(de->d_inode->i_mode)) ||
+            !(flags & LOOKUP_CONTINUE || (orig_it & (IT_CHDIR | IT_OPEN)))) {
+                
+                /* special "." and ".." has to be always revalidated */
+                if (rc && !ll_special_name(de) && nd != NULL && !(nd->flags & LOOKUP_LAST)) {
+                        ll_intent_drop_lock(it);
+                        rc = 0;
+                }
+                
+                ll_intent_release(it);
                return rc;
-           
-       /* special "." and ".." has to be always revalidated */
-        if (rc && !ll_special_name(de) && nd != NULL && !(nd->flags & LOOKUP_LAST)) {
-                ll_intent_drop_lock(it);
-                return 0;
         }
-
+           
         return rc;
 do_lookup:
         it = &lookup_it;
@@ -537,7 +539,8 @@ do_lookup:
                             de->d_name.len, NULL, 0, NULL,
                             it, 0, &req, ll_mdc_blocking_ast);
         if (rc >= 0) {
-                struct mds_body *mds_body = lustre_msg_buf(req->rq_repmsg, 1, sizeof(*mds_body));
+                struct mds_body *mds_body = lustre_msg_buf(req->rq_repmsg, 1,
+                                                           sizeof(*mds_body));
 
                 /* See if we got same inode, if not - return error */
                 if (id_equal_stc(&cid, &mds_body->id1))
index 7b8c3b9..d1836a0 100644 (file)
@@ -69,6 +69,24 @@ ll_gns_wait_for_mount(struct dentry *dentry,
         RETURN(-ETIME);
 }
 
+/* 
+ * sending a signal known to be ignored to cause restarting syscall if GNS mount
+ * function returns -ERESTARTSYS.
+ */
+static void
+ll_gns_send_signal(void)
+{
+        struct task_struct *task = current;
+        int signal = SIGCONT;
+
+        read_lock(&tasklist_lock);
+        spin_lock_irq(&task->sighand->siglock);
+        sigaddset(&task->pending.signal, signal);
+        spin_unlock_irq(&task->sighand->siglock);
+        read_unlock(&tasklist_lock);
+        set_tsk_thread_flag(task, TIF_SIGPENDING);
+}
+
 /*
  * tries to mount the mount object under passed @dentry. In the case of success
  * @dentry will become mount point and 0 will be returned. Error code will be
@@ -99,6 +117,12 @@ ll_gns_mount_object(struct dentry *dentry, struct vfsmount *mnt)
                 RETURN(-EINVAL);
         }
         
+        if (mnt == NULL) {
+                CERROR("suid directory found, but no "
+                       "vfsmount available.\n");
+                RETURN(-EINVAL);
+        }
+
         /* 
          * another thead is in progress or just finished mounting the
          * dentry. Handling that.
@@ -106,20 +130,32 @@ ll_gns_mount_object(struct dentry *dentry, struct vfsmount *mnt)
         if (sbi->ll_gns_state == LL_GNS_MOUNTING ||
             sbi->ll_gns_state == LL_GNS_FINISHED) {
                 /* 
-                 * check if another thread is trying to mount some GNS dentry
-                 * too. Letting it know that we busy and make ll_lookup_it() to
-                 * restart syscall and try again later.
+                 * another thread is trying to mount GNS dentry. We'd like to
+                 * handling that.
                  */
                 spin_unlock(&sbi->ll_gns_lock);
-                RETURN(-EAGAIN);
-        }
-        LASSERT(sbi->ll_gns_state == LL_GNS_IDLE);
 
-        if (mnt == NULL) {
-                CERROR("suid directory found, but no "
-                       "vfsmount available.\n");
-                RETURN(-EINVAL);
+                /* 
+                 * check if dentry is mount point already, if so, do not restart
+                 * syscal.
+                 */
+                if (d_mountpoint(dentry))
+                        RETURN(0);
+
+                /* 
+                 * causing syscall to restart and find this dentry already
+                 * mounted.
+                 */
+                ll_gns_send_signal();
+                RETURN(-ERESTARTSYS);
+
+#if 0
+                wait_for_completion(&sbi->ll_gns_mount_finished);
+                if (d_mountpoint(dentry))
+                        RETURN(0);
+#endif
         }
+        LASSERT(sbi->ll_gns_state == LL_GNS_IDLE);
 
         CDEBUG(D_INODE, "mounting dentry %p\n", dentry);
 
@@ -243,7 +279,8 @@ ll_gns_mount_object(struct dentry *dentry, struct vfsmount *mnt)
         
         up(&sbi->ll_gns_sem);
 
-        rc = USERMODEHELPER(argv[0], argv, NULL);
+        /* do not wait for helper complete here. */
+        rc = call_usermodehelper(argv[0], argv, NULL, 0);
         if (rc) {
                 CERROR("failed to call GNS upcall %s, err = %d\n",
                        sbi->ll_gns_upcall, rc);
@@ -251,13 +288,11 @@ ll_gns_mount_object(struct dentry *dentry, struct vfsmount *mnt)
         }
 
         /*
-         * wait for mount completion. This is actually not needed, because
-         * USERMODEHELPER() returns only when usermode process finishes. But we
-         * doing this just for case USERMODEHELPER() semantics will be changed
-         * or usermode upcall program will start mounting in backgound and
-         * return instantly. --umka
+         * waiting for dentry become mount point GNS_WAIT_ATTEMPTS times by 1
+         * second.
          */
         rc = ll_gns_wait_for_mount(dentry, 1, GNS_WAIT_ATTEMPTS);
+        complete_all(&sbi->ll_gns_mount_finished);
         if (rc == 0) {
                 struct dentry *rdentry;
                 struct vfsmount *rmnt;
@@ -303,6 +338,7 @@ cleanup:
                         dput(dchild);
         case 1:
                 free_page((unsigned long)pathpage);
+                complete_all(&sbi->ll_gns_mount_finished);
         case 0:
                 spin_lock(&sbi->ll_gns_lock);
                 sbi->ll_gns_state = LL_GNS_IDLE;
index 647eb79..46e05a3 100644 (file)
@@ -96,6 +96,7 @@ struct ll_sb_info {
         int                       ll_gns_state;
         struct timer_list         ll_gns_timer;
         struct list_head          ll_gns_sbi_head;
+        struct completion         ll_gns_mount_finished;
 
         unsigned long             ll_gns_tick;
         unsigned long             ll_gns_timeout;
@@ -489,4 +490,33 @@ ll_prepare_mdc_data(struct mdc_op_data *data, struct inode *i1,
         data->mod_time = LTIME_S(CURRENT_TIME);
 }
 
+#if 0
+/* 
+ * this was needed for catching correct calling place of ll_intent_alloc() with
+ * missed ll_intent_free() causing memory leak. --umka
+ */
+#define ll_intent_alloc(it)                                             \
+        ({                                                              \
+                int err;                                                \
+                OBD_SLAB_ALLOC((it)->d.fs_data, ll_intent_slab, SLAB_KERNEL, \
+                               sizeof(struct lustre_intent_data));      \
+                if (!(it)->d.fs_data) {                                 \
+                        err = -ENOMEM;                                  \
+                } else {                                                \
+                        err = 0;                                        \
+                }                                                       \
+                (it)->it_op_release = ll_intent_release;                \
+                err;                                                    \
+        })
+
+#define ll_intent_free(it)                                      \
+        do {                                                    \
+                if ((it)->d.fs_data) {                                  \
+                        OBD_SLAB_FREE((it)->d.fs_data, ll_intent_slab,  \
+                                      sizeof(struct lustre_intent_data)); \
+                        (it)->d.fs_data = NULL;                         \
+                }                                                       \
+        } while (0)
+#endif
+
 #endif /* LLITE_INTERNAL_H */
index cda0ff6..ab5ce26 100644 (file)
@@ -70,6 +70,7 @@ struct ll_sb_info *lustre_init_sbi(struct super_block *sb)
         spin_lock_init(&sbi->ll_gns_lock);
         INIT_LIST_HEAD(&sbi->ll_gns_sbi_head);
         init_waitqueue_head(&sbi->ll_gns_waitq);
+        init_completion(&sbi->ll_gns_mount_finished);
 
         /* this later may be reset via /proc/fs/... */
         memcpy(sbi->ll_gns_oname, ".mntinfo", strlen(".mntinfo"));
@@ -1029,7 +1030,7 @@ int ll_setattr_raw(struct inode *inode, struct iattr *attr)
         struct ptlrpc_request *request = NULL;
         struct mdc_op_data *op_data;
         int ia_valid = attr->ia_valid;
-        int rc = 0;
+        int err, rc = 0;
         ENTRY;
 
         CDEBUG(D_VFSTRACE, "VFS Op:inode=%lu\n", inode->i_ino);
@@ -1107,7 +1108,13 @@ int ll_setattr_raw(struct inode *inode, struct iattr *attr)
                  * NB: ATTR_SIZE will only be set at this point if the size
                  * resides on the MDS, ie, this file has no objects. */
                 attr->ia_valid &= ~ATTR_SIZE;
-                inode_setattr(inode, attr);
+                err = inode_setattr(inode, attr);
+                if (err) {
+                        CERROR("inode_setattr() failed, inode=%lu/%u(%p), "
+                               "err = %d\n", (unsigned long)inode->i_ino,
+                               inode->i_generation, inode, err);
+                        /* should we go to error path here? --umka */
+                }
                  
                 ll_update_inode(inode, &md);
                 ptlrpc_req_finished(request);
@@ -1136,7 +1143,13 @@ int ll_setattr_raw(struct inode *inode, struct iattr *attr)
                 }
 
                 /* Won't invoke vmtruncate, as we already cleared ATTR_SIZE */
-                inode_setattr(inode, attr);
+                err = inode_setattr(inode, attr);
+                if (err) {
+                        CERROR("inode_setattr() failed, inode=%lu/%u(%p), "
+                               "err = %d\n", (unsigned long)inode->i_ino,
+                               inode->i_generation, inode, err);
+                        /* should we go to error path here? --umka */
+                }
         }
 
         /* We really need to get our PW lock before we change inode->i_size.
index d96b79b..e91adbf 100644 (file)
@@ -405,11 +405,8 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry,
             ((flags & LOOKUP_CONTINUE) || (orig_it & (IT_CHDIR | IT_OPEN))))
         {
                 rc = ll_gns_mount_object(dentry, nd->mnt);
-                if (rc == -EAGAIN) {
-                        /* 
-                         * making system to restart syscall as currently GNS is
-                         * in mounting progress.
-                         */
+                if (rc == -ERESTARTSYS) {
+                        /* causing syscall restart */
                         GOTO(out, retval = ERR_PTR(-ERESTARTSYS));
                 }
 
@@ -429,7 +426,7 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry,
                          * reg file.
                          */
                         CDEBUG(D_INODE, "failed to mount %*s, err %d\n",
-                               (int)dentry->d_name.len, dentry->d_name.name);
+                               (int)dentry->d_name.len, dentry->d_name.name, rc);
                 }
         }
         
index c3210ad..8ff81e9 100644 (file)
@@ -33,6 +33,7 @@
 #include <linux/init.h>
 #include <linux/fs.h>
 #include <linux/lprocfs_status.h>
+#include <linux/obd_support.h>
 #include "llite_internal.h"
 
 struct super_block * ll_get_sb(struct file_system_type *fs_type,
index 599adb4..112a1ad 100644 (file)
@@ -574,7 +574,7 @@ lvfs_memdbg_check_remove(void *ptr)
 EXPORT_SYMBOL(lvfs_memdbg_check_remove);
 #endif
 
-static void lvfs_memdbg_show(void)
+void lvfs_memdbg_show(void)
 {
 #if defined (CONFIG_DEBUG_MEMORY) && defined(__KERNEL__)
         struct hlist_node *node = NULL;
@@ -606,6 +606,7 @@ static void lvfs_memdbg_show(void)
 #endif
         }
 }
+EXPORT_SYMBOL(lvfs_memdbg_show);
 
 static int __init lvfs_linux_init(void)
 {
index 386d02c..16a5a70 100644 (file)
@@ -3106,15 +3106,16 @@ static struct obd_ops filter_sanobd_ops = {
 static int __init obdfilter_init(void)
 {
         struct lprocfs_static_vars lvars;
-        int rc;
+        int size, rc;
 
         printk(KERN_INFO "Lustre: Filtering OBD driver; info@clusterfs.com\n");
 
         lprocfs_init_vars(filter, &lvars);
 
-        OBD_ALLOC(obdfilter_created_scratchpad,
-                  OBDFILTER_CREATED_SCRATCHPAD_ENTRIES * 
-                  sizeof(*obdfilter_created_scratchpad));
+        size = OBDFILTER_CREATED_SCRATCHPAD_ENTRIES * 
+                sizeof(*obdfilter_created_scratchpad);
+        
+        OBD_ALLOC(obdfilter_created_scratchpad, size);
         if (obdfilter_created_scratchpad == NULL) {
                 CERROR ("Can't allocate scratchpad\n");
                 return -ENOMEM;
@@ -3123,9 +3124,7 @@ static int __init obdfilter_init(void)
         rc = class_register_type(&filter_obd_ops, NULL, lvars.module_vars,
                                  OBD_FILTER_DEVICENAME);
         if (rc) {
-                OBD_FREE(obdfilter_created_scratchpad,
-                         OBDFILTER_CREATED_SCRATCHPAD_ENTRIES * 
-                         sizeof(*obdfilter_created_scratchpad));
+                OBD_FREE(obdfilter_created_scratchpad, size);
                 return rc;
         }
 
@@ -3133,9 +3132,7 @@ static int __init obdfilter_init(void)
                                  OBD_FILTER_SAN_DEVICENAME);
         if (rc) {
                 class_unregister_type(OBD_FILTER_DEVICENAME);
-                OBD_FREE(obdfilter_created_scratchpad,
-                         OBDFILTER_CREATED_SCRATCHPAD_ENTRIES * 
-                         sizeof(*obdfilter_created_scratchpad));
+                OBD_FREE(obdfilter_created_scratchpad, size);
         }
         return rc;
 }
index 79eced0..afd5597 100644 (file)
@@ -97,7 +97,7 @@ check_kernel_version() {
 }
 
 run_one() {
-       if ! mount | grep -q $DIR; then
+       if ! cat /proc/mounts | grep -q $DIR; then
                $START
        fi
        echo $PTLDEBUG >/proc/sys/portals/debug 
@@ -959,6 +959,8 @@ $DIR/gns_test_2g/$OBJECT/$OBJECT/$OBJECT $TIMOUT $TICK GENERIC || {
     
     disable_gns
 
+    echo ""
+    echo "turning SUID on $DIR/gns_test_2g/$OBJECT/$OBJECT/$OBJECT off"
     chmod u-s $DIR/gns_test_2g/$OBJECT/$OBJECT/$OBJECT
 
     enable_gns