> On Jul 9, 2024, at 2:08 PM, Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
>
> On Tue, Jul 09, 2024 at 12:10:23PM -0700, Wengang Wang wrote:
>> Add this handler to break the defrag better, so it has
>> 1. the stats reporting
>> 2. remove the temporary file
>>
>> Signed-off-by: Wengang Wang <wen.gang.wang@xxxxxxxxxx>
>> ---
>> spaceman/defrag.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/spaceman/defrag.c b/spaceman/defrag.c
>> index 9f11e36b..61e47a43 100644
>> --- a/spaceman/defrag.c
>> +++ b/spaceman/defrag.c
>> @@ -297,6 +297,13 @@ get_time_delta_us(struct timeval *pre_time, struct timeval *cur_time)
>> return us;
>> }
>>
>> +static volatile bool usedKilled = false;
>> +void defrag_sigint_handler(int dummy)
>> +{
>> + usedKilled = true;
>
> Not sure why some of these variables are camelCase and others not.
> Or why this global variable doesn't have a g_ prefix like the others?
>
Yep, will change it to g_user_killed.
>> + printf("Please wait until current segment is defragmented\n");
>
> Is it actually safe to call printf from a signal handler? Handlers must
> be very careful about what they call -- regreSSHion was a result of
> openssh not getting this right.
>
> (Granted spaceman isn't as critical...)
>
As the ioctl of UNSHARE takes time, so the process would really stop a while
After user’s kil. The message is used as a quick response to user. It’s not actually
Has any functionality. If it’s not safe, we can remove the message.
> Also would you rather SIGINT merely terminate the spaceman process? I
> think the file locks drop on termination, right?
Another purpose of the handler is that I want to show the stats like below even process is killed:
Pre-defrag 54699 extents detected, 0 are "unwritten",0 are "shared"
Tried to defragment 54697 extents (939511808 bytes) in 57 segments
Time stats(ms): max clone: 33, max unshare: 2254, max punch_hole: 286
Post-defrag 12617 extents detected
Thanks,
Winging
>
> --D
>
>> +};
>> +
>> /*
>> * defragment a file
>> * return 0 if successfully done, 1 otherwise
>> @@ -345,6 +352,8 @@ defrag_xfs_defrag(char *file_path) {
>> goto out;
>> }
>>
>> + signal(SIGINT, defrag_sigint_handler);
>> +
>> do {
>> struct timeval t_clone, t_unshare, t_punch_hole;
>> struct defrag_segment segment;
>> @@ -434,7 +443,7 @@ defrag_xfs_defrag(char *file_path) {
>> if (time_delta > max_punch_us)
>> max_punch_us = time_delta;
>>
>> - if (stop)
>> + if (stop || usedKilled)
>> break;
>> } while (true);
>> out:
>> --
>> 2.39.3 (Apple Git-146)
