Hi,
In order to avoid working both at the same thing, do you think about
implementing this yourself in near future or do you prefer that I try
implementing it myself and send you a patch ?
For the case where you prefer that I try implementing it, here a quick
information in natural language how I would implement this in order to
see if I am thinking it correctly (line beginning with *** are changed):
1166 /**
1167 * nilfs_cleanerd_clean_loop - main loop of the cleaner daemon
1168 * @cleanerd: cleanerd object
1169 */
1170 static int nilfs_cleanerd_clean_loop(struct nilfs_cleanerd *cleanerd)
1171 {
1172 struct nilfs_sustat sustat;
1173 __u64 prev_nongc_ctime = 0, prottime = 0, oldest = 0;
1174 __u64 segnums[NILFS_CLDCONFIG_NSEGMENTS_PER_CLEAN_MAX];
*** _u64 previousfirstsegnum = max value of _u64;
1175 struct timespec timeout;
1176 sigset_t sigset;
1177 int ns, ret;
1178
1179 sigemptyset(&sigset);
1180 if (sigprocmask(SIG_SETMASK, &sigset, NULL) < 0) {
1181 syslog(LOG_ERR, "cannot set signal mask: %m");
1182 return -1;
1183 }
1184 sigaddset(&sigset, SIGHUP);
1185
1186 if (set_sigterm_handler() < 0) {
1187 syslog(LOG_ERR, "cannot set SIGTERM signal handler:
%m");
1188 return -1;
1189 }
1190 if (set_sighup_handler() < 0) {
1191 syslog(LOG_ERR, "cannot set SIGHUP signal handler:
%m");
1192 return -1;
1193 }
1194
1195 nilfs_cleanerd_reload_config = 0;
1196
1197 ret = nilfs_cleanerd_init_interval(cleanerd);
1198 if (ret < 0)
1199 return -1;
1200
1201 cleanerd->c_running = 1;
1202 cleanerd->c_ncleansegs =
cleanerd->c_config.cf_nsegments_per_clean;
1203
1204 while (1) {
1205 if (sigprocmask(SIG_BLOCK, &sigset, NULL) < 0) {
1206 syslog(LOG_ERR, "cannot set signal mask: %m");
1207 return -1;
1208 }
1209
1210 if (nilfs_cleanerd_reload_config) {
1211 if (nilfs_cleanerd_reconfig(cleanerd)) {
1212 syslog(LOG_ERR, "cannot configure:
%m");
1213 return -1;
1214 }
1215 nilfs_cleanerd_reload_config = 0;
1216 syslog(LOG_INFO, "configuration file
reloaded");
1217 }
1218
1219 if (nilfs_get_sustat(cleanerd->c_nilfs, &sustat) < 0) {
1220 syslog(LOG_ERR, "cannot get segment usage
stat: %m");
1221 return -1;
1222 }
1223 if (sustat.ss_nongc_ctime != prev_nongc_ctime) {
1224 cleanerd->c_running = 1;
1225 prev_nongc_ctime = sustat.ss_nongc_ctime;
1226 }
1227 if (!cleanerd->c_running)
1228 goto sleep;
1229
1230 syslog(LOG_DEBUG, "ncleansegs = %llu",
1231 (unsigned long long)sustat.ss_ncleansegs);
1232
1233 ns = nilfs_cleanerd_select_segments(
1234 cleanerd, &sustat, segnums, &prottime,
&oldest);
1235 if (ns < 0) {
1236 syslog(LOG_ERR, "cannot select segments: %m");
1237 return -1;
1238 }
*** if ((in_configfile_defined_treshold > 0) &&
(segnums[0] < previousfirstsegnum)) // one full pass finished or first pass
{
if more than in_configfile_defined_treshold
space available (defined in configfile)
{
set timout to
in_configfile_definied_checktime secs;
goto sleep;
}
}
previousfirstsegum = segnums[0];
1239 syslog(LOG_DEBUG, "%d segment%s selected to be
cleaned",
1240 ns, (ns <= 1) ? "" : "s");
1241 if (ns > 0) {
1242 ret = nilfs_cleanerd_clean_segments(
1243 cleanerd, &sustat, segnums, ns,
prottime);
1244 if (ret < 0)
1245 return -1;
1246 }
1247
1248 ret = nilfs_cleanerd_recalc_interval(
1249 cleanerd, ns, prottime, oldest, &timeout);
1250 if (ret < 0)
1251 return -1;
1252 else if (ret > 0)
1253 continue;
1254 sleep:
1255 if (sigprocmask(SIG_UNBLOCK, &sigset, NULL) < 0) {
1256 syslog(LOG_ERR, "cannot set signal mask: %m");
1257 return -1;
1258 }
1259
1260 ret = nilfs_cleanerd_sleep(cleanerd, &timeout);
1261 if (ret < 0)
1262 return -1;
1263 }
1264 }
I suppose that calling nilfs_get_sustat and
nilfs_cleanerd_select_segments without actually cleaning wouldn't cause
problems and nilfs_cleanerd_select_segments would always return the
first segment equal or less than the one from last call or am I wrong
with this ? Is there a nilfs specific function I should use to determine
the free space available or should I use statfs ?
Thanks in advance,
Bye,
David Arendt
On 03/14/10 06:26, Ryusuke Konishi wrote:
> Hi,
> On Sat, 13 Mar 2010 21:49:43 +0100, David Arendt wrote:
>
>> Hi,
>>
>> In order to reduce cleaner io, I am thinking it could be usefull to
>> implement a parameter where you can specify the minimum free space. If
>> this parameter is set, instead of normal cleaning operation, the cleaner
>> would wait until there is less than minimum free space available and
>> then run one cleaning pass (from first to last segment). If after that,
>> there is again more than minimum free space available, continue waiting,
>> otherwise run cleaning passes until there is more than minimum free
>> space available.
>>
>> What would you think about this idea ?
>>
>> Bye,
>> David Arendt
>>
> Well, I think this is one of what cleaner should take in. It can
> prevent cleanerd from moving in-use blocks too often unless the actual
> free space is less than the threshold.
>
> It may be the first thing to do since it's not difficult in principle.
>
> I recognize there are more fundamental defects in the current cleaner:
>
> * It moves blocks in selected segments even if all of their blocks
> are in-use.
>
> * It doesn't give priority to reclaiming segments which have many
> obsolete blocks.
>
> * It keeps working without considering io workload of the time.
>
> But, I'd rather take a quick fix like your proposal.
>
> Regards,
> Ryusuke Konishi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majord...@vger.kernel.org
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
