2015-05-19 15:27:40

by Josef Bacik

[permalink] [raw]
Subject: [PATCH] tmpfs: truncate at i_size

If we fallocate past i_size with KEEP_SIZE, extend the file to use some but not
all of this space, and then truncate(i_size) we won't trim the excess
preallocated space. We decided at LSF that we want to truncate the fallocated
bit past i_size when we truncate to i_size, which is what this patch does.
Thanks,

Signed-off-by: Josef Bacik <[email protected]>
---
mm/shmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index de98137..089afde 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -569,7 +569,7 @@ static int shmem_setattr(struct dentry *dentry, struct iattr *attr)
i_size_write(inode, newsize);
inode->i_ctime = inode->i_mtime = CURRENT_TIME;
}
- if (newsize < oldsize) {
+ if (newsize <= oldsize) {
loff_t holebegin = round_up(newsize, PAGE_SIZE);
unmap_mapping_range(inode->i_mapping, holebegin, 0, 1);
shmem_truncate_range(inode, newsize, (loff_t)-1);
--
1.8.3.1


2015-06-16 20:02:47

by Hugh Dickins

[permalink] [raw]
Subject: Re: [PATCH] tmpfs: truncate at i_size

On Tue, 19 May 2015, Josef Bacik wrote:

> If we fallocate past i_size with KEEP_SIZE, extend the file to use some but not
> all of this space, and then truncate(i_size) we won't trim the excess
> preallocated space. We decided at LSF that we want to truncate the fallocated
> bit past i_size when we truncate to i_size, which is what this patch does.
> Thanks,
>
> Signed-off-by: Josef Bacik <[email protected]>

Sorry for the delay, it's been on my mind but only now I get to it.
Yes, that was agreed at LSF, and I've checked that indeed tmpfs is
out of line here: thank you for fixing it. But I do prefer your
original more explicit description, so I'll send the patch to akpm
now for v4.2, with that description instead (plus a reference to LSF).

Thanks,
Hugh

> ---
> mm/shmem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index de98137..089afde 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -569,7 +569,7 @@ static int shmem_setattr(struct dentry *dentry, struct iattr *attr)
> i_size_write(inode, newsize);
> inode->i_ctime = inode->i_mtime = CURRENT_TIME;
> }
> - if (newsize < oldsize) {
> + if (newsize <= oldsize) {
> loff_t holebegin = round_up(newsize, PAGE_SIZE);
> unmap_mapping_range(inode->i_mapping, holebegin, 0, 1);
> shmem_truncate_range(inode, newsize, (loff_t)-1);
> --
> 1.8.3.1

2015-06-16 20:10:12

by Hugh Dickins

[permalink] [raw]
Subject: [PATCH] tmpfs: truncate prealloc blocks past i_size

From: Josef Bacik <[email protected]>

One of the rocksdb people noticed that when you do something like this

fallocate(fd, FALLOC_FL_KEEP_SIZE, 0, 10M)
pwrite(fd, buf, 5M, 0)
ftruncate(5M)

on tmpfs, the file would still take up 10M: which led to super fun issues
because we were getting ENOSPC before we thought we should be getting
ENOSPC. This patch fixes the problem, and mirrors what all the other
fs'es do (and was agreed to be the correct behaviour at LSF).

I tested it locally to make sure it worked properly with the following

xfs_io -f -c "falloc -k 0 10M" -c "pwrite 0 5M" -c "truncate 5M" file

Without the patch we have "Blocks: 20480", with the patch we have the
correct value of "Blocks: 10240".

Signed-off-by: Josef Bacik <[email protected]>
Signed-off-by: Hugh Dickins <[email protected]>
---
mm/shmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -569,7 +569,7 @@ static int shmem_setattr(struct dentry *dentry, struct iattr *attr)
i_size_write(inode, newsize);
inode->i_ctime = inode->i_mtime = CURRENT_TIME;
}
- if (newsize < oldsize) {
+ if (newsize <= oldsize) {
loff_t holebegin = round_up(newsize, PAGE_SIZE);
unmap_mapping_range(inode->i_mapping, holebegin, 0, 1);
shmem_truncate_range(inode, newsize, (loff_t)-1);
--
1.8.3.1

2015-06-16 20:08:38

by Josef Bacik

[permalink] [raw]
Subject: Re: [PATCH] tmpfs: truncate at i_size

On 06/16/2015 01:02 PM, Hugh Dickins wrote:
> On Tue, 19 May 2015, Josef Bacik wrote:
>
>> If we fallocate past i_size with KEEP_SIZE, extend the file to use some but not
>> all of this space, and then truncate(i_size) we won't trim the excess
>> preallocated space. We decided at LSF that we want to truncate the fallocated
>> bit past i_size when we truncate to i_size, which is what this patch does.
>> Thanks,
>>
>> Signed-off-by: Josef Bacik <[email protected]>
>
> Sorry for the delay, it's been on my mind but only now I get to it.
> Yes, that was agreed at LSF, and I've checked that indeed tmpfs is
> out of line here: thank you for fixing it. But I do prefer your
> original more explicit description, so I'll send the patch to akpm
> now for v4.2, with that description instead (plus a reference to LSF).
>

Sounds good, thanks Hugh.

Josef