unlink=1 issue on Windows

Flexible I/O Tester development
 help / color / mirror / Atom feed

* unlink=1 issue on Windows
@ 2014-03-19 16:02 Sébastien Bouchex Bellomié
  2014-03-20 14:29 ` Jens Axboe
  2014-03-20 17:59 ` unlink castor.fu
  0 siblings, 2 replies; 8+ messages in thread
From: Sébastien Bouchex Bellomié @ 2014-03-19 16:02 UTC (permalink / raw)
  To: fio@vger.kernel.org

Hi,

I'm having an issue on windows : unlink=1 is not working (temp file are still there) and it's working fine on unix

Looking at the code in close_and_free_files function :

[...]
if (td->o.unlink && f->filetype == FIO_TYPE_FILE) {
			dprint(FD_FILE, "free unlink %s\n", f->file_name);
			unlink(f->file_name);
		}
[...]

Unlink() fails because the file is still open : On Solaris, truss shows the following :

[...]
write(3, "\0\0\0\0\0 ;80\0\0\0\0\0".., 32768)   = 32768
unlink("/data/fio/random_rw.0.0")               = 0
close(3)                                        = 0
[...]

So unlink is called first.

I would put this unlinking phase AFTER the remove_file_hash call.

Seb

Sï¿½bastien BOUCHEX BELLOMIï¿½
Infovistaï¿½ Server Technical Lead, Service Assurance R&D



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: unlink=1 issue on Windows
  2014-03-19 16:02 unlink=1 issue on Windows Sébastien Bouchex Bellomié
@ 2014-03-20 14:29 ` Jens Axboe
  2014-03-20 17:59 ` unlink castor.fu
  1 sibling, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2014-03-20 14:29 UTC (permalink / raw)
  To: Sébastien Bouchex Bellomié, fio@vger.kernel.org

On 03/19/2014 10:02 AM, Sï¿½bastien Bouchex Bellomiï¿½ wrote:
> Hi,
>
> I'm having an issue on windows : unlink=1 is not working (temp file are still there) and it's working fine on unix
>
> Looking at the code in close_and_free_files function :
>
> [...]
> if (td->o.unlink && f->filetype == FIO_TYPE_FILE) {
> 			dprint(FD_FILE, "free unlink %s\n", f->file_name);
> 			unlink(f->file_name);
> 		}
> [...]
>
> Unlink() fails because the file is still open : On Solaris, truss shows the following :
>
> [...]
> write(3, "\0\0\0\0\0 ;80\0\0\0\0\0".., 32768)   = 32768
> unlink("/data/fio/random_rw.0.0")               = 0
> close(3)                                        = 0
> [...]
>
> So unlink is called first.
>
> I would put this unlinking phase AFTER the remove_file_hash call.

Sure, we can move the unlink down a bit, should not matter really. Linux 
doesn't care when you call unlink, last close will just really unlink 
it. But if other operating systems do care, then I'll just move it.

http://git.kernel.dk/?p=fio.git;a=commit;h=b5f4d8baefc6eb3e13235b7d9042b7ffecdb23dd

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: unlink
  2014-03-19 16:02 unlink=1 issue on Windows Sébastien Bouchex Bellomié
  2014-03-20 14:29 ` Jens Axboe
@ 2014-03-20 17:59 ` castor.fu
  2014-03-20 17:59   ` [PATCH 1/2] Pass -Wstrict-prototypes -Wold-style-definition, whitespace castor.fu
  2014-03-20 17:59   ` [PATCH 2/2] Add unlink hook to ioengine API castor.fu
  1 sibling, 2 replies; 8+ messages in thread
From: castor.fu @ 2014-03-20 17:59 UTC (permalink / raw)
  To: fio

I think it makes sense to include 'unlink' in the ioengine API.

The Unix semantics of being able to unlink open files goes back a long ways,
but for those environments which don't match it, the following patch gives
an opportunity to handle things differently.

I also have a patch for some stylistic issues...

-castor

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] Pass -Wstrict-prototypes -Wold-style-definition, whitespace
  2014-03-20 17:59 ` unlink castor.fu
@ 2014-03-20 17:59   ` castor.fu
  2014-03-21 15:29     ` Jens Axboe
  2014-03-20 17:59   ` [PATCH 2/2] Add unlink hook to ioengine API castor.fu
  1 sibling, 1 reply; 8+ messages in thread
