It is very hard to predict runtime for fsstress. In many cases it
is useful to give test to run a reasonable time, and then kill it.
But currently there is no reliable way to kill test without leaving
running children.
This patch add sanity cleanup logic which looks follow:
- On sigterm received by parent, it resend signal to it's children
- Wait for each child to terminates
- EXTRA_SANITY: Even if parent was killed by other signal, children
will be terminated with SIGKILL to preven staled children.
So now one can simply run fsstress like this:
./fsstress -p 1000 -n999999999 -d $TEST_DIR &
PID=$!
sleep 300
kill $PID
wait $PID
Signed-off-by: Dmitry Monakhov <[email protected]>
---
aclocal.m4 | 5 +++++
configure.in | 1 +
ltp/fsstress.c | 38 +++++++++++++++++++++++++++++++++++++-
3 files changed, 43 insertions(+), 1 deletions(-)
diff --git a/aclocal.m4 b/aclocal.m4
index 168eb59..5532606 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -16,6 +16,11 @@ AC_DEFUN([AC_PACKAGE_WANT_LINUX_FIEMAP_H],
AC_SUBST(have_fiemap)
])
+AC_DEFUN([AC_PACKAGE_WANT_LINUX_PRCTL_H],
+ [ AC_CHECK_HEADERS([sys/prctl.h], [ have_prctl=true ], [ have_prctl=false ])
+ AC_SUBST(have_prctl)
+ ])
+
AC_DEFUN([AC_PACKAGE_WANT_FALLOCATE],
[ AC_MSG_CHECKING([for fallocate])
AC_TRY_LINK([
diff --git a/configure.in b/configure.in
index c697b4f..76d23e4 100644
--- a/configure.in
+++ b/configure.in
@@ -67,6 +67,7 @@ in
AC_PACKAGE_WANT_DMAPI
AC_PACKAGE_WANT_LINUX_FIEMAP_H
AC_PACKAGE_WANT_FALLOCATE
+ AC_PACKAGE_WANT_LINUX_PRCTL_H
;;
esac
diff --git a/ltp/fsstress.c b/ltp/fsstress.c
index c37cddf..cee2cad 100644
--- a/ltp/fsstress.c
+++ b/ltp/fsstress.c
@@ -28,7 +28,9 @@
#ifndef HAVE_ATTR_LIST
#define attr_list(path, buf, size, flags, cursor) (errno = -ENOSYS, -1)
#endif
-
+#ifdef HAVE_SYS_PRCTL_H
+#include <sys/prctl.h>
+#endif
#include <math.h>
#define XFS_ERRTAG_MAX 17
#define XFS_IDMODULO_MAX 31 /* user/group IDs (1 << x) */
@@ -209,6 +211,7 @@ int rtpct;
unsigned long seed = 0;
ino_t top_ino;
int verbose = 0;
+int should_stop = 0;
void add_to_flist(int, int, int);
void append_pathname(pathname_t *, char *);
@@ -253,6 +256,10 @@ void usage(void);
void write_freq(void);
void zero_freq(void);
+void sg_handler(int signum) {
+ should_stop = 1;
+}
+
int main(int argc, char **argv)
{
char buf[10];
@@ -267,6 +274,7 @@ int main(int argc, char **argv)
ptrdiff_t srval;
int nousage = 0;
xfs_error_injection_t err_inj;
+ struct sigaction action;
errrange = errtag = 0;
umask(0);
@@ -407,15 +415,43 @@ int main(int argc, char **argv)
}
} else
close(fd);
+ if (setpgid(0, 0) < 0) {
+ perror("setpgrp failed");
+ exit(1);
+ }
+ action.sa_handler = sg_handler;
+ sigemptyset(&action.sa_mask);
+ action.sa_flags = 0;
+ if (sigaction(SIGTERM, &action, 0)) {
+ perror("sigaction failed");
+ exit(1);
+ }
+
for (i = 0; i < nproc; i++) {
if (fork() == 0) {
+ action.sa_handler = SIG_DFL;
+ sigemptyset(&action.sa_mask);
+ if (sigaction(SIGTERM, &action, 0))
+ return 1;
+#ifdef HAVE_SYS_PRCTL_H
+ prctl(PR_SET_PDEATHSIG, SIGKILL);
+ if (getppid() == 1) /* parent died already? */
+ return 0;
+#endif
procid = i;
doproc();
return 0;
}
}
+ while (wait(&stat) > 0 && !should_stop) {
+ continue;
+ }
+ action.sa_flags = SA_RESTART;
+ sigaction(SIGTERM, &action, 0);
+ kill(-getpid(), SIGTERM);
while (wait(&stat) > 0)
continue;
+
if (errtag != 0) {
err_inj.errtag = 0;
err_inj.fd = fd;
--
1.7.2.3
Add two new operations:
- getattr: ioctl(fd, EXT2_IOC_GETFLAGS, &fl)
- setattr: ioctl(fd, EXT2_IOC_SETFLAGS, &random_flags)
By default IOC_SET_SETFLAGS has zero probability because
it may produce inodes with APPEND or IMMUTABLE flags which
are not deletable by default. Let's assumes that one who
enable it knows how to delete such inodes.
For example like follows:
find $TEST_PATH -exec chattr -i -a {} \;
rm -rf $TEST_PATH
Signed-off-by: Dmitry Monakhov <[email protected]>
---
aclocal.m4 | 6 +++++
configure.in | 2 +
ltp/fsstress.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++
m4/package_e2fslib.m4 | 6 +++++
4 files changed, 75 insertions(+), 0 deletions(-)
create mode 100644 m4/package_e2fslib.m4
diff --git a/aclocal.m4 b/aclocal.m4
index 5532606..30ac837 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -21,6 +21,11 @@ AC_DEFUN([AC_PACKAGE_WANT_LINUX_PRCTL_H],
AC_SUBST(have_prctl)
])
+AC_DEFUN([AC_PACKAGE_WANT_EXT2_INCLUDE],
+ [ AC_CHECK_HEADER([ext2fs/ext2fs.h],[ have_ext2_include=true ], [ have_ext2_include=false ])
+ AC_SUBST(have_ext2_include)
+ ])
+
AC_DEFUN([AC_PACKAGE_WANT_FALLOCATE],
[ AC_MSG_CHECKING([for fallocate])
AC_TRY_LINK([
@@ -43,3 +48,4 @@ m4_include([m4/package_globals.m4])
m4_include([m4/package_utilies.m4])
m4_include([m4/package_uuiddev.m4])
m4_include([m4/package_xfslibs.m4])
+m4_include([m4/package_e2fslib.m4])
diff --git a/configure.in b/configure.in
index 76d23e4..1e0d138 100644
--- a/configure.in
+++ b/configure.in
@@ -68,6 +68,8 @@ in
AC_PACKAGE_WANT_LINUX_FIEMAP_H
AC_PACKAGE_WANT_FALLOCATE
AC_PACKAGE_WANT_LINUX_PRCTL_H
+
+ AC_PACKAGE_WANT_EXT2_INCLUDE
;;
esac
diff --git a/ltp/fsstress.c b/ltp/fsstress.c
index cee2cad..416190b 100644
--- a/ltp/fsstress.c
+++ b/ltp/fsstress.c
@@ -31,6 +31,9 @@
#ifdef HAVE_SYS_PRCTL_H
#include <sys/prctl.h>
#endif
+#ifdef HAVE_EXT2_INCLUDE
+#include <ext2fs/ext2fs.h>
+#endif
#include <math.h>
#define XFS_ERRTAG_MAX 17
#define XFS_IDMODULO_MAX 31 /* user/group IDs (1 << x) */
@@ -68,6 +71,8 @@ typedef enum {
OP_UNLINK,
OP_UNRESVSP,
OP_WRITE,
+ OP_GETATTR,
+ OP_SETATTR,
OP_LAST
} opty_t;
@@ -142,6 +147,8 @@ void resvsp_f(int, long);
void rmdir_f(int, long);
void setxattr_f(int, long);
void stat_f(int, long);
+void getattr_f(int, long);
+void setattr_f(int, long);
void symlink_f(int, long);
void sync_f(int, long);
void truncate_f(int, long);
@@ -179,6 +186,8 @@ opdesc_t ops[] = {
{ OP_UNLINK, "unlink", unlink_f, 1, 1 },
{ OP_UNRESVSP, "unresvsp", unresvsp_f, 1, 1 },
{ OP_WRITE, "write", write_f, 4, 1 },
+ { OP_GETATTR, "getattr", getattr_f, 1, 0 },
+ { OP_SETATTR, "setattr", setattr_f, 0, 1 },
}, *ops_end;
flist_t flist[FT_nft] = {
@@ -1729,6 +1738,58 @@ setxattr_f(int opno, long r)
}
void
+getattr_f(int opno, long r)
+{
+#ifdef HAVE_EXT2_INCLUDE
+ int fd;
+ int e;
+ pathname_t f;
+ uint fl;
+ int v;
+
+ init_pathname(&f);
+ if (!get_fname(FT_ANYm, r, &f, NULL, NULL, &v))
+ append_pathname(&f, ".");
+ fd = open_path(&f, O_RDWR);
+ e = fd < 0 ? errno : 0;
+ check_cwd();
+
+ e = ioctl(fd, EXT2_IOC_GETFLAGS, &fl);
+ if (v)
+ printf("%d/%d: getattr %s %u %d\n", procid, opno, f.path, fl, e);
+ free_pathname(&f);
+ close(fd);
+#endif
+}
+
+void
+setattr_f(int opno, long r)
+{
+#ifdef HAVE_EXT2_INCLUDE
+ int fd;
+ int e;
+ pathname_t f;
+ uint fl;
+ int v;
+
+ init_pathname(&f);
+ if (!get_fname(FT_ANYm, r, &f, NULL, NULL, &v))
+ append_pathname(&f, ".");
+ fd = open_path(&f, O_RDWR);
+ e = fd < 0 ? errno : 0;
+ check_cwd();
+
+ fl = (uint)random();
+ e = ioctl(fd, EXT2_IOC_SETFLAGS, &fl);
+ if (v)
+ printf("%d/%d: setattr %s %u %d\n", procid, opno, f.path, fl, e);
+ free_pathname(&f);
+ close(fd);
+#endif
+}
+
+
+void
creat_f(int opno, long r)
{
struct fsxattr a;
diff --git a/m4/package_e2fslib.m4 b/m4/package_e2fslib.m4
new file mode 100644
index 0000000..6ab48f4
--- /dev/null
+++ b/m4/package_e2fslib.m4
@@ -0,0 +1,6 @@
+AC_DEFUN([AC_PACKAGE_WANT_EXT2_INCLUDE],
+ [AC_CHECK_HEADER([ext2fs/ext2fs.h])
+ if test "$ac_cv_header_ext2fs_ext2fs_h" == "yes"; then
+ AC_DEFINE([HAVE_EXT2_INCLUDE], 1, [Header files for e2fslib])
+ fi
+])
\ No newline at end of file
--
1.7.2.3
On Sun, Sep 18, 2011 at 06:54:59PM +0400, Dmitry Monakhov wrote:
> Add two new operations:
> - getattr: ioctl(fd, EXT2_IOC_GETFLAGS, &fl)
> - setattr: ioctl(fd, EXT2_IOC_SETFLAGS, &random_flags)
> By default IOC_SET_SETFLAGS has zero probability because
> it may produce inodes with APPEND or IMMUTABLE flags which
> are not deletable by default. Let's assumes that one who
> enable it knows how to delete such inodes.
> For example like follows:
> find $TEST_PATH -exec chattr -i -a {} \;
> rm -rf $TEST_PATH
In general I like this, but:
- please provide a testcase actually using this new feature, and
- please don't require e2fsprogs just for the ioctl subcommands,
and use the FS_IOC_GET/SETFLAGS names provided by recent kernels
in fs.h instead. You might still need an ifdef for old kernels,
like src/t_immutable.c does. In fact it might be a good idea
to just provide the values for them if they aren't present in
a header shared by fsstress and t_immutable.c
On Sun, 2011-09-18 at 18:54 +0400, Dmitry Monakhov wrote:
> It is very hard to predict runtime for fsstress. In many cases it
> is useful to give test to run a reasonable time, and then kill it.
> But currently there is no reliable way to kill test without leaving
> running children.
> This patch add sanity cleanup logic which looks follow:
> - On sigterm received by parent, it resend signal to it's children
> - Wait for each child to terminates
> - EXTRA_SANITY: Even if parent was killed by other signal, children
> will be terminated with SIGKILL to preven staled children.
>
> So now one can simply run fsstress like this:
> ./fsstress -p 1000 -n999999999 -d $TEST_DIR &
> PID=$!
> sleep 300
> kill $PID
> wait $PID
>
> Signed-off-by: Dmitry Monakhov <[email protected]>
I think this is an interesting change and it looks
OK to me. I agree with Christoph's suggestion (on
the second patch in this series) that it would be
nice to have at least one of the tests make use of
it, if nothing else just to document that it's a
reasonable thing to do.
But even without that I think this is both useful
and harmless.
Reviewed-by: Alex Elder <[email protected]>
On Sun, 2011-09-18 at 18:54 +0400, Dmitry Monakhov wrote:
> Add two new operations:
> - getattr: ioctl(fd, EXT2_IOC_GETFLAGS, &fl)
> - setattr: ioctl(fd, EXT2_IOC_SETFLAGS, &random_flags)
> By default IOC_SET_SETFLAGS has zero probability because
> it may produce inodes with APPEND or IMMUTABLE flags which
> are not deletable by default. Let's assumes that one who
> enable it knows how to delete such inodes.
> For example like follows:
> find $TEST_PATH -exec chattr -i -a {} \;
> rm -rf $TEST_PATH
>
> Signed-off-by: Dmitry Monakhov <[email protected]>
I have a question below. I think this is probably
a good addition, though it should be made so it
works for more than EXTx.
If I understand the way it would be used, this will
simply be another operation that gets randomly performed
by fsstress while it operates, right?
I have not done any testing with this yet.
Reviewed-by: Alex Elder <[email protected]>
. . .
> @@ -1729,6 +1738,58 @@ setxattr_f(int opno, long r)
> }
>
> void
> +getattr_f(int opno, long r)
> +{
> +#ifdef HAVE_EXT2_INCLUDE
> + int fd;
> + int e;
> + pathname_t f;
> + uint fl;
> + int v;
> +
> + init_pathname(&f);
> + if (!get_fname(FT_ANYm, r, &f, NULL, NULL, &v))
> + append_pathname(&f, ".");
I don't understand the purpose of appending a "." to the
end of the path. Do you intend to just use "." if
no other file matches? (That may not be a good thing to
do--it might not be testing the intended target.)
Or are you intending to append "/." so for a directory
its "." link gets used in the open? If so that's not
what this does (it simply makes "a/b/x" become "a/b/x.").
Same comments apply to setattr_f().
> + fd = open_path(&f, O_RDWR);
> + e = fd < 0 ? errno : 0;
> + check_cwd();
> +
> + e = ioctl(fd, EXT2_IOC_GETFLAGS, &fl);
> + if (v)
> + printf("%d/%d: getattr %s %u %d\n", procid, opno, f.path, fl, e);
> + free_pathname(&f);
> + close(fd);
> +#endif
> +}
> +
> +void
> +setattr_f(int opno, long r)
> +{
On Wed, 5 Oct 2011 08:20:38 -0500, Alex Elder <[email protected]> wrote:
> On Sun, 2011-09-18 at 18:54 +0400, Dmitry Monakhov wrote:
> > It is very hard to predict runtime for fsstress. In many cases it
> > is useful to give test to run a reasonable time, and then kill it.
> > But currently there is no reliable way to kill test without leaving
> > running children.
> > This patch add sanity cleanup logic which looks follow:
> > - On sigterm received by parent, it resend signal to it's children
> > - Wait for each child to terminates
> > - EXTRA_SANITY: Even if parent was killed by other signal, children
> > will be terminated with SIGKILL to preven staled children.
> >
> > So now one can simply run fsstress like this:
> > ./fsstress -p 1000 -n999999999 -d $TEST_DIR &
> > PID=$!
> > sleep 300
> > kill $PID
> > wait $PID
> >
> > Signed-off-by: Dmitry Monakhov <[email protected]>
>
> I think this is an interesting change and it looks
> OK to me. I agree with Christoph's suggestion (on
> the second patch in this series) that it would be
> nice to have at least one of the tests make use of
> it, if nothing else just to document that it's a
> reasonable thing to do.
>
> But even without that I think this is both useful
> and harmless.
>
> Reviewed-by: Alex Elder <[email protected]>
Ok i'll resend patch shortly, Actually test_case was explained inside
description. So far i've able to caught 3 different minor
fs-corruptions, one BUG_ON on ext4. And when i've run this test
on host with 24-cores it deadlock inside dcache core.
>
>
On Wed, 5 Oct 2011 08:20:40 -0500, Alex Elder <[email protected]> wrote:
> On Sun, 2011-09-18 at 18:54 +0400, Dmitry Monakhov wrote:
> > Add two new operations:
> > - getattr: ioctl(fd, EXT2_IOC_GETFLAGS, &fl)
> > - setattr: ioctl(fd, EXT2_IOC_SETFLAGS, &random_flags)
> > By default IOC_SET_SETFLAGS has zero probability because
> > it may produce inodes with APPEND or IMMUTABLE flags which
> > are not deletable by default. Let's assumes that one who
> > enable it knows how to delete such inodes.
> > For example like follows:
> > find $TEST_PATH -exec chattr -i -a {} \;
> > rm -rf $TEST_PATH
> >
> > Signed-off-by: Dmitry Monakhov <[email protected]>
>
> I have a question below. I think this is probably
> a good addition, though it should be made so it
> works for more than EXTx.
>
> If I understand the way it would be used, this will
> simply be another operation that gets randomly performed
> by fsstress while it operates, right?
>
> I have not done any testing with this yet.
>
> Reviewed-by: Alex Elder <[email protected]>
>
> . . .
>
> > @@ -1729,6 +1738,58 @@ setxattr_f(int opno, long r)
> > }
> >
> > void
> > +getattr_f(int opno, long r)
> > +{
> > +#ifdef HAVE_EXT2_INCLUDE
> > + int fd;
> > + int e;
> > + pathname_t f;
> > + uint fl;
> > + int v;
> > +
> > + init_pathname(&f);
> > + if (!get_fname(FT_ANYm, r, &f, NULL, NULL, &v))
> > + append_pathname(&f, ".");
>
> I don't understand the purpose of appending a "." to the
> end of the path. Do you intend to just use "." if
> no other file matches? (That may not be a good thing to
> do--it might not be testing the intended target.)
Yes just "." will be used. and infact i've copied that chunk from
chown_f, and similar approach is used for att_remove_f, attr_set_f
and setxattr_f operations. And in fact i'm not quite agree that they
are not independent, the only point they are connected is parent dir,
which IMHO acceptable in this case because operations result in inode
metadata changes only, w/o changing parent dir.
>
> Or are you intending to append "/." so for a directory
> its "." link gets used in the open? If so that's not
> what this does (it simply makes "a/b/x" become "a/b/x.").
> Same comments apply to setattr_f().
>
> > + fd = open_path(&f, O_RDWR);
> > + e = fd < 0 ? errno : 0;
> > + check_cwd();
> > +
> > + e = ioctl(fd, EXT2_IOC_GETFLAGS, &fl);
> > + if (v)
> > + printf("%d/%d: getattr %s %u %d\n", procid, opno, f.path, fl, e);
> > + free_pathname(&f);
> > + close(fd);
> > +#endif
> > +}
> > +
> > +void
> > +setattr_f(int opno, long r)
> > +{
>
>
On Wed, Oct 05, 2011 at 05:41:13PM +0400, Dmitry Monakhov wrote:
> Ok i'll resend patch shortly, Actually test_case was explained inside
> description. So far i've able to caught 3 different minor
> fs-corruptions, one BUG_ON on ext4. And when i've run this test
> on host with 24-cores it deadlock inside dcache core.
Having the test in the commit message doesn't really help with running
it as part of a regression test suite :)
Please report the details on the dcache deadlock to linux-fsdevel.
On Wed, Oct 05, 2011 at 08:20:40AM -0500, Alex Elder wrote:
> I have a question below. I think this is probably
> a good addition, though it should be made so it
> works for more than EXTx.
I actually works on more than extN, but at this point it requires
the ext2 headers to compiled, which isn't too nice.
Dmitry,
and updates on these patches?
On Wed, 19 Oct 2011 05:28:46 -0400, Christoph Hellwig <[email protected]> wrote:
> Dmitry,
>
> and updates on these patches?
I've posted new version here:
http://article.gmane.org/gmane.comp.file-systems.ext4/28602
Update for set flags patch now is named:
[PATCH 4/8] xfstests: fsstress add FS_IOC_{SET,GET}FLAGS operations
But still, ext4 panic in case of ordered+delalloc=>journalled switch
"-f setattr=1", i've not posted fix for that bug yet, so please run
this test carefully.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html