Re: cleaner: run one cleaning pass based on minimum free space

[admin]

Hi,

At the moment of submitting my cleaner patches, I didn't know if you would
use the new behavior as default behavior or the old one therefore in
documentation I had described that min_clean_segments = 0 would mean
normal cleaner behavior. However as the new behavior is now used as
default behavior, normal behavior might be confusing.

I am thinking about doing the following changes in documentation:

in nilfs_cleanerd.conf change

# Minium number of clean segments
# 0  = normal cleaner behaviour
# 0 = start cleaning if less segments are available
min_clean_segments  10%

to

# Minimum number of clean segments
#   0 = continuous cleaning
#  0 = pause cleaning until less segments are available
min_clean_segments  10%

I just saw that I had a typo in the word minimum.

in nilfs_cleanerd.conf.8 change

.B min_clean_segments
Specify the minimum number of clean segments. A value of 0 means
normal cleaner operation. A value greater than 0 means pause cleaning
until less than min_clean_segments are available.

to

.B min_clean_segments
Specify the minimum number of clean segments. A value of 0 means
continuous cleaning. A value greater than 0 means pause cleaning
until less than min_clean_segments are available.

What do you think about it ?

If you want to change, will you do changes or should I send you a patch ?

Thanks in advance,
Bye,
David Arendt

 Hi,
 On Tue, 06 Apr 2010 19:41:33 +0200, David Arendt ad...@prnet.org wrote:
 Hi,

 here the updated patch

 Thanks in advance
 Bye,
 David Arendt

 Quick work ;)
 Looks fine to me.

 I'll apply it after some tests.

 Thanks.
 Ryusuke Konishi


 On 04/06/10 18:06, Ryusuke Konishi wrote:
  Hi,
  On Mon, 05 Apr 2010 15:35:34 +0200, David Arendt ad...@prnet.org
 wrote:
 
  Hi,
 
  here is the patch
 
  Thanks in advance
  Bye,
  David Arendt
 
  Thanks.
 
  Looks functionally fine. Just a matter of coding style:
 
  The following part of cleaner loop looks bloated.  I think it's a good
  opportunity to divide it from the loop.
 
  Also, the length of their lines looks too long.  It's preferable if
  they're naturally wrapped within 80-columns.  This is not a must
  especially for userland tools, however kernel developers often prefer
  this conventional coding rule.  Separating the routine would make this
  easy.
 
  if (cleanerd-c_config.cf_min_clean_segments  0) {
  --- from
  if (cleanerd-c_running) {
  if (sustat.ss_ncleansegs 
 cleanerd-c_config.c\
  f_max_clean_segments + r_segments) {
  
  nilfs_cleanerd_clean_check_pause(cleane\
  rd, timeout);
  goto sleep;
  }
  }
  else {
  if (sustat.ss_ncleansegs 
 cleanerd-c_config.c\
  f_min_clean_segments + r_segments)
  
  nilfs_cleanerd_clean_check_resume(clean\
  erd);
  else
  goto sleep;
  }
 
  if (sustat.ss_ncleansegs 
 cleanerd-c_config.cf_min_cl\
  ean_segments + r_segments) {
  cleanerd-c_ncleansegs =
 cleanerd-c_config.cf_\
  mc_nsegments_per_clean;
  cleanerd-c_cleaning_interval =
 cleanerd-c_con\
  fig.cf_mc_cleaning_interval;
  }
  else {
  cleanerd-c_ncleansegs =
 cleanerd-c_config.cf_\
  nsegments_per_clean;
  cleanerd-c_cleaning_interval =
 cleanerd-c_con\
  fig.cf_cleaning_interval;
  }
   to
  }
 
  With regards,
  Ryusuke Konishi
 
 
 
  On 04/05/10 13:34, Ryusuke Konishi wrote:
 
  Hi,
  On Mon, 05 Apr 2010 09:50:11 +0200, David Arendt ad...@prnet.org
 wrote:
 
 
  Hi,
 
  Actually I run with min_clean_segments at 250 and found that to be
 a
  good value. However for example for a 2 gbyte usb key, this value
 would
  not work at all, therefor I find it a good idea to set the default
 at
  10% as it would be more general for any device size as lots of
 people
  simply try the defaults without changing configuration files.
 
 
  Ok, let's start with the 10% min_clean_segments for the default
  values.
 
 
 
  I really like your idea with having a second set of
  nsegments_per_clean and cleaning_interval_parameters for 
  min_clean_segments. I am wondering if adaptive optimization will be
  good, as I think different people will expect different behavior.
  Someones might prefer the system using 100% io usage for cleaning
  and the disk not getting full. Other ones might prefer the disk
  getting full and using less io usage.
 
 
  Well, that's true.  Netbook users would prefer suspending the
 cleaner
  wherever possible to avoid their SSD worn out. 

