All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch 4/4] reiser4: reduce frame size of reiser4_init_super_data
@ 2009-10-05  0:40 Edward Shishkin
  2009-10-07 19:45 ` Laurent Riffard
  0 siblings, 1 reply; 4+ messages in thread
From: Edward Shishkin @ 2009-10-05  0:40 UTC (permalink / raw)
  To: akpm, ReiserFS Development List

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: reiser4-reduce-frame-size.patch --]
[-- Type: text/plain, Size: 5784 bytes --]

Address a gcc warning for x86_64 about large frame size.
Add a new function push_sb_field_opts(). 

Signed-off-by Edward Shsihkin <edward.shishkin@gmail.com>
---
 fs/reiser4/init_super.c |  126 +++++++++++++++++++++++++-----------------------
 1 file changed, 66 insertions(+), 60 deletions(-)

--- mmotm.orig/fs/reiser4/init_super.c
+++ mmotm/fs/reiser4/init_super.c
@@ -292,66 +292,6 @@ static int parse_options(char *opt_strin
 
 #define MAX_NR_OPTIONS (30)
 
-/**
- * reiser4_init_super_data - initialize reiser4 private super block
- * @super: super block to initialize
- * @opt_string: list of reiser4 mount options
- *
- * Sets various reiser4 parameters to default values. Parses mount options and
- * overwrites default settings.
- */
-int reiser4_init_super_data(struct super_block *super, char *opt_string)
-{
-	int result;
-	struct opt_desc *opts, *p;
-	reiser4_super_info_data *sbinfo = get_super_private(super);
-
-	/* initialize super, export, dentry operations */
-	sbinfo->ops.super = reiser4_super_operations;
-	sbinfo->ops.export = reiser4_export_operations;
-	sbinfo->ops.dentry = reiser4_dentry_operations;
-	super->s_op = &sbinfo->ops.super;
-	super->s_export_op = &sbinfo->ops.export;
-
-	/* initialize transaction manager parameters to default values */
-	sbinfo->tmgr.atom_max_size = totalram_pages / 4;
-	sbinfo->tmgr.atom_max_age = REISER4_ATOM_MAX_AGE / HZ;
-	sbinfo->tmgr.atom_min_size = 256;
-	sbinfo->tmgr.atom_max_flushers = ATOM_MAX_FLUSHERS;
-
-	/* initialize cbk cache parameter */
-	sbinfo->tree.cbk_cache.nr_slots = CBK_CACHE_SLOTS;
-
-	/* initialize flush parameters */
-	sbinfo->flush.relocate_threshold = FLUSH_RELOCATE_THRESHOLD;
-	sbinfo->flush.relocate_distance = FLUSH_RELOCATE_DISTANCE;
-	sbinfo->flush.written_threshold = FLUSH_WRITTEN_THRESHOLD;
-	sbinfo->flush.scan_maxnodes = FLUSH_SCAN_MAXNODES;
-
-	sbinfo->optimal_io_size = REISER4_OPTIMAL_IO_SIZE;
-
-	/* preliminary tree initializations */
-	sbinfo->tree.super = super;
-	sbinfo->tree.carry.new_node_flags = REISER4_NEW_NODE_FLAGS;
-	sbinfo->tree.carry.new_extent_flags = REISER4_NEW_EXTENT_FLAGS;
-	sbinfo->tree.carry.paste_flags = REISER4_PASTE_FLAGS;
-	sbinfo->tree.carry.insert_flags = REISER4_INSERT_FLAGS;
-	rwlock_init(&(sbinfo->tree.tree_lock));
-	spin_lock_init(&(sbinfo->tree.epoch_lock));
-
-	/* initialize default readahead params */
-	sbinfo->ra_params.max = num_physpages / 4;
-	sbinfo->ra_params.flags = 0;
-
-	/* allocate memory for structure describing reiser4 mount options */
-	opts = kmalloc(sizeof(struct opt_desc) * MAX_NR_OPTIONS,
-		       reiser4_ctx_gfp_mask_get());
-	if (opts == NULL)
-		return RETERR(-ENOMEM);
-
-	/* initialize structure describing reiser4 mount options */
-	p = opts;
-
 #if REISER4_DEBUG
 #  define OPT_ARRAY_CHECK if ((p) > (opts) + MAX_NR_OPTIONS) {		\
 		warning("zam-1046", "opt array is overloaded"); break;	\
@@ -370,6 +310,10 @@ do {						\
 #define PUSH_SB_FIELD_OPT(field, format) PUSH_OPT(SB_FIELD_OPT(field, format))
 #define PUSH_BIT_OPT(name, bit) PUSH_OPT(BIT_OPT(name, bit))
 
+static noinline void push_sb_field_opts(struct opt_desc *p,
+					struct opt_desc *opts,
+					reiser4_super_info_data *sbinfo)
+{
 	/*
 	 * tmgr.atom_max_size=N
 	 * Atoms containing more than N blocks will be forced to commit. N is
@@ -435,7 +379,69 @@ do {						\
 	 */
 	PUSH_SB_FIELD_OPT(altsuper, "%lu");
 #endif
+}
+
+/**
+ * reiser4_init_super_data - initialize reiser4 private super block
+ * @super: super block to initialize
+ * @opt_string: list of reiser4 mount options
+ *
+ * Sets various reiser4 parameters to default values. Parses mount options and
+ * overwrites default settings.
+ */
+int reiser4_init_super_data(struct super_block *super, char *opt_string)
+{
+	int result;
+	struct opt_desc *opts, *p;
+	reiser4_super_info_data *sbinfo = get_super_private(super);
+
+	/* initialize super, export, dentry operations */
+	sbinfo->ops.super = reiser4_super_operations;
+	sbinfo->ops.export = reiser4_export_operations;
+	sbinfo->ops.dentry = reiser4_dentry_operations;
+	super->s_op = &sbinfo->ops.super;
+	super->s_export_op = &sbinfo->ops.export;
+
+	/* initialize transaction manager parameters to default values */
+	sbinfo->tmgr.atom_max_size = totalram_pages / 4;
+	sbinfo->tmgr.atom_max_age = REISER4_ATOM_MAX_AGE / HZ;
+	sbinfo->tmgr.atom_min_size = 256;
+	sbinfo->tmgr.atom_max_flushers = ATOM_MAX_FLUSHERS;
+
+	/* initialize cbk cache parameter */
+	sbinfo->tree.cbk_cache.nr_slots = CBK_CACHE_SLOTS;
+
+	/* initialize flush parameters */
+	sbinfo->flush.relocate_threshold = FLUSH_RELOCATE_THRESHOLD;
+	sbinfo->flush.relocate_distance = FLUSH_RELOCATE_DISTANCE;
+	sbinfo->flush.written_threshold = FLUSH_WRITTEN_THRESHOLD;
+	sbinfo->flush.scan_maxnodes = FLUSH_SCAN_MAXNODES;
+
+	sbinfo->optimal_io_size = REISER4_OPTIMAL_IO_SIZE;
+
+	/* preliminary tree initializations */
+	sbinfo->tree.super = super;
+	sbinfo->tree.carry.new_node_flags = REISER4_NEW_NODE_FLAGS;
+	sbinfo->tree.carry.new_extent_flags = REISER4_NEW_EXTENT_FLAGS;
+	sbinfo->tree.carry.paste_flags = REISER4_PASTE_FLAGS;
+	sbinfo->tree.carry.insert_flags = REISER4_INSERT_FLAGS;
+	rwlock_init(&(sbinfo->tree.tree_lock));
+	spin_lock_init(&(sbinfo->tree.epoch_lock));
+
+	/* initialize default readahead params */
+	sbinfo->ra_params.max = num_physpages / 4;
+	sbinfo->ra_params.flags = 0;
+
+	/* allocate memory for structure describing reiser4 mount options */
+	opts = kmalloc(sizeof(struct opt_desc) * MAX_NR_OPTIONS,
+		       reiser4_ctx_gfp_mask_get());
+	if (opts == NULL)
+		return RETERR(-ENOMEM);
+
+	/* initialize structure describing reiser4 mount options */
+	p = opts;
 
+	push_sb_field_opts(p, opts, sbinfo);
 	/* turn on BSD-style gid assignment */
 	PUSH_BIT_OPT("bsdgroups", REISER4_BSD_GID);
 	/* turn on 32 bit times */

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [patch 4/4] reiser4: reduce frame size of reiser4_init_super_data
  2009-10-05  0:40 [patch 4/4] reiser4: reduce frame size of reiser4_init_super_data Edward Shishkin
@ 2009-10-07 19:45 ` Laurent Riffard
  2009-10-07 19:54   ` Laurent Riffard
  2009-10-07 20:05   ` [patch] reiser4: reduce frame size of reiser4_init_super_data fixup Edward Shishkin
  0 siblings, 2 replies; 4+ messages in thread
From: Laurent Riffard @ 2009-10-07 19:45 UTC (permalink / raw)
  To: Edward Shishkin; +Cc: akpm, ReiserFS Development List

Hi Edward,

This patch is buggy, isn't it ?

I've got 2 reiser4 FS in my /etc/fstab:

/dev/vglinux1/lvkernel-r4 /home/laurent/kernel reiser4 defaults,noatime,nodiratime,tmgr.atom_max_size=2048 0 0
/dev/disk/by-uuid/b8dbe880-b664-49aa-8050-bddc91fd5e49 /mnt/diske reiser4 noauto,users,noatime,nodiratime 0 0

The first FS can't be mounted:

[  235.078342] reiser4[mount(4205)]: parse_options (fs/reiser4/init_super.c:253)[nikita-2307]:
[  235.078345] WARNING: Unrecognized option: "tmgr.atom_max_size=2048"

although the second one can be mounted:

[ 3152.046324] reiser4: sda7: found disk format 4.0.0.


Let's have a look at the code in fs/reiser4/init_super.c:


392 int reiser4_init_super_data(struct super_block *super, char *opt_string)
393 {
394         int result;
395         struct opt_desc *opts, *p;
396         reiser4_super_info_data *sbinfo = get_super_private(super);
397 
...
442         p = opts;
443 
444         push_sb_field_opts(p, opts, sbinfo);

p is passed by value to push_sb_field(). push_sb_field() does increment 
its local copy of p, but here p remains equal to opts.

...
501         result = parse_options(opt_string, opts, p - opts);

3rd argument is 0 because p==opts. Now let's have a look at parse_options()

230 static int parse_options(char *opt_string, struct opt_desc *opts, int nr_opts)
231 {

nr_opts always == 0 here.

232         int result;
233
234         result = 0;
235         while ((result == 0) && opt_string && *opt_string) {

I assume opt_string is not null (opt_string == "tmgr.atom_max_size=2048" ?), 
so let's loop:

236                 int j;
237                 char *next;
244                 for (j = 0; j < nr_opts; ++j) {

nr_opts == 0, so we won't do any iteration here.

245                         if (!strncmp(opt_string, opts[j].name,
246                                      strlen(opts[j].name))) {
247                                 result = parse_option(opt_string, &opts[j]);
248                                 break;
249                         }
250                 }

here, j==0 and nr_opts==0.

251                 if (j == nr_opts) {
252                         warning("nikita-2307", "Unrecognized option: \"%s\"",
253                                 opt_string);
254                         /* traditionally, -EINVAL is returned on wrong mount
255                            option */
256                         result = RETERR(-EINVAL);

oops !

~~
laurent

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [patch 4/4] reiser4: reduce frame size of reiser4_init_super_data
  2009-10-07 19:45 ` Laurent Riffard
@ 2009-10-07 19:54   ` Laurent Riffard
  2009-10-07 20:05   ` [patch] reiser4: reduce frame size of reiser4_init_super_data fixup Edward Shishkin
  1 sibling, 0 replies; 4+ messages in thread
From: Laurent Riffard @ 2009-10-07 19:54 UTC (permalink / raw)
  Cc: Edward Shishkin, akpm, ReiserFS Development List

Oops, sorry, Edward already sent a patch for this...

~~
laurent


Le 07/10/2009 21:45, Laurent Riffard a écrit :
> Hi Edward,
> 
> This patch is buggy, isn't it ?
> 
> I've got 2 reiser4 FS in my /etc/fstab:
> 
> /dev/vglinux1/lvkernel-r4 /home/laurent/kernel reiser4 defaults,noatime,nodiratime,tmgr.atom_max_size=2048 0 0
> /dev/disk/by-uuid/b8dbe880-b664-49aa-8050-bddc91fd5e49 /mnt/diske reiser4 noauto,users,noatime,nodiratime 0 0
> 
> The first FS can't be mounted:
> 
> [  235.078342] reiser4[mount(4205)]: parse_options (fs/reiser4/init_super.c:253)[nikita-2307]:
> [  235.078345] WARNING: Unrecognized option: "tmgr.atom_max_size=2048"
> 
> although the second one can be mounted:
> 
> [ 3152.046324] reiser4: sda7: found disk format 4.0.0.
> 
> 
> Let's have a look at the code in fs/reiser4/init_super.c:
> 
> 
> 392 int reiser4_init_super_data(struct super_block *super, char *opt_string)
> 393 {
> 394         int result;
> 395         struct opt_desc *opts, *p;
> 396         reiser4_super_info_data *sbinfo = get_super_private(super);
> 397 
> ...
> 442         p = opts;
> 443 
> 444         push_sb_field_opts(p, opts, sbinfo);
> 
> p is passed by value to push_sb_field(). push_sb_field() does increment 
> its local copy of p, but here p remains equal to opts.
> 
> ...
> 501         result = parse_options(opt_string, opts, p - opts);
> 
> 3rd argument is 0 because p==opts. Now let's have a look at parse_options()
> 
> 230 static int parse_options(char *opt_string, struct opt_desc *opts, int nr_opts)
> 231 {
> 
> nr_opts always == 0 here.
> 
> 232         int result;
> 233
> 234         result = 0;
> 235         while ((result == 0) && opt_string && *opt_string) {
> 
> I assume opt_string is not null (opt_string == "tmgr.atom_max_size=2048" ?), 
> so let's loop:
> 
> 236                 int j;
> 237                 char *next;
> 244                 for (j = 0; j < nr_opts; ++j) {
> 
> nr_opts == 0, so we won't do any iteration here.
> 
> 245                         if (!strncmp(opt_string, opts[j].name,
> 246                                      strlen(opts[j].name))) {
> 247                                 result = parse_option(opt_string, &opts[j]);
> 248                                 break;
> 249                         }
> 250                 }
> 
> here, j==0 and nr_opts==0.
> 
> 251                 if (j == nr_opts) {
> 252                         warning("nikita-2307", "Unrecognized option: \"%s\"",
> 253                                 opt_string);
> 254                         /* traditionally, -EINVAL is returned on wrong mount
> 255                            option */
> 256                         result = RETERR(-EINVAL);
> 
> oops !
> 
> ~~
> laurent
> --
> To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [patch] reiser4: reduce frame size of reiser4_init_super_data fixup
  2009-10-07 19:45 ` Laurent Riffard
  2009-10-07 19:54   ` Laurent Riffard
@ 2009-10-07 20:05   ` Edward Shishkin
  1 sibling, 0 replies; 4+ messages in thread
From: Edward Shishkin @ 2009-10-07 20:05 UTC (permalink / raw)
  To: Laurent Riffard, akpm; +Cc: ReiserFS Development List

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

Laurent Riffard wrote:
> Hi Edward,
>   

Hello Laurent.

> This patch is buggy, isn't it ?
>   

Yes, sorry, my fault..
I have sent the fixup already to the list yesterday..
Resending for you and Akpm.
Andrew, please apply.

Thanks.
Edward.

> I've got 2 reiser4 FS in my /etc/fstab:
>
> /dev/vglinux1/lvkernel-r4 /home/laurent/kernel reiser4 defaults,noatime,nodiratime,tmgr.atom_max_size=2048 0 0
> /dev/disk/by-uuid/b8dbe880-b664-49aa-8050-bddc91fd5e49 /mnt/diske reiser4 noauto,users,noatime,nodiratime 0 0
>
> The first FS can't be mounted:
>
> [  235.078342] reiser4[mount(4205)]: parse_options (fs/reiser4/init_super.c:253)[nikita-2307]:
> [  235.078345] WARNING: Unrecognized option: "tmgr.atom_max_size=2048"
>   


[-- Attachment #2: reiser4-reduce-frame-size-fix.patch --]
[-- Type: text/plain, Size: 2644 bytes --]

. Fix up the bug in reiser4_init_super_data():
  The pointer "p" to opt_desc structure is not
  incremented.
  Pass "&p" instead of "p" to push_sb_field_opts(),
  which is supposed to increment the pointer.
. Modify macros PUSH_OPT, OPT_ARRAY_CHECK to accept
  arguments.

Signed-off-by Edward Shsihkin <edward.shishkin@gmail.com>
---
 fs/reiser4/init_super.c |   28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

--- mmotm.orig/fs/reiser4/init_super.c
+++ mmotm/fs/reiser4/init_super.c
@@ -293,27 +293,27 @@ static int parse_options(char *opt_strin
 #define MAX_NR_OPTIONS (30)
 
 #if REISER4_DEBUG
-#  define OPT_ARRAY_CHECK if ((p) > (opts) + MAX_NR_OPTIONS) {		\
+#  define OPT_ARRAY_CHECK(opt, array)					\
+	if ((opt) > (array) + MAX_NR_OPTIONS) {				\
 		warning("zam-1046", "opt array is overloaded"); break;	\
 	}
 #else
-#   define OPT_ARRAY_CHECK noop
+#   define OPT_ARRAY_CHECK(opt, array) noop
 #endif
 
-#define PUSH_OPT(...)				\
+#define PUSH_OPT(opt, array, ...)		\
 do {						\
 	struct opt_desc o = __VA_ARGS__;	\
-	OPT_ARRAY_CHECK;			\
-	*p ++ = o;				\
+	OPT_ARRAY_CHECK(opt, array);		\
+	*(opt) ++ = o;				\
 } while (0)
 
-#define PUSH_SB_FIELD_OPT(field, format) PUSH_OPT(SB_FIELD_OPT(field, format))
-#define PUSH_BIT_OPT(name, bit) PUSH_OPT(BIT_OPT(name, bit))
-
-static noinline void push_sb_field_opts(struct opt_desc *p,
+static noinline void push_sb_field_opts(struct opt_desc **p,
 					struct opt_desc *opts,
 					reiser4_super_info_data *sbinfo)
 {
+#define PUSH_SB_FIELD_OPT(field, format)		\
+	PUSH_OPT(*p, opts, SB_FIELD_OPT(field, format))
 	/*
 	 * tmgr.atom_max_size=N
 	 * Atoms containing more than N blocks will be forced to commit. N is
@@ -441,8 +441,12 @@ int reiser4_init_super_data(struct super
 	/* initialize structure describing reiser4 mount options */
 	p = opts;
 
-	push_sb_field_opts(p, opts, sbinfo);
+	push_sb_field_opts(&p, opts, sbinfo);
 	/* turn on BSD-style gid assignment */
+
+#define PUSH_BIT_OPT(name, bit)			\
+	PUSH_OPT(p, opts, BIT_OPT(name, bit))
+
 	PUSH_BIT_OPT("bsdgroups", REISER4_BSD_GID);
 	/* turn on 32 bit times */
 	PUSH_BIT_OPT("32bittimes", REISER4_32_BIT_TIMES);
@@ -456,7 +460,7 @@ int reiser4_init_super_data(struct super
 	/* disable use of write barriers in the reiser4 log writer. */
 	PUSH_BIT_OPT("no_write_barrier", REISER4_NO_WRITE_BARRIER);
 
-	PUSH_OPT(
+	PUSH_OPT(p, opts,
 	{
 		/*
 		 * tree traversal readahead parameters:
@@ -482,7 +486,7 @@ int reiser4_init_super_data(struct super
 	);
 
 	/* What to do in case of fs error */
-	PUSH_OPT(
+	PUSH_OPT(p, opts,
 	{
 		.name = "onerror",
 		.type = OPT_ONEOF,

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2009-10-07 20:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-05  0:40 [patch 4/4] reiser4: reduce frame size of reiser4_init_super_data Edward Shishkin
2009-10-07 19:45 ` Laurent Riffard
2009-10-07 19:54   ` Laurent Riffard
2009-10-07 20:05   ` [patch] reiser4: reduce frame size of reiser4_init_super_data fixup Edward Shishkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.