]> granicus.if.org Git - zfs/commitdiff
Excessively large stack frames detected.
authorBrian Behlendorf <behlendorf1@llnl.gov>
Thu, 30 Jul 2009 04:18:48 +0000 (21:18 -0700)
committerBrian Behlendorf <behlendorf1@llnl.gov>
Thu, 30 Jul 2009 04:18:48 +0000 (21:18 -0700)
The 2.6.30 kernel build systems sets -Wframe-larger-than=2048 which causes
a warning to be generated when an individual stack frame exceeds 2048.
This caught the spa_history_log() and dmu_objset_snapshot() functions
which declared a data structure on the stack which contained a char
array of MAXPATHLEN.  This in defined to be 4096 in the linux kernel
and I imagine it is quite large under Solaris as well.  Regardless, the
offending data structures were moved to the heap to correctly keep the
stack depth to a minimum.  We might consider setting this value even
lower to catch additional offenders because we are expecting deep stacks.

module/zfs/dmu_objset.c
module/zfs/spa_history.c

index e962c4b88f645f3e4fa0213198d94585cf992cf2..78d3d8b6d37fa7cc279da85ff4655db6dde88780 100644 (file)
@@ -816,46 +816,50 @@ dmu_objset_snapshot(char *fsname, char *snapname,
     nvlist_t *props, boolean_t recursive)
 {
        dsl_sync_task_t *dst;
-       struct snaparg sn;
+       struct snaparg *sn;
        spa_t *spa;
        int err;
 
-       (void) strcpy(sn.failed, fsname);
+       sn = kmem_alloc(sizeof (struct snaparg), KM_SLEEP);
+       (void) strcpy(sn->failed, fsname);
 
        err = spa_open(fsname, &spa, FTAG);
-       if (err)
+       if (err) {
+               kmem_free(sn, sizeof (struct snaparg));
                return (err);
+       }
 
-       sn.dstg = dsl_sync_task_group_create(spa_get_dsl(spa));
-       sn.snapname = snapname;
-       sn.props = props;
+       sn->dstg = dsl_sync_task_group_create(spa_get_dsl(spa));
+       sn->snapname = snapname;
+       sn->props = props;
 
        if (recursive) {
-               sn.checkperms = B_TRUE;
+               sn->checkperms = B_TRUE;
                err = dmu_objset_find(fsname,
-                   dmu_objset_snapshot_one, &sn, DS_FIND_CHILDREN);
+                   dmu_objset_snapshot_one, sn, DS_FIND_CHILDREN);
        } else {
-               sn.checkperms = B_FALSE;
-               err = dmu_objset_snapshot_one(fsname, &sn);
+               sn->checkperms = B_FALSE;
+               err = dmu_objset_snapshot_one(fsname, sn);
        }
 
        if (err == 0)
-               err = dsl_sync_task_group_wait(sn.dstg);
+               err = dsl_sync_task_group_wait(sn->dstg);
 
-       for (dst = list_head(&sn.dstg->dstg_tasks); dst;
-           dst = list_next(&sn.dstg->dstg_tasks, dst)) {
+       for (dst = list_head(&sn->dstg->dstg_tasks); dst;
+           dst = list_next(&sn->dstg->dstg_tasks, dst)) {
                objset_t *os = dst->dst_arg1;
                dsl_dataset_t *ds = os->os->os_dsl_dataset;
                if (dst->dst_err)
-                       dsl_dataset_name(ds, sn.failed);
+                       dsl_dataset_name(ds, sn->failed);
                zil_resume(dmu_objset_zil(os));
                dmu_objset_close(os);
        }
 
        if (err)
-               (void) strcpy(fsname, sn.failed);
-       dsl_sync_task_group_destroy(sn.dstg);
+               (void) strcpy(fsname, sn->failed);
+       dsl_sync_task_group_destroy(sn->dstg);
        spa_close(spa, FTAG);
+       kmem_free(sn, sizeof (struct snaparg));
        return (err);
 }
 
index 97d97d847875178becbe9a388779a95705fca9ff..fa57e9428400b1797395620872adb50bb7e20496 100644 (file)
@@ -290,15 +290,19 @@ spa_history_log_sync(void *arg1, void *arg2, cred_t *cr, dmu_tx_t *tx)
 int
 spa_history_log(spa_t *spa, const char *history_str, history_log_type_t what)
 {
-       history_arg_t ha;
+       history_arg_t *ha;
+       int err;
 
        ASSERT(what != LOG_INTERNAL);
 
-       ha.ha_history_str = history_str;
-       ha.ha_log_type = what;
-       (void) strlcpy(ha.ha_zone, spa_history_zone(), sizeof (ha.ha_zone));
-       return (dsl_sync_task_do(spa_get_dsl(spa), NULL, spa_history_log_sync,
-           spa, &ha, 0));
+       ha = kmem_alloc(sizeof (history_arg_t), KM_SLEEP);
+       ha->ha_history_str = history_str;
+       ha->ha_log_type = what;
+       (void) strlcpy(ha->ha_zone, spa_history_zone(), sizeof (ha->ha_zone));
+       err = dsl_sync_task_do(spa_get_dsl(spa), NULL, spa_history_log_sync,
+           spa, ha, 0);
+       kmem_free(ha, sizeof (history_arg_t));
+       return (err);
 }
 
 /*