Hi Vyacheslav, Thank you for reviewing my patch set so thoroughly. I already have a few new ideas to improve it from our exchange. On 2014-01-20 11:46, Vyacheslav Dubeyko wrote: > On Sun, 2014-01-19 at 15:01 +0100, Andreas Rohner wrote: >> This patch adds the custom error value EGCTRYAGAIN, which signals to >> cleanerd to retry the gc process as soon as possible with no timeout. >> >> If the GC decides not to do any real work and only changes a few >> metadata bytes, there is no need for a timeout. Furthermore if the GC is >> running, there is probably not enough free space on the device and it is >> crucial to retry quickly. > > Now GC really decrease performance very frequently. So, you suggest > don't use timeout. :) I am afraid that it can decrease performance more. > Why do you think in opposite manner? I don't suggest to eliminate timeout in general. It just adds the option for nilfs_reclaim_segment to skip the timeout once by returning the custom error code -EGCTRYAGAIN. This is useful because if the number of free blocks is below the threshold the cleaner won't write to the device and won't free any segments, so it is important to retry another segment quickly. I guess it could be reasonable to use a shorter timeout instead of no timeout in this case. > If we doesn't relate with idle system state or I/O load then rigid > timeout can really decrease performance, as far as I can judge. The main > problem of GC that it works in the background of main file system > operations and, as a result, decrease the whole performance. So, how the > "no-timeout" does correlate with main file system operation or with the > whole system load? > > Could you describe your vision how it will work? Well the GC uses the normal timeout for its normal operation. If the number of free blocks is below the threshold it skips the current segment or segments and returns -EGCTRYAGAIN to skip the timeout ONCE. I thought it is good practice to separate a big patch into logical units. That's why it is probably not clear what the intent is by just looking at a single patch in the patch set. I will try to improve my commit message to make it clearer. Best regards, Andreas Rohner -- To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
