All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bernhard Kraft <kraftb@think-open.at>
To: linux-kernel@vger.kernel.org
Subject: ext2/3/4 performance issue
Date: Sun, 10 May 2015 10:01:09 +0200	[thread overview]
Message-ID: <554F1045.4070105@think-open.at> (raw)

Hello folks,

I work on implementing the ext2 filesystem for a PIC microcontroller and 
while reading the sources of it in the linux kernel I stumbled upon the 
following performance issue. I don't know if it is really important but 
I tought I will ask:

In super.c in function "ext2_statfs" there is a for loop [1] which 
iterates over all block groups to determine the overhead each block 
group has. For this it determines if the block group has a superblock 
(by calling "ext2_bg_has_super") and how many blocks are occupied by the 
group descriptor blocks (by calling "ext2_bg_num_gdb").

Now it is the case, that "ext2_bg_num_gdb" itself calls 
"ext2_bg_has_super" in balloc.c [2]. See ext2_bg_num_gdb at the very 
bottom of balloc.c

There will only be a group descriptor if there is a superblock. The 
overhead generated by calling "ext2_bg_has_super" twice is not quite 
minimal. At least if the sparse superblock feature is used as it 
involves checking if the block number equals any power of 3, 5 or 7.

So as every block which has a superblock must also have a group 
descriptor it would be fine to replace the for loop mentioned in [1] by 
a for loop like:

------------ snip -------------------
for (i = 0; i < sbi->s_groups_count; i++)
	if (ext2_bg_has_super(sb, i))
		overhead += 1 + sbi->s_gdb_count;
------------ snip -------------------

The call to "ext_bg_num_gb" is avoided an by this the redundant call to 
"ext2_bg_has_super". As the function isn't used anywhere else it could 
get removed at all.

I don't know if "ext2_statfs" is called often enough that such an 
optimization would make sense. And I am also not involved in kernel 
development so I am not quite into kernel coding guidelines.

The same issue is also valid for ext3 and ext4.


[1] 
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/fs/ext2/super.c#n1395

[2] 
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/fs/ext2/balloc.c#n1532


greetings,
Bernhard
-- 
Wer nicht gelegentlich auch einmal kausalwidrige Dinge zu denken vermag,
wird seine Wissenschaft nie um eine neue Idee bereichern können.
Max Planck (1858-1947)

             reply	other threads:[~2015-05-10  8:28 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-10  8:01 Bernhard Kraft [this message]
2015-05-10 21:56 ` ext2/3/4 performance issue Theodore Ts'o

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=554F1045.4070105@think-open.at \
    --to=kraftb@think-open.at \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.