Re: cleaner: run one cleaning pass based on minimum free space

[David Arendt]

Hi,

here the changes

Thank in advance,
David Arendt

On 03/29/10 05:59, Ryusuke Konishi wrote:
 Hi,
 On Sun, 28 Mar 2010 23:52:52 +0200, David Arendt ad...@prnet.org wrote:
   
 Hi,

 thanks for applying the patches. I did all my tests on 2 gbyte loop
 devices and now that it is officially in git, I deployed it to some
 production systems with big disks. Here I have noticed, that I have
 completely forgotten the reserved segments. Technically this is not a
 problem, but I think people changing configuration files will tend to
 forget about it. I'm thinking it might be useful to add them internally
 to min_free_segments and max_free_segments so users don't need to worry
 about them. What do you think ?
 
 Ahh, we should take into account the number of reserved segments.  If
 not so, cleaner control with the two threshold values will not work
 properly for large drives.

   
 If you like to change the current behavior to this behavior, I will
 submit a short update patch.
 
 Yes, please do.

   
 I am thinking about getting the number of reserved segments this way:

 (nilfs_cleanerd-c_nilfs-n_sb-s_nsegments *
 nilfs_cleanerd-c_nilfs-n_sb-s_r_segments_percentage) / 100

 or do you know any better way ?
 
 The kernel code calulates the number by:

   = max(NILFS_MIN_NRSVSEGS,
 DIV_ROUND_UP(nsegments * r_segments_percentage, 100))

   where NILFS_MIN_NRSVSEGS is defined in include/nilfs2_fs.h, and
   DIV_ROUND_UP is defined as follows:

  #define DIV_ROUND_UP(n,d)(((n) +  (d) - 1) / (d))

 The same or some equivelent calculation seems preferable.
  
 With regards,
 Ryusuke Konishi

   
 On 03/28/10 17:26, Ryusuke Konishi wrote:
 
 Hi,
 On Sun, 28 Mar 2010 14:17:00 +0200, David Arendt ad...@prnet.org wrote:
   
   
 Hi,

 here the nogc patch

 As changelog description for this one, we could put:

 add mount option to disable garbage collection

 Thanks in advance
 Bye,
 David Arendt
 
 
 Hmm, the patch looks perfect.

 Will queue both in the git tree of utils.

 Thanks,
 Ryusuke Konishi
   
   
 

