On 2017/6/9 下午3:11, tang.junhui@xxxxxxxxxx wrote:
> From: Tang Junhui <tang.junhui@xxxxxxxxxx>
>
> I try to execute the following command to trigger gc thread:
> [root@localhost internal]# echo 1 > trigger_gc
> But it does not work, I debug the code in gc_should_run(), It works only
> if in invalidating or sectors_to_gc < 0. So set sectors_to_gc to -1 to
> meet the condition when we trigger gc by manual command.
>
Hi Junhui,
Thanks for the patch, it totally makes sense IMHO. In most of use cases
I know, people writing to trigger_gc file expecting a force garbage
collection.
But assign a minus value here is quite confused for people who read the
code. Could you please also add some comments in the code to explain why
you set c->sectors_to_gc to -1 here.
And there might be a race that when you set c->sectors_to_gc to -1, and
call wake_up_gc(), but before code go into gc_should_run(),
c->sectors_to_gc is reset by set_gc_sectors(). I have no object if you
don't want to handle this situation, but I do suggest to explain it in
code why we can ignore such a race.
> Signed-off-by: Tang Junhui <tang.junhui@xxxxxxxxxx>
> ---
> drivers/md/bcache/sysfs.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c
> index b3ff57d..f5bb93a 100644
> --- a/drivers/md/bcache/sysfs.c
> +++ b/drivers/md/bcache/sysfs.c
> @@ -614,8 +614,10 @@ STORE(__bch_cache_set)
> bch_cache_accounting_clear(&c->accounting);
> }
>
> - if (attr == &sysfs_trigger_gc)
> + if (attr == &sysfs_trigger_gc) {
> + atomic_set(&c->sectors_to_gc, -1);
> wake_up_gc(c);
> + }
>
> if (attr == &sysfs_prune_cache) {
> struct shrink_control sc;
>
--
Coly Li
--
To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
