> On Jul 9, 2024, at 1:21 PM, Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
>
> On Tue, Jul 09, 2024 at 12:10:28PM -0700, Wengang Wang wrote:
>> According to current kernel implemenation, non-zero extsize might affect
>> the result of defragmentation.
>> Just print a warning on that if non-zero extsize is set on file.
>
> I'm not sure what's the point of warning vaguely about extent size
> hints? I'd have thought that would help reduce the number of extents;
> is that not the case?
Not exactly.
Same 1G file with about 54K extents,
The one with 16K extsize, after defrag, it’s extents drops to 13K.
And the one with 0 extsize, after defrag, it’s extents dropped to 22.
Above is tested with UEK6 (5.4 kernel). I can get the numbers on mainline if you want.
>
>> Signed-off-by: Wengang Wang <wen.gang.wang@xxxxxxxxxx>
>> ---
>> spaceman/defrag.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/spaceman/defrag.c b/spaceman/defrag.c
>> index ab8508bb..b6b89dd9 100644
>> --- a/spaceman/defrag.c
>> +++ b/spaceman/defrag.c
>> @@ -526,6 +526,18 @@ defrag_xfs_defrag(char *file_path) {
>> goto out;
>> }
>>
>> + if (ioctl(defrag_fd, FS_IOC_FSGETXATTR, &fsx) < 0) {
>> + fprintf(stderr, "FSGETXATTR failed %s\n",
>> + strerror(errno));
>
> Also we usually indent continuations by two tabs (not one) so that the
> continuation is more obvious:
>
> fprintf(stderr, "FSGETXATTR failed %s\n",
> strerror(errno));
OK.
>
>> + ret = 1;
>> + goto out;
>> + }
>> +
>> + if (fsx.fsx_extsize != 0)
>> + fprintf(stderr, "%s has extsize set %d. That might affect defrag "
>> + "according to kernel implementation\n",
>
> Format strings in userspace printf calls should be wrapped so that
> gettext can provide translated versions:
>
> fprintf(stderr, _("%s has extsize...\n"), file_path...);
>
> (I know, xfsprogs isn't as consistent as it probably ought to be...)
>
OK.
Thanks,
Wengang
> --D
>
>> + file_path, fsx.fsx_extsize);
>> +
>> clone.src_fd = defrag_fd;
>>
>> defrag_dir = dirname(file_path);
>> --
>> 2.39.3 (Apple Git-146)
