Skip to contentSkip to conversation

gfs2: Don't support fallocate on jdata files

PATCH New Abhi Das 4 participants 5 messages Jun 3, 2015 – Jun 9, 2015 GFS2 file system

lore
Share & tools
lore

Abhi Das

author
#0 lore raw

PATCH 1/1 gfs2: Don't support fallocate on jdata files

We cannot provide an efficient implementation due to the headers on the data blocks, so there doesn't seem much point in having it.

Resolves: rhbz#1221331

---
 fs/gfs2/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/gfs2/file.c +1 -1
index c706c6d..8252115 100644
@@ -917,7 +917,7 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t le
 	struct gfs2_holder gh;
 	int ret;
 
-	if (mode & ~FALLOC_FL_KEEP_SIZE)
+	if ((mode & ~FALLOC_FL_KEEP_SIZE) || gfs2_is_jdata(ip))
 		return -EOPNOTSUPP;
 
 	mutex_lock(&inode->i_mutex);
Signature · 1 line
1.8.1.4
#1 lore raw

PATCH 1/1 gfs2: Don't support fallocate on jdata files

Hi,

Will glibc do the fallback path, or just return this as an error? I think thats worth checking as it would be nice it it would transparently fall back in this case,

Steve.

Quoted 22 lines · Abhi Das

We cannot provide an efficient implementation due to the headers on the data blocks, so there doesn't seem much point in having it.

Resolves: rhbz#1221331

---
  fs/gfs2/file.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
fs/gfs2/file.c +1 -1
index c706c6d..8252115 100644
@@ -917,7 +917,7 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t le
  	struct gfs2_holder gh;
  	int ret;
 
-	if (mode & ~FALLOC_FL_KEEP_SIZE)
+	if ((mode & ~FALLOC_FL_KEEP_SIZE) || gfs2_is_jdata(ip))
  		return -EOPNOTSUPP;
 
  	mutex_lock(&inode->i_mutex);
#2 lore raw

PATCH 1/1 gfs2: Don't support fallocate on jdata files

Quoted 5 lines · Steven Whitehouse

Hi,

Will glibc do the fallback path, or just return this as an error? I think thats worth checking as it would be nice it it would transparently fall back in this case,

You only get the fallback with posix_fallocate() but many applications will use fallocate() directly for various reasons?.

Quoted 3 lines · Abhi Das
Quoted 2 lines

We cannot provide an efficient implementation due to the headers on the data blocks, so there doesn't seem much point in having it.

I'm not sure I like the idea that fallocate() could work in one directory and fail in another... What exactly is the issue here? Is it just the journal space required or that writing the data block headers would be too slow?

Andy

? https://sourceware.org/bugzilla/show_bug.cgi?id=15661

Quoted 19 lines
Quoted 18 lines

Resolves: rhbz#1221331

---
  fs/gfs2/file.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
fs/gfs2/file.c +0 -0
index c706c6d..8252115 100644
@@ -917,7 +917,7 @@ static long gfs2_fallocate(struct file *file, int

mode, loff_t offset, loff_t le

      struct gfs2_holder gh;
      int ret;
fs/gfs2/file.c +1 -1
-    if (mode & ~FALLOC_FL_KEEP_SIZE)
+    if ((mode & ~FALLOC_FL_KEEP_SIZE) || gfs2_is_jdata(ip))
          return -EOPNOTSUPP;
      mutex_lock(&inode->i_mutex);
#3 lore raw

PATCH 1/1 gfs2: Don't support fallocate on jdata files

Hi,

Quoted 14 lines · Andrew Price
Quoted 5 lines · Steven Whitehouse

Hi,

Will glibc do the fallback path, or just return this as an error? I think thats worth checking as it would be nice it it would transparently fall back in this case,

You only get the fallback with posix_fallocate() but many applications will use fallocate() directly for various reasons?.

Quoted 3 lines · Abhi Das
Quoted 2 lines

We cannot provide an efficient implementation due to the headers on the data blocks, so there doesn't seem much point in having it.

Quoted 7 lines

I'm not sure I like the idea that fallocate() could work in one directory and fail in another... What exactly is the issue here? Is it just the journal space required or that writing the data block headers would be too slow?

Andy

The issue is that data blocks for journaled data have metadata headers on them. Although I guess that would not affect the normal journaled data. but it would affect the journaled data used by the various kernel internal files (i.e. on the meta fs).

The idea of fallocate is to be able to allocate the zero the blocks efficiently, but if they have to go through the journal anyway, then it is going to be really slow, so not a lot of point in doing it. We should definitely try and encourage applications to use posix_fallocate where possible, so that the fallback will work, although I know that we cannot cover all situations.

The reality is that virtually nobody uses jdata files anyway, so this is not likely to affect anybody, otherwise they'd have reported this as not working before now,

Steve.

Quoted 22 lines

? https://sourceware.org/bugzilla/show_bug.cgi?id=15661

Quoted 19 lines
Quoted 18 lines

Resolves: rhbz#1221331

---
  fs/gfs2/file.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
fs/gfs2/file.c +0 -0
index c706c6d..8252115 100644
@@ -917,7 +917,7 @@ static long gfs2_fallocate(struct file *file, int

mode, loff_t offset, loff_t le

      struct gfs2_holder gh;
      int ret;
fs/gfs2/file.c +1 -1
-    if (mode & ~FALLOC_FL_KEEP_SIZE)
+    if ((mode & ~FALLOC_FL_KEEP_SIZE) || gfs2_is_jdata(ip))
          return -EOPNOTSUPP;
      mutex_lock(&inode->i_mutex);
#4 lore raw

PATCH 1/1 gfs2: Don't support fallocate on jdata files

In reply to Abhi Das

----- Original Message -----

Quoted 24 lines

We cannot provide an efficient implementation due to the headers on the data blocks, so there doesn't seem much point in having it.

Resolves: rhbz#1221331

---
 fs/gfs2/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/gfs2/file.c +0 -0
index c706c6d..8252115 100644
@@ -917,7 +917,7 @@ static long gfs2_fallocate(struct file *file, int mode,

loff_t offset, loff_t le

fs/gfs2/file.c +1 -2
 	struct gfs2_holder gh;
 	int ret;
 
-	if (mode & ~FALLOC_FL_KEEP_SIZE)
+	if ((mode & ~FALLOC_FL_KEEP_SIZE) || gfs2_is_jdata(ip))
 		return -EOPNOTSUPP;
 
 	mutex_lock(&inode->i_mutex);
--
Quoted 3 lines

1.8.1.4

Hi,

Now that all the questions have been answered, I pushed this patch to the for-next branch of the linux-gfs2 tree:
https://git.kernel.org/cgit/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next&id=86066914edff2316cbed63aac8a87d5001441a16

Regards,

Bob Peterson
Red Hat File Systems