All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Namjae Jeon <namjae.jeon@samsung.com>
Cc: Dave Chinner <david@fromorbit.com>,
	"Theodore Ts'o" <tytso@mit.edu>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-ext4 <linux-ext4@vger.kernel.org>,
	xfs@oss.sgi.com, Ashish Sangwan <a.sangwan@samsung.com>
Subject: Re: [PATCH v7 10/11]  xfstests: fsstress: Add fallocate insert range operation
Date: Tue, 6 Jan 2015 13:13:09 -0500	[thread overview]
Message-ID: <20150106181309.GH5874@bfoster.bfoster> (raw)
In-Reply-To: <004801d02670$79bd5bf0$6d3813d0$@samsung.com>

On Fri, Jan 02, 2015 at 06:42:51PM +0900, Namjae Jeon wrote:
> This commit adds insert operation support for fsstress, which is
> meant to exercise fallocate FALLOC_FL_INSERT_RANGE support.
> 
> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
> Signed-off-by: Ashish Sangwan <a.sangwan@samsung.com>
> ---

It might be nice to avoid setting FALLOC_FL_KEEP_SIZE for insert and
collapse ops alike (see the only use of KEEP_SIZE in fsstress.c) to
increase the likelihood of valid fallocate() calls. We currently set it
randomly, but unconditionally across all fallocate ops.

That could be done in a separate patch, however, and this looks good to
me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  ltp/fsstress.c | 19 ++++++++++++++++---
>  src/global.h   |  4 ++++
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/ltp/fsstress.c b/ltp/fsstress.c
> index b56fe5c..aa3e0c3 100644
> --- a/ltp/fsstress.c
> +++ b/ltp/fsstress.c
> @@ -72,6 +72,7 @@ typedef enum {
>  	OP_PUNCH,
>  	OP_ZERO,
>  	OP_COLLAPSE,
> +	OP_INSERT,
>  	OP_READ,
>  	OP_READLINK,
>  	OP_RENAME,
> @@ -170,6 +171,7 @@ void	mknod_f(int, long);
>  void	punch_f(int, long);
>  void	zero_f(int, long);
>  void	collapse_f(int, long);
> +void	insert_f(int, long);
>  void	read_f(int, long);
>  void	readlink_f(int, long);
>  void	rename_f(int, long);
> @@ -209,6 +211,7 @@ opdesc_t	ops[] = {
>  	{ OP_PUNCH, "punch", punch_f, 1, 1 },
>  	{ OP_ZERO, "zero", zero_f, 1, 1 },
>  	{ OP_COLLAPSE, "collapse", collapse_f, 1, 1 },
> +	{ OP_INSERT, "insert", insert_f, 1, 1 },
>  	{ OP_READ, "read", read_f, 1, 0 },
>  	{ OP_READLINK, "readlink", readlink_f, 1, 0 },
>  	{ OP_RENAME, "rename", rename_f, 2, 1 },
> @@ -2176,6 +2179,7 @@ struct print_flags falloc_flags [] = {
>  	{ FALLOC_FL_NO_HIDE_STALE, "NO_HIDE_STALE"},
>  	{ FALLOC_FL_COLLAPSE_RANGE, "COLLAPSE_RANGE"},
>  	{ FALLOC_FL_ZERO_RANGE, "ZERO_RANGE"},
> +	{ FALLOC_FL_INSERT_RANGE, "INSERT_RANGE"},
>  	{ -1, NULL}
>  };
>  
> @@ -2227,10 +2231,11 @@ do_fallocate(int opno, long r, int mode)
>  	off %= maxfsize;
>  	len = (off64_t)(random() % (1024 * 1024));
>  	/*
> -	 * Collapse range requires off and len to be block aligned, make it
> -	 * more likely to be the case.
> +	 * Collapse/insert range requires off and len to be block aligned,
> +	 * make it more likely to be the case.
>  	 */
> -	if ((mode & FALLOC_FL_COLLAPSE_RANGE) && (opno % 2)) {
> +	if ((mode & (FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE)) &&
> +		(opno % 2)) {
>  		off = ((off + stb.st_blksize - 1) & ~(stb.st_blksize - 1));
>  		len = ((len + stb.st_blksize - 1) & ~(stb.st_blksize - 1));
>  	}
> @@ -2656,6 +2661,14 @@ collapse_f(int opno, long r)
>  }
>  
>  void
> +insert_f(int opno, long r)
> +{
> +#ifdef HAVE_LINUX_FALLOC_H
> +	do_fallocate(opno, r, FALLOC_FL_INSERT_RANGE);
> +#endif
> +}
> +
> +void
>  read_f(int opno, long r)
>  {
>  	char		*buf;
> diff --git a/src/global.h b/src/global.h
> index 8180f66..f63246b 100644
> --- a/src/global.h
> +++ b/src/global.h
> @@ -172,6 +172,10 @@
>  #define FALLOC_FL_ZERO_RANGE		0x10
>  #endif
>  
> +#ifndef FALLOC_FL_INSERT_RANGE
> +#define FALLOC_FL_INSERT_RANGE		0x20
> +#endif
> +
>  #endif /* HAVE_LINUX_FALLOC_H */
>  
>  #endif /* GLOBAL_H */
> -- 
> 1.7.11-rc0
> 

WARNING: multiple messages have this Message-ID (diff)
From: Brian Foster <bfoster@redhat.com>
To: Namjae Jeon <namjae.jeon@samsung.com>
Cc: Theodore Ts'o <tytso@mit.edu>,
	linux-kernel@vger.kernel.org, xfs@oss.sgi.com,
	Ashish Sangwan <a.sangwan@samsung.com>,
	linux-fsdevel@vger.kernel.org,
	linux-ext4 <linux-ext4@vger.kernel.org>
Subject: Re: [PATCH v7 10/11] xfstests: fsstress: Add fallocate insert range operation
Date: Tue, 6 Jan 2015 13:13:09 -0500	[thread overview]
Message-ID: <20150106181309.GH5874@bfoster.bfoster> (raw)
In-Reply-To: <004801d02670$79bd5bf0$6d3813d0$@samsung.com>

On Fri, Jan 02, 2015 at 06:42:51PM +0900, Namjae Jeon wrote:
> This commit adds insert operation support for fsstress, which is
> meant to exercise fallocate FALLOC_FL_INSERT_RANGE support.
> 
> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
> Signed-off-by: Ashish Sangwan <a.sangwan@samsung.com>
> ---

It might be nice to avoid setting FALLOC_FL_KEEP_SIZE for insert and
collapse ops alike (see the only use of KEEP_SIZE in fsstress.c) to
increase the likelihood of valid fallocate() calls. We currently set it
randomly, but unconditionally across all fallocate ops.

That could be done in a separate patch, however, and this looks good to
me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  ltp/fsstress.c | 19 ++++++++++++++++---
>  src/global.h   |  4 ++++
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/ltp/fsstress.c b/ltp/fsstress.c
> index b56fe5c..aa3e0c3 100644
> --- a/ltp/fsstress.c
> +++ b/ltp/fsstress.c
> @@ -72,6 +72,7 @@ typedef enum {
>  	OP_PUNCH,
>  	OP_ZERO,
>  	OP_COLLAPSE,
> +	OP_INSERT,
>  	OP_READ,
>  	OP_READLINK,
>  	OP_RENAME,
> @@ -170,6 +171,7 @@ void	mknod_f(int, long);
>  void	punch_f(int, long);
>  void	zero_f(int, long);
>  void	collapse_f(int, long);
> +void	insert_f(int, long);
>  void	read_f(int, long);
>  void	readlink_f(int, long);
>  void	rename_f(int, long);
> @@ -209,6 +211,7 @@ opdesc_t	ops[] = {
>  	{ OP_PUNCH, "punch", punch_f, 1, 1 },
>  	{ OP_ZERO, "zero", zero_f, 1, 1 },
>  	{ OP_COLLAPSE, "collapse", collapse_f, 1, 1 },
> +	{ OP_INSERT, "insert", insert_f, 1, 1 },
>  	{ OP_READ, "read", read_f, 1, 0 },
>  	{ OP_READLINK, "readlink", readlink_f, 1, 0 },
>  	{ OP_RENAME, "rename", rename_f, 2, 1 },
> @@ -2176,6 +2179,7 @@ struct print_flags falloc_flags [] = {
>  	{ FALLOC_FL_NO_HIDE_STALE, "NO_HIDE_STALE"},
>  	{ FALLOC_FL_COLLAPSE_RANGE, "COLLAPSE_RANGE"},
>  	{ FALLOC_FL_ZERO_RANGE, "ZERO_RANGE"},
> +	{ FALLOC_FL_INSERT_RANGE, "INSERT_RANGE"},
>  	{ -1, NULL}
>  };
>  
> @@ -2227,10 +2231,11 @@ do_fallocate(int opno, long r, int mode)
>  	off %= maxfsize;
>  	len = (off64_t)(random() % (1024 * 1024));
>  	/*
> -	 * Collapse range requires off and len to be block aligned, make it
> -	 * more likely to be the case.
> +	 * Collapse/insert range requires off and len to be block aligned,
> +	 * make it more likely to be the case.
>  	 */
> -	if ((mode & FALLOC_FL_COLLAPSE_RANGE) && (opno % 2)) {
> +	if ((mode & (FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE)) &&
> +		(opno % 2)) {
>  		off = ((off + stb.st_blksize - 1) & ~(stb.st_blksize - 1));
>  		len = ((len + stb.st_blksize - 1) & ~(stb.st_blksize - 1));
>  	}
> @@ -2656,6 +2661,14 @@ collapse_f(int opno, long r)
>  }
>  
>  void
> +insert_f(int opno, long r)
> +{
> +#ifdef HAVE_LINUX_FALLOC_H
> +	do_fallocate(opno, r, FALLOC_FL_INSERT_RANGE);
> +#endif
> +}
> +
> +void
>  read_f(int opno, long r)
>  {
>  	char		*buf;
> diff --git a/src/global.h b/src/global.h
> index 8180f66..f63246b 100644
> --- a/src/global.h
> +++ b/src/global.h
> @@ -172,6 +172,10 @@
>  #define FALLOC_FL_ZERO_RANGE		0x10
>  #endif
>  
> +#ifndef FALLOC_FL_INSERT_RANGE
> +#define FALLOC_FL_INSERT_RANGE		0x20
> +#endif
> +
>  #endif /* HAVE_LINUX_FALLOC_H */
>  
>  #endif /* GLOBAL_H */
> -- 
> 1.7.11-rc0
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2015-01-06 18:13 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-02  9:42 [PATCH v7 10/11] xfstests: fsstress: Add fallocate insert range operation Namjae Jeon
2015-01-02  9:42 ` Namjae Jeon
2015-01-06 18:13 ` Brian Foster [this message]
2015-01-06 18:13   ` Brian Foster
2015-01-07  5:47   ` Namjae Jeon
2015-01-07  5:47     ` Namjae Jeon

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=20150106181309.GH5874@bfoster.bfoster \
    --to=bfoster@redhat.com \
    --cc=a.sangwan@samsung.com \
    --cc=david@fromorbit.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namjae.jeon@samsung.com \
    --cc=tytso@mit.edu \
    --cc=xfs@oss.sgi.com \
    /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.