[LTP] [RFC PATCH] tst_checkpoint_destroy

[LTP] [RFC PATCH] tst_checkpoint_destroy

[Stanislav Kholmanskikh]

Hi.

As of now, if we run test cases which use the tst_checkpoint framework on NFS,
they will fail with errors like:

creat07     0  TWARN  :  tst_tmpdir.c:206: tst_rmdir: rmobj(/tmpdir/ltp-0IVQPP0NK6/creigi0kV) failed: remove(/tmpdir/ltp-0IVQPP0NK6/creigi0kV) failed; errno=66: Directory not empty

This happens because we keep the futex page mapped while tst_rmdir() is running.

So on we need to unmap the page before tst_rmdir().

What do you think about the proposed solution?

Thanks.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 include/tst_checkpoint.h                  |    9 +++++++++
 lib/tst_checkpoint.c                      |   19 ++++++++++++++++++-
 testcases/kernel/syscalls/creat/creat07.c |    2 ++
 3 files changed, 29 insertions(+), 1 deletions(-)

diff --git a/include/tst_checkpoint.h b/include/tst_checkpoint.h
index e6b8100..95a82b4 100644
--- a/include/tst_checkpoint.h
+++ b/include/tst_checkpoint.h
@@ -48,7 +48,16 @@
 void tst_checkpoint_init(const char *file, const int lineno,
 			 void (*cleanup_fn)(void));
 
+/*
+ * Destroy the checkpoint.
+ *
+ * Call it once you are done with the syncronization.
+ */
+#define TST_CHECKPOINT_DESTROY(cleanup_fn) \
+	tst_checkpoint_destroy(__FILE__, __LINE__, cleanup_fn)
 
+void tst_checkpoint_destroy(const char *file, const int lineno,
+			    void (*cleanup_fn)(void));
 
 /*
  * Waits for wakeup.
diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
index 79e49ac..2957f33 100644
--- a/lib/tst_checkpoint.c
+++ b/lib/tst_checkpoint.c
@@ -31,6 +31,7 @@
 #include "safe_macros.h"
 
 #define DEFAULT_MSEC_TIMEOUT 10000
+#define CHECKPOINT_BASE_FILE "checkpoint_futex_base_file"
 
 typedef volatile uint32_t futex_t;
 
@@ -67,7 +68,7 @@ void tst_checkpoint_init(const char *file, const int lineno,
 
 	page_size = getpagesize();
 
-	fd = SAFE_OPEN(cleanup_fn, "checkpoint_futex_base_file",
+	fd = SAFE_OPEN(cleanup_fn, CHECKPOINT_BASE_FILE,
 	               O_RDWR | O_CREAT, 0666);
 
 	SAFE_FTRUNCATE(cleanup_fn, fd, page_size);
@@ -78,6 +79,22 @@ void tst_checkpoint_init(const char *file, const int lineno,
 	SAFE_CLOSE(cleanup_fn, fd);
 }
 
+void tst_checkpoint_destroy(const char *file, const int lineno,
+			    void (*cleanup_fn)(void))
+{
+	if (!futexes || !page_size) {
+		tst_brkm(TBROK, cleanup_fn,
+			 "%s: %d checkpoints are not initialized",
+			 file, lineno);
+	}
+
+	SAFE_MUNMAP(cleanup_fn, (void *)futexes, page_size);
+
+	futexes = NULL;
+
+	SAFE_UNLINK(cleanup_fn, CHECKPOINT_BASE_FILE);
+}
+
 int tst_checkpoint_wait(unsigned int id, unsigned int msec_timeout)
 {
 	struct timespec timeout;
diff --git a/testcases/kernel/syscalls/creat/creat07.c b/testcases/kernel/syscalls/creat/creat07.c
index c3cbd37..7b67d73 100644
--- a/testcases/kernel/syscalls/creat/creat07.c
+++ b/testcases/kernel/syscalls/creat/creat07.c
@@ -106,5 +106,7 @@ static void setup(void)
 
 static void cleanup(void)
 {
+	TST_CHECKPOINT_DESTROY(NULL);
+
 	tst_rmdir();
 }
-- 
1.7.1


------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [RFC PATCH] tst_checkpoint_destroy

[Cyril Hrubis]

Hi!
> As of now, if we run test cases which use the tst_checkpoint framework on NFS,
> they will fail with errors like:
> 
> creat07     0  TWARN  :  tst_tmpdir.c:206: tst_rmdir: rmobj(/tmpdir/ltp-0IVQPP0NK6/creigi0kV) failed: remove(/tmpdir/ltp-0IVQPP0NK6/creigi0kV) failed; errno=66: Directory not empty
> 
> This happens because we keep the futex page mapped while tst_rmdir() is running.
> 
> So on we need to unmap the page before tst_rmdir().
> 
> What do you think about the proposed solution?

Why don't we hide this from user and unmap() the file in the tst_rmdir()
if checkpoint was initialized?

Otherwise I'm all for this fix.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [RFC PATCH] tst_checkpoint_destroy

[Cyril Hrubis]

Hi!
> Ok, we can do it this way.
> 
> One question regarding the typedef of futex_t. As of now the futex_t 
> type is defined in two places:
>   * testcases/kernel/syscalls/futex/futextest.h
>   * lib/tst_checkpoint.h
> 
> If I put unmap() in tst_rmdir(), I will add futex_t to tst_tmpdir.c 
> either, so it will be the third occurrence.
> 
> It seems it would be good to move the typedef of futex_t into a common 
> header.
> 
> I'm unsure which one to use :) Or just create include/lapi/futex.h?

Header in lapi sounds good, I should have probably done so when I was
rewriting the checkpoint interface.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH 1/2] Move futex_t into a common header

[Stanislav Kholmanskikh]

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 include/lapi/futex.h                        |   24 ++++++++++++++++++++++++
 lib/tst_checkpoint.c                        |    3 +--
 testcases/kernel/syscalls/futex/futextest.h |    2 +-
 3 files changed, 26 insertions(+), 3 deletions(-)
 create mode 100644 include/lapi/futex.h

diff --git a/include/lapi/futex.h b/include/lapi/futex.h
new file mode 100644
index 0000000..ac4036b
--- /dev/null
+++ b/include/lapi/futex.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2015 Linux Test Project
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef LAPI_FUTEX_H__
+#define LAPI_FUTEX_H__
+
+typedef volatile uint32_t futex_t;
+
+#endif /* LAPI_FUTEX_H__ */
diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
index 79e49ac..0e2c402 100644
--- a/lib/tst_checkpoint.c
+++ b/lib/tst_checkpoint.c
@@ -29,11 +29,10 @@
 
 #include "test.h"
 #include "safe_macros.h"
+#include "lapi/futex.h"
 
 #define DEFAULT_MSEC_TIMEOUT 10000
 
-typedef volatile uint32_t futex_t;
-
 static futex_t *futexes;
 static int page_size;
 
diff --git a/testcases/kernel/syscalls/futex/futextest.h b/testcases/kernel/syscalls/futex/futextest.h
index 1e9713d..5754d36 100644
--- a/testcases/kernel/syscalls/futex/futextest.h
+++ b/testcases/kernel/syscalls/futex/futextest.h
@@ -38,8 +38,8 @@
 #include <sys/syscall.h>
 #include <sys/types.h>
 #include <linux/futex.h>
+#include "lapi/futex.h"
 
-typedef volatile u_int32_t futex_t;
 #define FUTEX_INITIALIZER 0
 
 #ifndef FUTEX_CMP_REQUEUE
-- 
1.7.1


------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH 2/2] Unmap the futexes backend file in tst_rmdir

[Stanislav Kholmanskikh]

As of now, test cases utilizing tst_checkpoint framework fail on NFS
with similar errors:

creat07     0  TWARN  :  tst_tmpdir.c:206: tst_rmdir: rmobj(/tmpdir/ltp-0IVQPP0NK6/creigi0kV) failed: remove(/tmpdir/ltp-0IVQPP0NK6/creigi0kV) failed; errno=66: Directory not empty

This happens because we keep the futex page mapped while tst_rmdir() is running.

Let's unmap it before the actual delete procedure.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 lib/tst_checkpoint.c |    2 +-
 lib/tst_tmpdir.c     |   11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
index 0e2c402..36eeff2 100644
--- a/lib/tst_checkpoint.c
+++ b/lib/tst_checkpoint.c
@@ -33,7 +33,7 @@
 
 #define DEFAULT_MSEC_TIMEOUT 10000
 
-static futex_t *futexes;
+futex_t *futexes;
 static int page_size;
 
 void tst_checkpoint_init(const char *file, const int lineno,
diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
index dd82f35..6a6c0c0 100644
--- a/lib/tst_tmpdir.c
+++ b/lib/tst_tmpdir.c
@@ -57,6 +57,7 @@
  *
  *********************************************************/
 
+#include <sys/mman.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <assert.h>
@@ -70,6 +71,7 @@
 #include "test.h"
 #include "rmobj.h"
 #include "ltp_priv.h"
+#include "lapi/futex.h"
 
 /*
  * Define some useful macros.
@@ -97,6 +99,8 @@ static char *TESTDIR = NULL;	/* the directory created */
 
 static char test_start_work_dir[PATH_MAX];
 
+/* lib/tst_checkpoint.c */
+extern futex_t *futexes;
 
 int tst_tmpdir_created(void)
 {
@@ -199,6 +203,13 @@ void tst_rmdir(void)
 	}
 
 	/*
+	 * Unmap the backend file.
+	 * This is needed to overcome the NFS "silly rename" feature.
+	 */
+	if (futexes)
+		munmap((void *)futexes, getpagesize());
+
+	/*
 	 * Attempt to remove the "TESTDIR" directory, using rmobj().
 	 */
 	if (rmobj(TESTDIR, &errmsg) == -1) {
-- 
1.7.1


------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 2/2] Unmap the futexes backend file in tst_rmdir

[Cyril Hrubis]

Hi!
> As of now, test cases utilizing tst_checkpoint framework fail on NFS
> with similar errors:
> 
> creat07     0  TWARN  :  tst_tmpdir.c:206: tst_rmdir: rmobj(/tmpdir/ltp-0IVQPP0NK6/creigi0kV) failed: remove(/tmpdir/ltp-0IVQPP0NK6/creigi0kV) failed; errno=66: Directory not empty
> 
> This happens because we keep the futex page mapped while tst_rmdir() is running.
> 
> Let's unmap it before the actual delete procedure.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  lib/tst_checkpoint.c |    2 +-
>  lib/tst_tmpdir.c     |   11 +++++++++++
>  2 files changed, 12 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
> index 0e2c402..36eeff2 100644
> --- a/lib/tst_checkpoint.c
> +++ b/lib/tst_checkpoint.c
> @@ -33,7 +33,7 @@
>  
>  #define DEFAULT_MSEC_TIMEOUT 10000
>  
> -static futex_t *futexes;
> +futex_t *futexes;
>  static int page_size;

Now that futexes is a global symbol we should prefix it with tst_ to
avoid clashes with testcase defined symbols.

Otherwise it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH V2 2/2] Unmap the futexes backend file in tst_rmdir

[Stanislav Kholmanskikh]

As of now, test cases utilizing tst_checkpoint framework fail on NFS
with similar errors:

creat07     0  TWARN  :  tst_tmpdir.c:206: tst_rmdir: rmobj(/tmpdir/ltp-0IVQPP0NK6/creigi0kV) failed: remove(/tmpdir/ltp-0IVQPP0NK6/creigi0kV) failed; errno=66: Directory not empty

This happens because we keep the futex page mapped while tst_rmdir() is running.

Let's unmap it before the actual delete procedure.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
Changes since V1:
 * renamed 'futex_t *futexes' to 'futex_t *tst_futexes'


 lib/tst_checkpoint.c |   12 +++++++-----
 lib/tst_tmpdir.c     |   11 +++++++++++
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
index 0e2c402..a2c9563 100644
--- a/lib/tst_checkpoint.c
+++ b/lib/tst_checkpoint.c
@@ -33,7 +33,7 @@
 
 #define DEFAULT_MSEC_TIMEOUT 10000
 
-static futex_t *futexes;
+futex_t *tst_futexes;
 static int page_size;
 
 void tst_checkpoint_init(const char *file, const int lineno,
@@ -41,7 +41,7 @@ void tst_checkpoint_init(const char *file, const int lineno,
 {
 	int fd;
 
-	if (futexes) {
+	if (tst_futexes) {
 		tst_brkm(TBROK, cleanup_fn,
 		         "%s: %d checkopoints allready initialized",
 		         file, lineno);
@@ -71,7 +71,7 @@ void tst_checkpoint_init(const char *file, const int lineno,
 
 	SAFE_FTRUNCATE(cleanup_fn, fd, page_size);
 
-	futexes = SAFE_MMAP(cleanup_fn, NULL, page_size,
+	tst_futexes = SAFE_MMAP(cleanup_fn, NULL, page_size,
 	                    PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
 
 	SAFE_CLOSE(cleanup_fn, fd);
@@ -89,7 +89,8 @@ int tst_checkpoint_wait(unsigned int id, unsigned int msec_timeout)
 	timeout.tv_sec = msec_timeout/1000;
 	timeout.tv_nsec = (msec_timeout%1000) * 1000000;
 
-	return syscall(SYS_futex, &futexes[id], FUTEX_WAIT, futexes[id], &timeout);
+	return syscall(SYS_futex, &tst_futexes[id], FUTEX_WAIT,
+		       tst_futexes[id], &timeout);
 }
 
 int tst_checkpoint_wake(unsigned int id, unsigned int nr_wake,
@@ -103,7 +104,8 @@ int tst_checkpoint_wake(unsigned int id, unsigned int nr_wake,
 	}
 
 	do {
-		waked += syscall(SYS_futex, &futexes[id], FUTEX_WAKE, INT_MAX, NULL);
+		waked += syscall(SYS_futex, &tst_futexes[id], FUTEX_WAKE,
+				 INT_MAX, NULL);
 		usleep(1000);
 		msecs++;
 
diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
index dd82f35..f4f3b30 100644
--- a/lib/tst_tmpdir.c
+++ b/lib/tst_tmpdir.c
@@ -57,6 +57,7 @@
  *
  *********************************************************/
 
+#include <sys/mman.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <assert.h>
@@ -70,6 +71,7 @@
 #include "test.h"
 #include "rmobj.h"
 #include "ltp_priv.h"
+#include "lapi/futex.h"
 
 /*
  * Define some useful macros.
@@ -97,6 +99,8 @@ static char *TESTDIR = NULL;	/* the directory created */
 
 static char test_start_work_dir[PATH_MAX];
 
+/* lib/tst_checkpoint.c */
+extern futex_t *tst_futexes;
 
 int tst_tmpdir_created(void)
 {
@@ -199,6 +203,13 @@ void tst_rmdir(void)
 	}
 
 	/*
+	 * Unmap the backend file.
+	 * This is needed to overcome the NFS "silly rename" feature.
+	 */
+	if (tst_futexes)
+		munmap((void *)tst_futexes, getpagesize());
+
+	/*
 	 * Attempt to remove the "TESTDIR" directory, using rmobj().
 	 */
 	if (rmobj(TESTDIR, &errmsg) == -1) {
-- 
1.7.1


------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V2 2/2] Unmap the futexes backend file in tst_rmdir

[Stanislav Kholmanskikh]

On 05/26/2015 01:21 PM, Stanislav Kholmanskikh wrote:
> As of now, test cases utilizing tst_checkpoint framework fail on NFS
> with similar errors:
>
> creat07     0  TWARN  :  tst_tmpdir.c:206: tst_rmdir: rmobj(/tmpdir/ltp-0IVQPP0NK6/creigi0kV) failed: remove(/tmpdir/ltp-0IVQPP0NK6/creigi0kV) failed; errno=66: Directory not empty
>
> This happens because we keep the futex page mapped while tst_rmdir() is running.
>
> Let's unmap it before the actual delete procedure.
>
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
> Changes since V1:
>   * renamed 'futex_t *futexes' to 'futex_t *tst_futexes'

Pushed this patch and "[PATCH 1/2] Move futex_t into a common header".

Thanks.

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list