On 2020/7/14 13:57, Hannes Reinecke wrote:
> On 7/13/20 5:28 AM, Guoju Fang wrote:
>> When read cache miss, backing device will be read first, and then refill
>> the cache device. But under some scenarios there are large number of new
>> reads and rarely hit, so it's necessary to disable the refill when read
>> miss to save space for writes.
>>
>> This patch add a new config called refill_on_miss_disabled which is
>> not set
>> by default. Bcache user can set it by sysfs interface and then the bcache
>> device will not refill when read cache miss.
>>
>> Signed-off-by: Guoju Fang <fangguoju@xxxxxxxxx>
>> ---
>> drivers/md/bcache/bcache.h | 1 +
>> drivers/md/bcache/request.c | 2 ++
>> drivers/md/bcache/super.c | 1 +
>> drivers/md/bcache/sysfs.c | 5 +++++
>> 4 files changed, 9 insertions(+)
>>
>> diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
>> index 221e0191b687..3a19ee6de3a7 100644
>> --- a/drivers/md/bcache/bcache.h
>> +++ b/drivers/md/bcache/bcache.h
>> @@ -730,6 +730,7 @@ struct cache_set {
>> unsigned int shrinker_disabled:1;
>> unsigned int copy_gc_enabled:1;
>> unsigned int idle_max_writeback_rate_enabled:1;
>> + unsigned int refill_on_miss_disabled:1;
>> #define BUCKET_HASH_BITS 12
>> struct hlist_head bucket_hash[1 << BUCKET_HASH_BITS];
>> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
>> index 7acf024e99f3..4bfa0e0b4b3f 100644
>> --- a/drivers/md/bcache/request.c
>> +++ b/drivers/md/bcache/request.c
>> @@ -378,6 +378,8 @@ static bool check_should_bypass(struct cached_dev
>> *dc, struct bio *bio)
>> op_is_write(bio_op(bio))))
>> goto skip;
>> + if (c->refill_on_miss_disabled && !op_is_write(bio_op(bio)))
>> + goto skip;
>> /*
>> * If the bio is for read-ahead or background IO, bypass it or
>> * not depends on the following situations,
>> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
>> index 2014016f9a60..c1e9bfec1267 100644
>> --- a/drivers/md/bcache/super.c
>> +++ b/drivers/md/bcache/super.c
>> @@ -1862,6 +1862,7 @@ struct cache_set *bch_cache_set_alloc(struct
>> cache_sb *sb)
>> c->congested_write_threshold_us = 20000;
>> c->error_limit = DEFAULT_IO_ERROR_LIMIT;
>> c->idle_max_writeback_rate_enabled = 1;
>> + c->refill_on_miss_disabled = 0;
>> WARN_ON(test_and_clear_bit(CACHE_SET_IO_DISABLE, &c->flags));
>> return c;
>> diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c
>> index 0dadec5a78f6..178300f401bb 100644
>> --- a/drivers/md/bcache/sysfs.c
>> +++ b/drivers/md/bcache/sysfs.c
>> @@ -144,6 +144,7 @@ rw_attribute(copy_gc_enabled);
>> rw_attribute(idle_max_writeback_rate);
>> rw_attribute(gc_after_writeback);
>> rw_attribute(size);
>> +rw_attribute(refill_on_miss_disabled);
>> static ssize_t bch_snprint_string_list(char *buf,
>> size_t size,
>> @@ -779,6 +780,8 @@ SHOW(__bch_cache_set)
>> if (attr == &sysfs_bset_tree_stats)
>> return bch_bset_print_stats(c, buf);
>> + sysfs_printf(refill_on_miss_disabled, "%i",
>> c->refill_on_miss_disabled);
>> +
>> return 0;
>> }
>> SHOW_LOCKED(bch_cache_set)
>> @@ -898,6 +901,7 @@ STORE(__bch_cache_set)
>> * set in next chance.
>> */
>> sysfs_strtoul_clamp(gc_after_writeback, c->gc_after_writeback,
>> 0, 1);
>> + sysfs_strtoul(refill_on_miss_disabled, c->refill_on_miss_disabled);
>> return size;
>> }
>> @@ -948,6 +952,7 @@ static struct attribute *bch_cache_set_files[] = {
>> &sysfs_congested_read_threshold_us,
>> &sysfs_congested_write_threshold_us,
>> &sysfs_clear_stats,
>> + &sysfs_refill_on_miss_disabled,
>> NULL
>> };
>> KTYPE(bch_cache_set);
>>
Hi Hannes,
> Please don't call the attribute refill_on_miss_disabled.
> This kind of double-negation will always lead to issues; please invert
> the meaning and call it 'refill_on_miss'.
The original purpose is to implement a "write-only" or "read-around"
like cache. Such kind of cache model is desired for heavy write
condition, and disable the read-miss-refill may also help to extend the
SSD life time.
Naming is a challenge here. Becuase "write-only" is not true while read
hitting on dirty data on cache device may successfully return, and
"read-around" is also not true due to the same reason.
We do need help for people to suggest a proper mode for such cache mode,
or cache configuration (if we don't want one more cache mode).
Thanks.
Coly Li
