Hi Mingming,
> Overall I think the header changes probably should go with the patches
> that need those changes, that helps explain why we need this header
> changes and makes the patch compile itself. Right now this patch failed
> to compile alone.
I see.
I will reorganize my patches into meaningful groups.
>> /*
>> @@ -299,6 +300,14 @@ struct ext4_new_group_data {
>> #define EXT4_IOC_GETRSVSZ _IOR('f', 5, long)
>> #define EXT4_IOC_SETRSVSZ _IOW('f', 6, long)
>> #define EXT4_IOC_MIGRATE _IO('f', 7)
>> +#define EXT4_IOC_FIBMAP _IOW('f', 9, ext4_fsblk_t)
>> +#define EXT4_IOC_DEFRAG _IOW('f', 10, struct ext4_ext_defrag_data)
>> +#define EXT4_IOC_GROUP_INFO _IOW('f', 11, struct ext4_group_data_info)
>> +#define EXT4_IOC_FREE_BLOCKS_INFO _IOW('f', 12, struct ext4_extents_info)
>> +#define EXT4_IOC_EXTENTS_INFO _IOW('f', 13, struct ext4_extents_info)
>> +#define EXT4_IOC_RESERVE_BLOCK _IOW('f', 14, struct ext4_extents_info)
>> +#define EXT4_IOC_MOVE_VICTIM _IOW('f', 15, struct ext4_extents_info)
>> +#define EXT4_IOC_BLOCK_RELEASE _IO('f', 8)
>
> These should go with the last patch in this series, where the ioctl
> commands get implemented
OK.
> Could you add more comments about the tunables below? What they are used
> for?
>
>> +/* Used for defrag */
>> +#define DEFRAG_MAX_ENT 32
>
> This means at most 32 free space extents per block group?
OK. I will add more comments later.
This means the maximum count of extents(or free space extents) for exchanging
between kernel-space and user-space at once.
For example, EXT4_IOC_EXTENTS_INFO is called multiple times(per DEFRAG_MAX_ENT)
to get its block distribution if there is a large number of
extents(or free space extents) in the target block group.
>> +#define DEFRAG_FORCE_TRY 1
>> +#define DEFRAG_FORCE_VICTIM 2
>> +#define DEFRAG_FORCE_GATHER 3
>> +
>
> And these tunables are used in the last patch in this series, so it make
> sense to move there too.
OK.
>> +struct ext4_extent_data {
>> + ext4_lblk_t block; /* start logical block number */
>> + ext4_fsblk_t start; /* start physical block number */
>> + int len; /* blocks count */
>> +};
>> +
>
> Not related to defrag, but I would like to consider this as in-core
> extent structure. Maybe we should use this structure in other extents.c,
> instead of sharing the same on-disk extent structure, which needs to
> worry about little endian?
I agree.
How about renaming this structure from ext4_extent_data to
ext4_extent_info which used the extent in-core structure
and put it into ext4_fs_extents.h?
Because in-core super_block is ext4_sb_info.
/*
* This is the extent in-core structure.
*/
struct ext4_extent_info {
ext4_lblk_t ee_block; /* first logical block extent covers */
int ee_len; /* number of blocks covered by extent */
ext4_fsblk_t ee_start; /* first physical block extent covers */
};
Regards, Akira
--
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