diff -ur nilfs2-utils.orig/sbin/cleanerd/cleanerd.c 
nilfs2-utils/sbin/cleanerd/cleanerd.c
--- nilfs2-utils.orig/sbin/cleanerd/cleanerd.c  2010-03-29 06:05:51.382126765 
+0200
+++ nilfs2-utils/sbin/cleanerd/cleanerd.c   2010-03-29 06:32:09.129775882 
+0200
@@ -1185,7 +1185,7 @@
 static int nilfs_cleanerd_clean_loop(struct nilfs_cleanerd *cleanerd)
 {
struct nilfs_sustat sustat;
-   __u64 prev_nongc_ctime = 0, prottime = 0, oldest = 0;
+   __u64 r_segments, prev_nongc_ctime = 0, prottime = 0, oldest = 0;
__u64 segnums[NILFS_CLDCONFIG_NSEGMENTS_PER_CLEAN_MAX];
struct timespec timeout;
sigset_t sigset;
@@ -1215,6 +1215,10 @@
 
cleanerd-c_ncleansegs = cleanerd-c_config.cf_nsegments_per_clean;
 
+   r_segments = ((nilfs_cleanerd-c_nilfs-n_sb-s_nsegments * 
nilfs_cleanerd-c_nilfs-n_sb-s_r_segments_percentage) + 99) / 100;
+   if (r_segments  NILFS_MIN_NRSVSEGS)
+   r_segments = NILFS_MIN_NRSVSEGS;
+
if (cleanerd-c_config.cf_min_clean_segments  0)
nilfs_cleanerd_clean_check_pause(cleanerd, timeout);
else
@@ -1242,13 +1246,13 @@
 
if (cleanerd-c_config.cf_min_clean_segments  0) {
if (cleanerd-c_running) {
-   if (sustat.ss_ncleansegs  
cleanerd-c_config.cf_max_clean_segments) {
+   if (sustat.ss_ncleansegs  
cleanerd-c_config.cf_max_clean_segments + r_segments) {

nilfs_cleanerd_clean_check_pause(cleanerd, timeout);
goto sleep;
}
}
else {
-   if (sustat.ss_ncleansegs  
cleanerd-c_config.cf_min_clean_segments)
+   if (sustat.ss_ncleansegs  
cleanerd-c_config.cf_min_clean_segments + r_segments)

nilfs_cleanerd_clean_check_resume(cleanerd);
else
goto sleep;

Re: cleaner: run one cleaning pass based on minimum free space

[David Arendt]

Hi,

On 03/27/10 18:48, Ryusuke Konishi wrote:
 Hi David,
 On Wed, 24 Mar 2010 06:35:00 +0100, David Arendt wrote:
   
 Hi,

 just for completeness, here is a re-post of the complete patch using
 cleanerd-c_running instead of local variable sleeping.

 Bye,
 David Arendt
 
 Sorry for my late response.

 I'm planning to apply your patch.

 The patch looks reducible some more, for example, the preparation:

   
 +   if (cleanerd-c_config.cf_min_clean_segments  0) {
 +   syslog(LOG_INFO, cleaner paused);
 +   cleanerd-c_running = 0;
 +   timeout.tv_sec = cleanerd-c_config.cf_clean_check_interval;
 +   timeout.tv_nsec = 0;
 +   }
 +   else
 + cleanerd-c_running = 1;
 +
 
 can be simplified as follows:

   if (cleanerd-c_config.cf_min_clean_segments == 0)
   cleanerd-c_running = 1;

   

Well, the cleanerd-c_running=0 would not be needed, but I thought the
code would be more understandable if putting it once again here. As I
start in paused state, I thought it would also be good to log this
paused state. The time initialisation is done here as otherwise it would
have always to be done inside the loop using a very very little bit more
resources. It think it could be better readable if I would make to
functions pause and resume and calling the respective functions. I will
make a try and send you the revised patch, so you can tell me what you
think about it.
 And, the status control using cleanerd-c_running seems to have room
 for improvement.  Except for these trivial matters, your change looks
 simple but effective, and is flawlessly keeping compatibility.

 If you have a revised patch, please send me for merge.  Also, I would
 appreciate it if you could write some changelog description.

 Thank you in advance,
 Ryusuke Konishi

   
Bye,
David Arendt
 On 03/17/10 19:11, Ryusuke Konishi wrote:
 
 Hi,
 On Mon, 15 Mar 2010 22:24:28 +0100, David Arendt wrote:
   
   
 Hi,

 Well I didn't know that a few days can pass as fast :-)

 I have attached the patch to this mail.

 Until now the patch has only been shortly tested on a loop device, so it
 might contain bugs and destroy your data.
 
 
 Thank you for posting the patch!

 The patch looks rougly ok to me.
 I'll comment on it later.

 At first glance, I felt it would be nice if cleanerd-c_running is
 nicely used instead of adding a local variable sleeping.

 Thanks,
 Ryusuke Konishi
  
   
   
 If you decide to apply it, please change the default values to the ones
 you find the most appropriate.

 Thanks in advance,
 Bye,
 David Arendt

 On 03/15/10 16:58, Ryusuke Konishi wrote:
 
 
 Hi,
 On Mon, 15 Mar 2010 00:03:45 +0100, David Arendt wrote:
   
   
   
 Hi,

 I am posting this again to the correct mailing list as I cc'ed it to the
 old inactive one.

 Maybe I am understanding something wrong, but if I would use the count
 of reclaimed segments, how could I determine if one cleaning pass has
 finished as I don't know in advance how many segments could be reclaimed 
 ?
 
 
 
 For example, how about this?

  nmax = (number of segments) - (number of clean segments)
  nblk = (max_clean_segments - (number of clean segments)) *
 (number of blocks per segment)

  * If (number of clean segments)  min_clean_segments, then start 
 reclamation
  * Try to reclaim nmax segments (at a maximum).
  * When the cleaner found and freed nblk blocks during the
reclamation, then end one cleaning pass.

   
   
   
 Another approach would be not basing cleaning on a whole cleaning pass
 but instead creating these addtional configfile options:

 # start cleaning if less than 100 free segments
 min_clean_segments 100

 # stop cleaning if more than 200 free segments
 max_clean_segments 200

 # check free space once an hour
 segment_check_interval 3600

 Basically in this example if less than 800mb are free cleaner is run
 until 1600mb are free. If min_clean_segments is 0, the cleaner would do
 normal operation.
 
 
 
 The first two parameters look Ok.
 (I've already referred to these in the above example.)

 We may well be able to make segment_check_interval more frequent.
 or do you have something in mind? 

 Do you mean interval of cleaning passes ?

   
   
   
 For this solution only changes in configfile loading and
 nilfs_cleanerd_clean_loop would be necessary which would lower the risk
 of introducing new bugs.

 If this solution is ok for you, I will implement it this way and send
 you the patch in a few days. Also tell me if the names I have choosen
 for the options are ok for you or if you would prefer other ones.
 
 
 
 The option names look fine to me.
 Or should we use percentage for them?
 (number of segments is device dependent)

 Is there anything else that isn't clear?

   
   
   
 Thanks in advance
 Bye,
 

Re: cleaner: run one cleaning pass based on minimum free space

[Ryusuke Konishi]

Hi,
On Mon, 15 Mar 2010 18:09:38 +0100, David Arendt wrote:
 Hi,
 
 In fact by segment check interval I mean the time to sleep before
 checking again for free space. I used 3600 in the example as this is
 would be suitable for my workload, but 60 might be a safer default value.
 
 Specifying a percentage would also be an idea. I thought about segments
 as nsegments_per_clean is also referring to segments. I think I will
 start implementing this now using segments, as changing it to percent
 wouldn't be big changes. Another idea that comes me to mind would be
 that specifying for example 10 in any configfile option accepting
 segments would be 10 segments and 10% ten percent of total segments.

Sounds nice.

As for nsegments_per_clean, seems like the percentage notation should
not be applied because it has a bad effect on memory usage of the
nilfs GC cache in kernel. (So, the parameter is limited by a ceiling.)

But, I agree that the notation is suited for such threshold
parameters.

Regards,
Ryusuke Konishi

 For the rest, I think everything should be clear and I should have a
 patch ready in a few days.
 
 Bye,
 David Arendt
 
 On 03/15/10 16:58, Ryusuke Konishi wrote:
  Hi,
  On Mon, 15 Mar 2010 00:03:45 +0100, David Arendt wrote:

  Hi,
 
  I am posting this again to the correct mailing list as I cc'ed it to the
  old inactive one.
 
  Maybe I am understanding something wrong, but if I would use the count
  of reclaimed segments, how could I determine if one cleaning pass has
  finished as I don't know in advance how many segments could be reclaimed ?
  
  For example, how about this?
 
   nmax = (number of segments) - (number of clean segments)
   nblk = (max_clean_segments - (number of clean segments)) *
  (number of blocks per segment)
 
   * If (number of clean segments)  min_clean_segments, then start 
  reclamation
   * Try to reclaim nmax segments (at a maximum).
   * When the cleaner found and freed nblk blocks during the
 reclamation, then end one cleaning pass.
 

  Another approach would be not basing cleaning on a whole cleaning pass
  but instead creating these addtional configfile options:
 
  # start cleaning if less than 100 free segments
  min_clean_segments 100
 
  # stop cleaning if more than 200 free segments
  max_clean_segments 200
 
  # check free space once an hour
  segment_check_interval 3600
 
  Basically in this example if less than 800mb are free cleaner is run
  until 1600mb are free. If min_clean_segments is 0, the cleaner would do
  normal operation.
  
  The first two parameters look Ok.
  (I've already referred to these in the above example.)
 
  We may well be able to make segment_check_interval more frequent.
  or do you have something in mind? 
 
  Do you mean interval of cleaning passes ?
 

  For this solution only changes in configfile loading and
  nilfs_cleanerd_clean_loop would be necessary which would lower the risk
  of introducing new bugs.
 
  If this solution is ok for you, I will implement it this way and send
  you the patch in a few days. Also tell me if the names I have choosen
  for the options are ok for you or if you would prefer other ones.
  
  The option names look fine to me.
  Or should we use percentage for them?
  (number of segments is device dependent)
 
  Is there anything else that isn't clear?
 

  Thanks in advance
  Bye,
  David Arendt
  
  Thanks,
  Ryusuke Konishi 
 

  On 03/14/10 15:28, Ryusuke Konishi wrote:
  
  Hi,
  On Sun, 14 Mar 2010 14:00:19 +0100, ad...@prnet.org wrote:


  Hi,
 
  I will try to implement this myself then. Concerning the
  nilfs_cleanerd_select segments function I was unclear in my post. In
  fact I did not mean the return value but the first element from the
  segnums array.
  
  
  Ok. So you thought of determining termination of one cleaning pass by
  the segment number stored preliminarily.
 
  Why not just use count of processed (i.e. reclaimed) segments?
 
  Note that it's not guranteed that segments are selected in the order
  of segment number though this premise looks almost right.
 
  It depends on the behavior of segment allocator and the current
  Select-oldest algorithm used behind
  nilfs_cleanerd_select_segments().  Nilfs log writer occasionally
  behaves differently and disturbs this order.
 
  I think you can ignore the exceptional behavior of the segment
  allocator, and rotate target segments with skipping free or mostly
  in-use ones.  In that case, nilfs_cleanerd_select_segments() should be
  modified to select segments in the order of segment number.
 
  Cheers,
  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 

Re: cleaner: run one cleaning pass based on minimum free space

[Ryusuke Konishi]

Hi,
On Mon, 15 Mar 2010 00:03:45 +0100, David Arendt wrote:
 Hi,
 
 I am posting this again to the correct mailing list as I cc'ed it to the
 old inactive one.
 
 Maybe I am understanding something wrong, but if I would use the count
 of reclaimed segments, how could I determine if one cleaning pass has
 finished as I don't know in advance how many segments could be reclaimed ?

For example, how about this?

 nmax = (number of segments) - (number of clean segments)
 nblk = (max_clean_segments - (number of clean segments)) *
(number of blocks per segment)

 * If (number of clean segments)  min_clean_segments, then start reclamation
 * Try to reclaim nmax segments (at a maximum).
 * When the cleaner found and freed nblk blocks during the
   reclamation, then end one cleaning pass.

 Another approach would be not basing cleaning on a whole cleaning pass
 but instead creating these addtional configfile options:
 
 # start cleaning if less than 100 free segments
 min_clean_segments 100
 
 # stop cleaning if more than 200 free segments
 max_clean_segments 200
 
 # check free space once an hour
 segment_check_interval 3600
 
 Basically in this example if less than 800mb are free cleaner is run
 until 1600mb are free. If min_clean_segments is 0, the cleaner would do
 normal operation.

The first two parameters look Ok.
(I've already referred to these in the above example.)

We may well be able to make segment_check_interval more frequent.
or do you have something in mind? 

Do you mean interval of cleaning passes ?

 For this solution only changes in configfile loading and
 nilfs_cleanerd_clean_loop would be necessary which would lower the risk
 of introducing new bugs.

 If this solution is ok for you, I will implement it this way and send
 you the patch in a few days. Also tell me if the names I have choosen
 for the options are ok for you or if you would prefer other ones.

The option names look fine to me.
Or should we use percentage for them?
(number of segments is device dependent)

Is there anything else that isn't clear?

 Thanks in advance
 Bye,
 David Arendt

Thanks,
Ryusuke Konishi 

 On 03/14/10 15:28, Ryusuke Konishi wrote:
  Hi,
  On Sun, 14 Mar 2010 14:00:19 +0100, ad...@prnet.org wrote:

  Hi,
 
  I will try to implement this myself then. Concerning the
  nilfs_cleanerd_select segments function I was unclear in my post. In
  fact I did not mean the return value but the first element from the
  segnums array.
  
  Ok. So you thought of determining termination of one cleaning pass by
  the segment number stored preliminarily.
 
  Why not just use count of processed (i.e. reclaimed) segments?
 
  Note that it's not guranteed that segments are selected in the order
  of segment number though this premise looks almost right.
 
  It depends on the behavior of segment allocator and the current
  Select-oldest algorithm used behind
  nilfs_cleanerd_select_segments().  Nilfs log writer occasionally
  behaves differently and disturbs this order.
 
  I think you can ignore the exceptional behavior of the segment
  allocator, and rotate target segments with skipping free or mostly
  in-use ones.  In that case, nilfs_cleanerd_select_segments() should be
  modified to select segments in the order of segment number.
 
  Cheers,
  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

Re: cleaner: run one cleaning pass based on minimum free space

[admin]

Hi,

I will try to implement this myself then. Concerning the nilfs_cleanerd_select 
segments function I was unclear in  my post. In fact I did not mean the return 
value but the first element from the segnums array.

Bye,
David Arendt

-original message-
Subject: Re: cleaner: run one cleaning pass based on minimum free space
From: Ryusuke Konishi ryus...@osrg.net
Date: 14/03/2010 12:59

Hi,
On Sun, 14 Mar 2010 09:47:55 +0100, David Arendt wrote:
 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 ?

I'd appreciate it if you try it yourself.  I cannot commit time to
this at least for a few weeks.

 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,
 1240ns, (ns = 1) ?  : s);
 1241 if (ns  0) {
 1242 ret = nilfs_cleanerd_clean_segments(
 1243 cleanerd, sustat, segnums, ns

Re: cleaner: run one cleaning pass based on minimum free space

[David Arendt]

Hi,

I am posting this again to the correct mailing list as I cc'ed it to the
old inactive one.

Maybe I am understanding something wrong, but if I would use the count
of reclaimed segments, how could I determine if one cleaning pass has
finished as I don't know in advance how many segments could be reclaimed ?

Another approach would be not basing cleaning on a whole cleaning pass
but instead creating these addtional configfile options:

# start cleaning if less than 100 free segments
min_clean_segments 100

# stop cleaning if more than 200 free segments
max_clean_segments 200

# check free space once an hour
segment_check_interval 3600

Basically in this example if less than 800mb are free cleaner is run
until 1600mb are free. If min_clean_segments is 0, the cleaner would do
normal operation.

For this solution only changes in configfile loading and
nilfs_cleanerd_clean_loop would be necessary which would lower the risk
of introducing new bugs.

If this solution is ok for you, I will implement it this way and send
you the patch in a few days. Also tell me if the names I have choosen
for the options are ok for you or if you would prefer other ones.

Thanks in advance
Bye,
David Arendt


On 03/14/10 15:28, Ryusuke Konishi wrote:
 Hi,
 On Sun, 14 Mar 2010 14:00:19 +0100, ad...@prnet.org wrote:
   
 Hi,

 I will try to implement this myself then. Concerning the
 nilfs_cleanerd_select segments function I was unclear in my post. In
 fact I did not mean the return value but the first element from the
 segnums array.
 
 Ok. So you thought of determining termination of one cleaning pass by
 the segment number stored preliminarily.

 Why not just use count of processed (i.e. reclaimed) segments?

 Note that it's not guranteed that segments are selected in the order
 of segment number though this premise looks almost right.

 It depends on the behavior of segment allocator and the current
 Select-oldest algorithm used behind
 nilfs_cleanerd_select_segments().  Nilfs log writer occasionally
 behaves differently and disturbs this order.

 I think you can ignore the exceptional behavior of the segment
 allocator, and rotate target segments with skipping free or mostly
 in-use ones.  In that case, nilfs_cleanerd_select_segments() should be
 modified to select segments in the order of segment number.

 Cheers,
 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