[Cluster-devel] [GFS2 PATCH] gfs2: Don't support fallocate on jdata files

[Cluster-devel] [GFS2 PATCH] gfs2: Don't support fallocate on jdata files

[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
Signed-off-by: Abhi Das <adas@redhat.com>
---
 fs/gfs2/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index c706c6d..8252115 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -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);
-- 
1.8.1.4

[Cluster-devel] [GFS2 PATCH] gfs2: Don't support fallocate on jdata files

[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,

Steve.

On 03/06/15 22:30, Abhi Das wrote:
> 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
> Signed-off-by: Abhi Das <adas@redhat.com>
> ---
>   fs/gfs2/file.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index c706c6d..8252115 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -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);

[Cluster-devel] [GFS2 PATCH] gfs2: Don't support fallocate on jdata files

[Andrew Price]

On 04/06/15 09:42, Steven Whitehouse wrote:
> 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?.

> On 03/06/15 22:30, Abhi Das wrote:
>> 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

>> Resolves: rhbz#1221331
>> Signed-off-by: Abhi Das <adas@redhat.com>
>> ---
>>   fs/gfs2/file.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
>> index c706c6d..8252115 100644
>> --- a/fs/gfs2/file.c
>> +++ b/fs/gfs2/file.c
>> @@ -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);
>

[Cluster-devel] [GFS2 PATCH] gfs2: Don't support fallocate on jdata files

[Steven Whitehouse]

Hi,


On 04/06/15 11:27, Andrew Price wrote:
> On 04/06/15 09:42, Steven Whitehouse wrote:
>> 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?.
>
>> On 03/06/15 22:30, Abhi Das wrote:
>>> 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
>
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.

> ? https://sourceware.org/bugzilla/show_bug.cgi?id=15661
>
>>> Resolves: rhbz#1221331
>>> Signed-off-by: Abhi Das <adas@redhat.com>
>>> ---
>>>   fs/gfs2/file.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
>>> index c706c6d..8252115 100644
>>> --- a/fs/gfs2/file.c
>>> +++ b/fs/gfs2/file.c
>>> @@ -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);
>>
>

[Cluster-devel] [GFS2 PATCH] gfs2: Don't support fallocate on jdata files

[Bob Peterson]

----- Original Message -----
> 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
> Signed-off-by: Abhi Das <adas@redhat.com>
> ---
>  fs/gfs2/file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index c706c6d..8252115 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -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);
> --
> 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