From: castor.fu @ 2014-03-20 17:59 UTC (permalink / raw)
  To: fio; +Cc: Castor Fu

From: Castor Fu <castor@alumni.caltech.edu>

---
 crc/xxhash.c | 14 +++++++-------
 crc/xxhash.h |  2 +-
 filesetup.c  |  9 ++++++---
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/crc/xxhash.c b/crc/xxhash.c
index 5190330..db7890b 100644
--- a/crc/xxhash.c
+++ b/crc/xxhash.c
@@ -149,11 +149,11 @@ typedef enum { XXH_bigEndian=0, XXH_littleEndian=1 } XXH_endianess;
 typedef enum { XXH_aligned, XXH_unaligned } XXH_alignment;
 
 uint32_t XXH_readLE32_align(const uint32_t* ptr, XXH_endianess endian, XXH_alignment align)
-{ 
+{
     if (align==XXH_unaligned)
-        return endian==XXH_littleEndian ? A32(ptr) : XXH_swap32(A32(ptr)); 
+        return endian==XXH_littleEndian ? A32(ptr) : XXH_swap32(A32(ptr));
     else
-        return endian==XXH_littleEndian ? *ptr : XXH_swap32(*ptr); 
+        return endian==XXH_littleEndian ? *ptr : XXH_swap32(*ptr);
 }
 
 uint32_t XXH_readLE32(const uint32_t* ptr, XXH_endianess endian) { return XXH_readLE32_align(ptr, endian, XXH_unaligned); }
@@ -253,15 +253,15 @@ uint32_t XXH32(const void* input, int len, uint32_t seed)
 // Advanced Hash Functions
 //****************************
 
-int XXH32_sizeofState() 
+int XXH32_sizeofState(void)
 {
     XXH_STATIC_ASSERT(XXH32_SIZEOFSTATE >= sizeof(struct XXH_state32_t));   // A compilation error here means XXH32_SIZEOFSTATE is not large enough
-    return sizeof(struct XXH_state32_t); 
+    return sizeof(struct XXH_state32_t);
 }
 
 
 XXH_errorcode XXH32_resetState(void* state_in, uint32_t seed)
-{ 
+{
     struct XXH_state32_t * state = (struct XXH_state32_t *) state_in;
     state->seed = seed;
     state->v1 = seed + PRIME32_1 + PRIME32_2;
@@ -307,7 +307,7 @@ XXH_errorcode XXH32_update_endian (void* state_in, const void* input, int len, X
         {
             const uint32_t* p32 = (const uint32_t*)state->memory;
             state->v1 += XXH_readLE32(p32, endian) * PRIME32_2; state->v1 = XXH_rotl32(state->v1, 13); state->v1 *= PRIME32_1; p32++;
-            state->v2 += XXH_readLE32(p32, endian) * PRIME32_2; state->v2 = XXH_rotl32(state->v2, 13); state->v2 *= PRIME32_1; p32++; 
+            state->v2 += XXH_readLE32(p32, endian) * PRIME32_2; state->v2 = XXH_rotl32(state->v2, 13); state->v2 *= PRIME32_1; p32++;
             state->v3 += XXH_readLE32(p32, endian) * PRIME32_2; state->v3 = XXH_rotl32(state->v3, 13); state->v3 *= PRIME32_1; p32++;
             state->v4 += XXH_readLE32(p32, endian) * PRIME32_2; state->v4 = XXH_rotl32(state->v4, 13); state->v4 *= PRIME32_1; p32++;
         }
diff --git a/crc/xxhash.h b/crc/xxhash.h
index bcd060e..e80a91d 100644
--- a/crc/xxhash.h
+++ b/crc/xxhash.h
@@ -134,7 +134,7 @@ Memory will be freed by XXH32_digest().
 */
 
 
-int           XXH32_sizeofState();
+int           XXH32_sizeofState(void);
 XXH_errorcode XXH32_resetState(void* state, unsigned int seed);
 
 #define       XXH32_SIZEOFSTATE 48
diff --git a/filesetup.c b/filesetup.c
index 4bfa470..033b119 100644
--- a/filesetup.c
+++ b/filesetup.c
@@ -1103,7 +1103,8 @@ static void get_file_type(struct fio_file *f)
 	}
 }
 
-static void set_already_allocated(const char *fname) {
+static void set_already_allocated(const char *fname)
+{
 	struct file_name *fn;
 
 	fn = malloc(sizeof(struct file_name));
@@ -1129,7 +1130,8 @@ static int is_already_allocated(const char *fname)
 	return 0;
 }
 
-static void free_already_allocated() {
+static void free_already_allocated(void)
+{
 	struct flist_head *entry, *tmp;
 	struct file_name *fn;
 
@@ -1482,6 +1484,7 @@ int fio_files_done(struct thread_data *td)
 }
 
 /* free memory used in initialization phase only */
-void filesetup_mem_free() {
+void filesetup_mem_free(void)
+{
 	free_already_allocated();
 }
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] Pass -Wstrict-prototypes -Wold-style-definition, whitespace
  2014-03-20 17:59   ` [PATCH 1/2] Pass -Wstrict-prototypes -Wold-style-definition, whitespace castor.fu
@ 2014-03-21 15:29     ` Jens Axboe
  0 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2014-03-21 15:29 UTC (permalink / raw)
  To: castor.fu, fio; +Cc: Castor Fu

On 03/20/2014 11:59 AM, castor.fu@egocast.org wrote:
> From: Castor Fu <castor@alumni.caltech.edu>
>
> ---
>   crc/xxhash.c | 14 +++++++-------
>   crc/xxhash.h |  2 +-
>   filesetup.c  |  9 ++++++---
>   3 files changed, 14 insertions(+), 11 deletions(-)

Added, thanks!

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] Add unlink hook to ioengine API
  2014-03-20 17:59 ` unlink castor.fu
  2014-03-20 17:59   ` [PATCH 1/2] Pass -Wstrict-prototypes -Wold-style-definition, whitespace castor.fu
@ 2014-03-20 17:59   ` castor.fu
  2014-03-21 15:27     ` Jens Axboe
  1 sibling, 1 reply; 8+ messages in thread
From: castor.fu @ 2014-03-20 17:59 UTC (permalink / raw)
  To: fio; +Cc: Castor Fu

From: Castor Fu <castor@alumni.caltech.edu>

---
 filesetup.c | 6 +++---
 ioengine.h  | 4 +++-
 ioengines.c | 8 ++++++++
 iolog.c     | 2 +-
 4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/filesetup.c b/filesetup.c
index 033b119..eae1b99 100644
--- a/filesetup.c
+++ b/filesetup.c
@@ -58,7 +58,7 @@ static int extend_file(struct thread_data *td, struct fio_file *f)
 
 	if (unlink_file || new_layout) {
 		dprint(FD_FILE, "layout unlink %s\n", f->file_name);
-		if ((unlink(f->file_name) < 0) && (errno != ENOENT)) {
+		if ((td_io_unlink_file(td, f) < 0) && (errno != ENOENT)) {
 			td_verror(td, errno, "unlink");
 			return 1;
 		}
@@ -167,7 +167,7 @@ static int extend_file(struct thread_data *td, struct fio_file *f)
 
 	if (td->terminate) {
 		dprint(FD_FILE, "terminate unlink %s\n", f->file_name);
-		unlink(f->file_name);
+		td_io_unlink_file(td, f);
 	} else if (td->o.create_fsync) {
 		if (fsync(f->fd) < 0) {
 			td_verror(td, errno, "fsync");
@@ -1054,7 +1054,7 @@ void close_and_free_files(struct thread_data *td)
 	for_each_file(td, f, i) {
 		if (td->o.unlink && f->filetype == FIO_TYPE_FILE) {
 			dprint(FD_FILE, "free unlink %s\n", f->file_name);
-			unlink(f->file_name);
+			td_io_unlink_file(td, f);
 		}
 
 		if (fio_file_open(f))
diff --git a/ioengine.h b/ioengine.h
index 6e3c717..da9e904 100644
--- a/ioengine.h
+++ b/ioengine.h
@@ -15,7 +15,7 @@
 #include <guasi.h>
 #endif
 
-#define FIO_IOOPS_VERSION	18
+#define FIO_IOOPS_VERSION	19
 
 enum {
 	IO_U_F_FREE		= 1 << 0,
@@ -143,6 +143,7 @@ struct ioengine_ops {
 	void (*cleanup)(struct thread_data *);
 	int (*open_file)(struct thread_data *, struct fio_file *);
 	int (*close_file)(struct thread_data *, struct fio_file *);
+	int (*unlink_file)(struct thread_data *, struct fio_file *);
 	int (*get_file_size)(struct thread_data *, struct fio_file *);
 	void (*terminate)(struct thread_data *);
 	int (*io_u_init)(struct thread_data *, struct io_u *);
@@ -183,6 +184,7 @@ extern int __must_check td_io_getevents(struct thread_data *, unsigned int, unsi
 extern int __must_check td_io_commit(struct thread_data *);
 extern int __must_check td_io_open_file(struct thread_data *, struct fio_file *);
 extern int td_io_close_file(struct thread_data *, struct fio_file *);
+extern int td_io_unlink_file(struct thread_data *, struct fio_file *);
 extern int __must_check td_io_get_file_size(struct thread_data *, struct fio_file *);
 
 extern struct ioengine_ops *load_ioengine(struct thread_data *, const char *);
diff --git a/ioengines.c b/ioengines.c
index 3c75fa6..0b4983f 100644
--- a/ioengines.c
+++ b/ioengines.c
@@ -503,6 +503,14 @@ int td_io_close_file(struct thread_data *td, struct fio_file *f)
 	return put_file(td, f);
 }
 
+int td_io_unlink_file(struct thread_data *td, struct fio_file *f)
+{
+	if (td->io_ops->unlink_file)
+		return td->io_ops->unlink_file(td, f);
+	else
+		return unlink(f->file_name);
+}
+
 int td_io_get_file_size(struct thread_data *td, struct fio_file *f)
 {
 	if (!td->io_ops->get_file_size)
diff --git a/iolog.c b/iolog.c
index b8ee067..8d76fbf 100644
--- a/iolog.c
+++ b/iolog.c
@@ -96,7 +96,7 @@ static int ipo_special(struct thread_data *td, struct io_piece *ipo)
 		td_io_close_file(td, f);
 		break;
 	case FIO_LOG_UNLINK_FILE:
-		unlink(f->file_name);
+		td_io_unlink_file(td, f);
 		break;
 	default:
 		log_err("fio: bad file action %d\n", ipo->file_action);
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] Add unlink hook to ioengine API
  2014-03-20 17:59   ` [PATCH 2/2] Add unlink hook to ioengine API castor.fu
@ 2014-03-21 15:27     ` Jens Axboe
  2014-03-21 17:03       ` Castor Fu
  0 siblings, 1 reply; 8+ messages in thread
From: Jens Axboe @ 2014-03-21 15:27 UTC (permalink / raw)
  To: castor.fu, fio; +Cc: Castor Fu

On 03/20/2014 11:59 AM, castor.fu@egocast.org wrote:
> From: Castor Fu <castor@alumni.caltech.edu>
>
> ---
>   filesetup.c | 6 +++---
>   ioengine.h  | 4 +++-
>   ioengines.c | 8 ++++++++
>   iolog.c     | 2 +-
>   4 files changed, 15 insertions(+), 5 deletions(-)

This patch seems a bit pointless, without wiring some IO engine up for 
an unlink hook that isn't unlink(2).

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] Add unlink hook to ioengine API
  2014-03-21 15:27     ` Jens Axboe
@ 2014-03-21 17:03       ` Castor Fu
  0 siblings, 0 replies; 8+ messages in thread
From: Castor Fu @ 2014-03-21 17:03 UTC (permalink / raw)
  To: Jens Axboe; +Cc: fio

> This patch seems a bit pointless, without wiring some IO engine up for an
> unlink hook that isn't unlink(2).

That's true, though for the engines which currently don't touch the
local filesystem fio is doing
unlink's on the jobfile names.  So I guess on some of those engines
the unlink operator should be a no-op.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-03-21 17:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-19 16:02 unlink=1 issue on Windows Sébastien Bouchex Bellomié
2014-03-20 14:29 ` Jens Axboe
2014-03-20 17:59 ` unlink castor.fu
2014-03-20 17:59   ` [PATCH 1/2] Pass -Wstrict-prototypes -Wold-style-definition, whitespace castor.fu
2014-03-21 15:29     ` Jens Axboe
2014-03-20 17:59   ` [PATCH 2/2] Add unlink hook to ioengine API castor.fu
2014-03-21 15:27     ` Jens Axboe
2014-03-21 17:03       ` Castor Fu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox