On Aug 31, 2009 12:40 -0700, Jiaying Zhang wrote:
> > It's better to define the flag as EXT4_KEEPSIZE_FL, and to use it as
> > EXT4_KEEPSIZE_FL, but make a note of that bitfield position as being
> > reserved in include/linux/fs.h.
>
> Here is the modified patch based on your suggestions. I stick with the
> KEEPSIZE_FL approach that I think can allow us to handle the special
> truncation accordingly during fsck. Other file systems can also re-use
> this flag when they want to support fallocate with KEEP_SIZE. As you
> suggested, I moved the EXT4_KEEPSIZE_FL checking to ext4_setattr
> that now calls vmtruncate if the KEEPSIZE flag is set in the i_flag.
> Please let me know what you think about this proposed patch.
>
> --- .pc/fallocate_keepsizse.patch/fs/ext4/extents.c 2009-08-31
> 12:08:10.000000000 -0700
> +++ fs/ext4/extents.c 2009-08-31 12:12:16.000000000 -0700
> @@ -3095,7 +3095,13 @@ static void ext4_falloc_update_inode(str
> i_size_write(inode, new_size);
> if (new_size > EXT4_I(inode)->i_disksize)
> ext4_update_i_disksize(inode, new_size);
> + inode->i_flags &= ~EXT4_KEEPSIZE_FL;
Note that fallocate can be called multiple times for a file. The
EXT4_KEEPSIZE_FL should only be cleared if there were writes to
the end of the fallocated space. In that regard, I think the name
of this flag should be changed to something like "EXT4_EOFBLOCKS_FL"
to indicate that blocks are allocated beyond the end of file (i_size).
> } else {
> + /*
> + * Mark that we allocate beyond EOF so the subsequent truncate
> + * can proceed even if the new size is the same as i_size.
> + */
> + inode->i_flags |= EXT4_KEEPSIZE_FL;
Similarly, this should only be done in case the fallocate is actually
beyond i_size. While that is the most common case, it isn't necessarily
ALWAYS going to be true (e.g. if multiple threads are calling fallocate()
on a single file, or if a program always calls fallocate() on a file
without first checking if the file size is large enough).
> +++ include/linux/fs.h 2009-08-31 12:12:16.000000000 -0700
> #define FS_DIRECTIO_FL 0x00100000 /* Use direct i/o */
> +++ fs/ext4/ext4.h 2009-08-31 12:12:16.000000000 -0700
> #define EXT4_EXT_MIGRATE 0x00100000 /* Inode is migrating */
Should we redefine EXT4_EXT_MIGRATE not to conflict with FS_DIRECTIO_FL?
I don't think much, if any, use has been made of this flag, and I can
imagine a major headache in the future if this isn't changed now.
Also, EXT4_EXT_MIGRATE doesn't necessarily belong in the i_flags space,
since it is only used in-memory rather than on-disk as all of the others
are.
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
