On Wed, 29 Oct 2014, Josef Bacik wrote:
> 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 lead 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. 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". Thanks,
>
> Signed-off-by: Josef Bacik <jbacik@xxxxxx>
That is a very good catch, and thank you for the patch. But I am not
convinced that the patch is correct - even if it does happen to end
up doing what other filesystems do here (I haven't checked).
Your patch makes it look like a fix to an off-by-one, but that is
not really the case. What if you change your final ftruncate(5M)
to ftruncate(6M): what should happen then?
My intuition says that what should happen is that i_size is set to 6M,
and the fallocated excess blocks beyond 6M be trimmed off: so that
it's both an extending and a shrinking truncate at the same time.
And I think that behavior would be served by removing the
"if (newsize < oldsize)" condition completely.
But perhaps I'm wrong: can you or anyone shed more light on this,
or point to documentation of what should happen in these cases?
Thanks,
Hugh
> ---
> mm/shmem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 185836b..79b7fb5 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -574,7 +574,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
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
