On Wed, 5 Feb 2014 03:16:39 +0100, Andreas Rohner wrote:
> This patch adds a flag, that enables the GC to skip the timeout in
> certain situations. For example if the cleaning of some segments
> was deferred to a later time, then no real progress has been
> made. Apart from reading a few summary blocks, no data was read or
> written to disk. In this situation it makes sense to skip the
> normal timeout once and immediately try to clean the next set of
> segments.
>
> Unfortunately it is not possible, to directly continue the cleaning
> loop, because this would lead to an unresponsive GC process.
> Therefore the timeout is simply set to 0 seconds.
>
> Signed-off-by: Andreas Rohner <[email protected]>
> ---
> sbin/cleanerd/cleanerd.c | 27 ++++++++++++++++-----------
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
> index 1374e38..e1bd558 100644
> --- a/sbin/cleanerd/cleanerd.c
> +++ b/sbin/cleanerd/cleanerd.c
> @@ -167,6 +167,7 @@ struct nilfs_cleanerd {
> int running;
> int fallback;
> int retry_cleaning;
> + int no_timeout;
Corresponding comment is missing for this one, too.
> int shutdown;
> long ncleansegs;
> struct timeval cleaning_interval;
> @@ -894,7 +895,7 @@ static int nilfs_cleanerd_recalc_interval(struct
> nilfs_cleanerd *cleanerd,
> interval = nilfs_cleanerd_cleaning_interval(cleanerd);
> /* timercmp() does not work for '>=' or '<='. */
> /* curr >= target */
> - if (!timercmp(&curr, &cleanerd->target, <)) {
> + if (!timercmp(&curr, &cleanerd->target, <) || cleanerd->no_timeout) {
> cleanerd->timeout.tv_sec = 0;
> cleanerd->timeout.tv_usec = 0;
> timeradd(&curr, interval, &cleanerd->target);
> @@ -1395,6 +1396,7 @@ static int nilfs_cleanerd_clean_segments(struct
> nilfs_cleanerd *cleanerd,
> "number: %m");
> goto out;
> }
> + cleanerd->no_timeout = 0;
This one is needed for the else case of nilfs_cleanerd_clean_loop() ?
if (ns > 0) {
ret = nilfs_cleanerd_clean_segments(
cleanerd, segnums, ns, sustat.ss_prot_seq,
prottime, &ndone);
if (ret < 0)
return -1;
} else {
cleanerd->retry_cleaning = 0;
+ cleanerd->no_timeout = 0;
}
>
> memset(&stat, 0, sizeof(stat));
> ret = nilfs_xreclaim_segment(cleanerd->nilfs, segnums, nsegs, 0,
> @@ -1409,16 +1411,18 @@ static int nilfs_cleanerd_clean_segments(struct
> nilfs_cleanerd *cleanerd,
> goto out;
> }
>
> - if (stat.cleaned_segs > 0) {
> - if (stat.deferred_segs > 0) {
> - for (i = 0; i < stat.cleaned_segs; i++)
> - syslog(LOG_DEBUG, "segment %llu deferred",
> - (unsigned long long)segnums[i]);
> - } else {
> - for (i = 0; i < stat.cleaned_segs; i++)
> - syslog(LOG_DEBUG, "segment %llu cleaned",
> - (unsigned long long)segnums[i]);
> - }
> + if (stat.deferred_segs > 0) {
> + for (i = 0; i < stat.cleaned_segs; i++)
Should be stat.deferred_segs ?
Looks like you took the meaning of stat.cleaned_segs differently.
I meant stat.cleaned_segs is decreased if stat.deferred_segs > 0.
So, the following relation will be fulfilled.
cleaned_segs + deferred_segs + protected_segs == nsegs (number of requested
segments)
> + syslog(LOG_DEBUG, "segment %llu deferred",
> + (unsigned long long)segnums[i]);
> + nilfs_cleanerd_progress(cleanerd, stat.cleaned_segs);
> + cleanerd->fallback = 0;
> + cleanerd->retry_cleaning = 0;
> + cleanerd->no_timeout = 1;
So, I think this should be set only if
stat.deferred_segs > 0 && stat.cleaned_segs == 0
Regards,
Ryusuke Konishi
> + } else if (stat.cleaned_segs > 0) {
> + for (i = 0; i < stat.cleaned_segs; i++)
> + syslog(LOG_DEBUG, "segment %llu cleaned",
> + (unsigned long long)segnums[i]);
> nilfs_cleanerd_progress(cleanerd, stat.cleaned_segs);
> cleanerd->fallback = 0;
> cleanerd->retry_cleaning = 0;
> @@ -1475,6 +1479,7 @@ static int nilfs_cleanerd_clean_loop(struct
> nilfs_cleanerd *cleanerd)
> cleanerd->running = 1;
> cleanerd->fallback = 0;
> cleanerd->retry_cleaning = 0;
> + cleanerd->no_timeout = 0;
> nilfs_cnoconv_reset(cleanerd->cnoconv);
> nilfs_gc_logger = syslog;
>
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
