All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jörn Engel" <joern@logfs.org>
To: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	tux3@tux3.org
Subject: Re: [PATCH] Optimize wait_sb_inodes()
Date: Wed, 26 Jun 2013 10:32:22 -0400	[thread overview]
Message-ID: <20130626143222.GA27481@logfs.org> (raw)
In-Reply-To: <87ehbpntuk.fsf@devron.myhome.or.jp>

On Wed, 26 June 2013 17:45:23 +0900, OGAWA Hirofumi wrote:
>
> 
>  fs/fs-writeback.c  |    5 ++++-
>  include/linux/fs.h |    1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff -puN include/linux/fs.h~tux3-fix-wait_sb_inodes include/linux/fs.h
> --- tux3fs/include/linux/fs.h~tux3-fix-wait_sb_inodes	2013-06-24 19:03:10.000000000 +0900
> +++ tux3fs-hirofumi/include/linux/fs.h	2013-06-24 19:03:10.000000000 +0900
> @@ -1595,6 +1595,7 @@ struct super_operations {
>  	int (*write_inode) (struct inode *, struct writeback_control *wbc);
>  	int (*drop_inode) (struct inode *);
>  	void (*evict_inode) (struct inode *);
> +	void (*wait_inodes)(struct super_block *);
>  	void (*put_super) (struct super_block *);
>  	int (*sync_fs)(struct super_block *sb, int wait);
>  	int (*freeze_fs) (struct super_block *);
> diff -puN fs/fs-writeback.c~tux3-fix-wait_sb_inodes fs/fs-writeback.c
> --- tux3fs/fs/fs-writeback.c~tux3-fix-wait_sb_inodes	2013-06-24 19:03:10.000000000 +0900
> +++ tux3fs-hirofumi/fs/fs-writeback.c	2013-06-24 19:03:10.000000000 +0900
> @@ -1401,7 +1401,10 @@ void sync_inodes_sb(struct super_block *
>  	bdi_queue_work(sb->s_bdi, &work);
>  	wait_for_completion(&done);
>  
> -	wait_sb_inodes(sb);
> +	if (sb->s_op->wait_inodes)
> +		sb->s_op->wait_inodes(sb);
> +	else
> +		wait_sb_inodes(sb);
>  }
>  EXPORT_SYMBOL(sync_inodes_sb);

Two things.  Until there are actual implementations of
s_op->wait_inodes, this is pure obfuscation.  You already know this,
of course.

More interestingly, I personally hate methods with a default option if
they are not implemented.  Not too bad in this particular case, but
the same pattern has burned me a number of times and wasted weeks of
my life.  So I would prefer to unconditionally call
sb->s_op->wait_inodes(sb) and set it to wait_sb_inodes for all
filesystems that don't have a smarter way to do things.

Jörn

--
Linux is more the core point of a concept that surrounds "open source"
which, in turn, is based on a false concept. This concept is that
people actually want to look at source code.
-- Rob Enderle

  reply	other threads:[~2013-06-26 14:32 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-26  8:45 [PATCH] Optimize wait_sb_inodes() OGAWA Hirofumi
2013-06-26  8:45 ` OGAWA Hirofumi
2013-06-26 14:32 ` Jörn Engel [this message]
2013-06-27  0:01   ` OGAWA Hirofumi
2013-06-27  0:01     ` OGAWA Hirofumi
2013-06-26 23:11 ` Dave Chinner
2013-06-27  0:14   ` OGAWA Hirofumi
2013-06-27  0:14     ` OGAWA Hirofumi
2013-06-27  4:47     ` Dave Chinner
2013-06-27  5:18       ` OGAWA Hirofumi
2013-06-27  5:18         ` OGAWA Hirofumi
2013-06-27  6:38         ` Dave Chinner
2013-06-27  7:22           ` OGAWA Hirofumi
2013-06-27  7:22             ` OGAWA Hirofumi
2013-06-27  9:40             ` Dave Chinner
2013-06-27 10:06               ` OGAWA Hirofumi
2013-06-27 10:06                 ` OGAWA Hirofumi
2013-06-27 18:36                 ` Theodore Ts'o
2013-06-27 23:37                   ` OGAWA Hirofumi
2013-06-27 23:37                     ` OGAWA Hirofumi
2013-06-27 23:45                     ` Theodore Ts'o
2013-06-28  0:30                       ` OGAWA Hirofumi
2013-06-28  0:30                         ` OGAWA Hirofumi
2013-06-28  5:23                         ` Dave Chinner
2013-06-28  7:39                           ` OGAWA Hirofumi
2013-06-28  7:39                             ` OGAWA Hirofumi
2013-06-28  3:00                   ` Daniel Phillips
2013-06-27  7:22         ` Daniel Phillips
2013-06-27  5:50       ` Daniel Phillips
2013-06-27  5:50         ` Daniel Phillips

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=20130626143222.GA27481@logfs.org \
    --to=joern@logfs.org \
    --cc=hirofumi@mail.parknet.co.jp \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tux3@tux3.org \
    --cc=viro@zeniv.linux.org.uk \
    /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.