Re: [PATCH 8/9] spaceman/defrag: readahead for better performance

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Jul 09, 2024 at 12:10:27PM -0700, Wengang Wang wrote:
> Reading ahead take less lock on file compared to "unshare" the file via ioctl.
> Do readahead when defrag sleeps for better defrag performace and thus more
> file IO time.
> 
> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
> ---
>  spaceman/defrag.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/spaceman/defrag.c b/spaceman/defrag.c
> index 415fe9c2..ab8508bb 100644
> --- a/spaceman/defrag.c
> +++ b/spaceman/defrag.c
> @@ -331,6 +331,18 @@ defrag_fs_limit_hit(int fd)
>  }
>  
>  static bool g_enable_first_ext_share = true;
> +static bool g_readahead = false;
> +
> +static void defrag_readahead(int defrag_fd, off64_t offset, size_t count)
> +{
> +	if (!g_readahead || g_idle_time <= 0)
> +		return;
> +
> +	if (readahead(defrag_fd, offset, count) < 0) {
> +		fprintf(stderr, "readahead failed: %s, errno=%d\n",
> +			strerror(errno), errno);

Why is it worth reporting if readahead fails?  Won't the unshare also
fail?  I'm also wondering why we wouldn't want readahead all the time?

--D

> +	}
> +}
>  
>  static int
>  defrag_get_first_real_ext(int fd, struct getbmapx *mapx)
> @@ -578,6 +590,8 @@ defrag_xfs_defrag(char *file_path) {
>  
>  		/* checks for EoF and fix up clone */
>  		stop = defrag_clone_eof(&clone);
> +		defrag_readahead(defrag_fd, seg_off, seg_size);
> +
>  		if (sleep_time_us > 0)
>  			usleep(sleep_time_us);
>  
> @@ -698,6 +712,7 @@ static void defrag_help(void)
>  " -i idle_time       -- time in ms to be idle between segments, 250ms by default\n"
>  " -n                 -- disable the \"share first extent\" featue, it's\n"
>  "                       enabled by default to speed up\n"
> +" -a                 -- do readahead to speed up defrag, disabled by default\n"
>  	));
>  }
>  
> @@ -709,7 +724,7 @@ defrag_f(int argc, char **argv)
>  	int	i;
>  	int	c;
>  
> -	while ((c = getopt(argc, argv, "s:f:ni")) != EOF) {
> +	while ((c = getopt(argc, argv, "s:f:nia")) != EOF) {
>  		switch(c) {
>  		case 's':
>  			g_segment_size_lmt = atoi(optarg) * 1024 * 1024 / 512;
> @@ -731,6 +746,10 @@ defrag_f(int argc, char **argv)
>  			g_idle_time = atoi(optarg) * 1000;
>  			break;
>  
> +		case 'a':
> +			g_readahead = true;
> +			break;
> +
>  		default:
>  			command_usage(&defrag_cmd);
>  			return 1;
> -- 
> 2.39.3 (Apple Git-146)
> 
>