From 869764f64df5b2ef2e978c38bd051b39e65cb97e Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Wed, 16 Jun 2010 13:49:25 -0700 Subject: [PATCH 1/2] Add fix-stack-dsl_deleg_get topic branch Reduce stack usage in dsl_deleg_get, gcc flagged it as consuming a whopping 1040 bytes or potentially 1/4 of a 4K stack. This patch moves all the large structures and buffer off the stack and on to the heap. This includes 2 zap_cursor_t structs each 52 bytes in size, 2 zap_attribute_t structs each 280 bytes in size, and 1 256 byte char array. The total saves on the stack is 880 bytes after you account for the 5 new pointers added. Also the source buffer length has been increased from MAXNAMELEN to MAXNAMELEN+strlen(MOS_DIR_NAME)+1 as described by the comment in dsl_dir_name(). A buffer overrun may have been possible with the slightly smaller buffer. --- .topdeps | 1 + .topmsg | 17 +++++++++++++++++ module/zfs/dsl_deleg.c | 43 +++++++++++++++++++++++++----------------- 3 files changed, 44 insertions(+), 17 deletions(-) create mode 100644 .topdeps create mode 100644 .topmsg diff --git a/.topdeps b/.topdeps new file mode 100644 index 0000000000..1f7391f92b --- /dev/null +++ b/.topdeps @@ -0,0 +1 @@ +master diff --git a/.topmsg b/.topmsg new file mode 100644 index 0000000000..9af76c1d4c --- /dev/null +++ b/.topmsg @@ -0,0 +1,17 @@ +From: Brian Behlendorf +Subject: [PATCH] fix stack dsl_deleg_get + +Reduce stack usage in dsl_deleg_get, gcc flagged it as consuming a +whopping 1040 bytes or potentially 1/4 of a 4K stack. This patch +moves all the large structures and buffer off the stack and on to +the heap. This includes 2 zap_cursor_t structs each 52 bytes in +size, 2 zap_attribute_t structs each 280 bytes in size, and 1 +256 byte char array. The total saves on the stack is 880 bytes +after you account for the 5 new pointers added. + +Also the source buffer length has been increased from MAXNAMELEN +to MAXNAMELEN+strlen(MOS_DIR_NAME)+1 as described by the comment in +dsl_dir_name(). A buffer overrun may have been possible with the +slightly smaller buffer. + +Signed-off-by: Brian Behlendorf diff --git a/module/zfs/dsl_deleg.c b/module/zfs/dsl_deleg.c index 85490c8d5f..54dbe4aa1e 100644 --- a/module/zfs/dsl_deleg.c +++ b/module/zfs/dsl_deleg.c @@ -296,6 +296,9 @@ dsl_deleg_get(const char *ddname, nvlist_t **nvp) dsl_pool_t *dp; int error; objset_t *mos; + zap_cursor_t *basezc, *zc; + zap_attribute_t *baseza, *za; + char *source; error = dsl_dir_open(ddname, FTAG, &startdd, NULL); if (error) @@ -304,15 +307,17 @@ dsl_deleg_get(const char *ddname, nvlist_t **nvp) dp = startdd->dd_pool; mos = dp->dp_meta_objset; + zc = kmem_alloc(sizeof(zap_cursor_t), KM_SLEEP); + za = kmem_alloc(sizeof(zap_attribute_t), KM_SLEEP); + basezc = kmem_alloc(sizeof(zap_cursor_t), KM_SLEEP); + baseza = kmem_alloc(sizeof(zap_attribute_t), KM_SLEEP); + source = kmem_alloc(MAXNAMELEN + strlen(MOS_DIR_NAME) + 1, KM_SLEEP); VERIFY(nvlist_alloc(nvp, NV_UNIQUE_NAME, KM_SLEEP) == 0); rw_enter(&dp->dp_config_rwlock, RW_READER); for (dd = startdd; dd != NULL; dd = dd->dd_parent) { - zap_cursor_t basezc; - zap_attribute_t baseza; nvlist_t *sp_nvp; uint64_t n; - char source[MAXNAMELEN]; if (dd->dd_phys->dd_deleg_zapobj && (zap_count(mos, dd->dd_phys->dd_deleg_zapobj, @@ -323,32 +328,30 @@ dsl_deleg_get(const char *ddname, nvlist_t **nvp) continue; } - for (zap_cursor_init(&basezc, mos, + for (zap_cursor_init(basezc, mos, dd->dd_phys->dd_deleg_zapobj); - zap_cursor_retrieve(&basezc, &baseza) == 0; - zap_cursor_advance(&basezc)) { - zap_cursor_t zc; - zap_attribute_t za; + zap_cursor_retrieve(basezc, baseza) == 0; + zap_cursor_advance(basezc)) { nvlist_t *perms_nvp; - ASSERT(baseza.za_integer_length == 8); - ASSERT(baseza.za_num_integers == 1); + ASSERT(baseza->za_integer_length == 8); + ASSERT(baseza->za_num_integers == 1); VERIFY(nvlist_alloc(&perms_nvp, NV_UNIQUE_NAME, KM_SLEEP) == 0); - for (zap_cursor_init(&zc, mos, baseza.za_first_integer); - zap_cursor_retrieve(&zc, &za) == 0; - zap_cursor_advance(&zc)) { + for (zap_cursor_init(zc, mos, baseza->za_first_integer); + zap_cursor_retrieve(zc, za) == 0; + zap_cursor_advance(zc)) { VERIFY(nvlist_add_boolean(perms_nvp, - za.za_name) == 0); + za->za_name) == 0); } - zap_cursor_fini(&zc); - VERIFY(nvlist_add_nvlist(sp_nvp, baseza.za_name, + zap_cursor_fini(zc); + VERIFY(nvlist_add_nvlist(sp_nvp, baseza->za_name, perms_nvp) == 0); nvlist_free(perms_nvp); } - zap_cursor_fini(&basezc); + zap_cursor_fini(basezc); dsl_dir_name(dd, source); VERIFY(nvlist_add_nvlist(*nvp, source, sp_nvp) == 0); @@ -356,6 +359,12 @@ dsl_deleg_get(const char *ddname, nvlist_t **nvp) } rw_exit(&dp->dp_config_rwlock); + kmem_free(source, MAXNAMELEN + strlen(MOS_DIR_NAME) + 1); + kmem_free(baseza, sizeof(zap_attribute_t)); + kmem_free(basezc, sizeof(zap_cursor_t)); + kmem_free(za, sizeof(zap_attribute_t)); + kmem_free(zc, sizeof(zap_cursor_t)); + dsl_dir_close(startdd, FTAG); return (0); } From 2709f5731b33f127af6e5d2852a1f00c02d2a077 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Wed, 16 Jun 2010 13:49:53 -0700 Subject: [PATCH 2/2] New TopGit dependency: fix-stack-dsl_deleg_get --- .topdeps | 1 + 1 file changed, 1 insertion(+) diff --git a/.topdeps b/.topdeps index a9532822f6..903387bf5f 100644 --- a/.topdeps +++ b/.topdeps @@ -17,3 +17,4 @@ fix-metaslab fix-kstat-xuio fix-stack-lzjb fix-stack-dsl_dir_open_spa +fix-stack-dsl_deleg_get