Re: [PATCH 9/9] spaceman/defrag: warn on extsize

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> 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)






[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux