* [PATCH] bcache: limit create flash device size
@ 2022-09-07 11:29 mingzhe.zou
2022-09-07 17:15 ` Coly Li
0 siblings, 1 reply; 3+ messages in thread
From: mingzhe.zou @ 2022-09-07 11:29 UTC (permalink / raw)
To: colyli, linux-bcache; +Cc: zoumingzhe, mingzhe
From: mingzhe <mingzhe.zou@easystack.cn>
Currently, size is specified and not checked when creating a flash device.
This will cause a problem, IO maybe hang when creating a flash device with
the actual size of the device.
```
if (attr == &sysfs_flash_vol_create) {
int r;
uint64_t v;
strtoi_h_or_return(buf, v);
r = bch_flash_dev_create(c, v);
if (r)
return r;
}
```
Because the flash device needs some space for superblock, journal and btree.
If the size of data reaches the available size, the new IO cannot allocate
space and will hang. At this time, the gc thread will be started frequently.
Even more unreasonable, we can create flash devices larger than actual size.
```
[root@zou ~]# echo 2G > /sys/block/vdb/bcache/set/flash_vol_create
[root@zou ~]# lsblk /dev/vdb
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
vdb 252:16 0 1G 0 disk
└─bcache0 251:0 0 2G 0 disk
```
This patch will limit the size of flash device, reserving at least 5% of
available size for the btree.
```
[root@zou ~]# echo 2G > /sys/block/vdb/bcache/set/flash_vol_create
[root@zou ~]# lsblk /dev/vdb
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
vdb 252:16 0 1G 0 disk
└─bcache0 251:0 0 950M 0 disk
```
Signed-off-by: mingzhe <mingzhe.zou@easystack.cn>
---
drivers/md/bcache/super.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index ba3909bb6bea..f41e09e0e8ee 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1579,6 +1579,17 @@ static int flash_devs_run(struct cache_set *c)
return ret;
}
+static inline sector_t flash_dev_max_sectors(struct cache_set *c)
+{
+ size_t avail_nbuckets;
+ struct cache *ca = c->cache;
+ size_t first_bucket = ca->sb.first_bucket;
+ size_t njournal_buckets = ca->sb.njournal_buckets;
+
+ avail_nbuckets = c->nbuckets - first_bucket - njournal_buckets;
+ return bucket_to_sector(c, avail_nbuckets / 100 * 95);
+}
+
int bch_flash_dev_create(struct cache_set *c, uint64_t size)
{
struct uuid_entry *u;
@@ -1600,7 +1611,7 @@ int bch_flash_dev_create(struct cache_set *c, uint64_t size)
u->first_reg = u->last_reg = cpu_to_le32((u32)ktime_get_real_seconds());
SET_UUID_FLASH_ONLY(u, 1);
- u->sectors = size >> 9;
+ u->sectors = min(flash_dev_max_sectors(c), size >> 9);
bch_uuid_write(c);
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] bcache: limit create flash device size
2022-09-07 11:29 [PATCH] bcache: limit create flash device size mingzhe.zou
@ 2022-09-07 17:15 ` Coly Li
2022-09-08 5:55 ` Zou Mingzhe
0 siblings, 1 reply; 3+ messages in thread
From: Coly Li @ 2022-09-07 17:15 UTC (permalink / raw)
To: Mingzhe Zou; +Cc: linux-bcache, zoumingzhe
> 2022年9月7日 19:29,mingzhe.zou@easystack.cn 写道:
>
> From: mingzhe <mingzhe.zou@easystack.cn>
>
> Currently, size is specified and not checked when creating a flash device.
> This will cause a problem, IO maybe hang when creating a flash device with
> the actual size of the device.
>
> ```
> if (attr == &sysfs_flash_vol_create) {
> int r;
> uint64_t v;
>
> strtoi_h_or_return(buf, v);
>
> r = bch_flash_dev_create(c, v);
> if (r)
> return r;
> }
> ```
>
> Because the flash device needs some space for superblock, journal and btree.
> If the size of data reaches the available size, the new IO cannot allocate
> space and will hang. At this time, the gc thread will be started frequently.
>
> Even more unreasonable, we can create flash devices larger than actual size.
>
> ```
> [root@zou ~]# echo 2G > /sys/block/vdb/bcache/set/flash_vol_create
> [root@zou ~]# lsblk /dev/vdb
> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
> vdb 252:16 0 1G 0 disk
> └─bcache0 251:0 0 2G 0 disk
> ```
>
> This patch will limit the size of flash device, reserving at least 5% of
> available size for the btree.
>
> ```
> [root@zou ~]# echo 2G > /sys/block/vdb/bcache/set/flash_vol_create
> [root@zou ~]# lsblk /dev/vdb
> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
> vdb 252:16 0 1G 0 disk
> └─bcache0 251:0 0 950M 0 disk
> ```
>
> Signed-off-by: mingzhe <mingzhe.zou@easystack.cn>
> ---
> drivers/md/bcache/super.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index ba3909bb6bea..f41e09e0e8ee 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -1579,6 +1579,17 @@ static int flash_devs_run(struct cache_set *c)
> return ret;
> }
>
Hi Mingzhe,
> +static inline sector_t flash_dev_max_sectors(struct cache_set *c)
> +{
> + size_t avail_nbuckets;
> + struct cache *ca = c->cache;
> + size_t first_bucket = ca->sb.first_bucket;
> + size_t njournal_buckets = ca->sb.njournal_buckets;
> +
> + avail_nbuckets = c->nbuckets - first_bucket - njournal_buckets;
> + return bucket_to_sector(c, avail_nbuckets / 100 * 95);
> +}
Overall I like this idea. This is really something I didn’t realize to fix, nice catch!
BTW, I feel 95% is still quite high rate, how about using 90%? And you may define the rate as a macro in bcache.h.
Thanks for this patch.
Coly Li
> +
> int bch_flash_dev_create(struct cache_set *c, uint64_t size)
> {
> struct uuid_entry *u;
> @@ -1600,7 +1611,7 @@ int bch_flash_dev_create(struct cache_set *c, uint64_t size)
> u->first_reg = u->last_reg = cpu_to_le32((u32)ktime_get_real_seconds());
>
> SET_UUID_FLASH_ONLY(u, 1);
> - u->sectors = size >> 9;
> + u->sectors = min(flash_dev_max_sectors(c), size >> 9);
>
> bch_uuid_write(c);
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] bcache: limit create flash device size
2022-09-07 17:15 ` Coly Li
@ 2022-09-08 5:55 ` Zou Mingzhe
0 siblings, 0 replies; 3+ messages in thread
From: Zou Mingzhe @ 2022-09-08 5:55 UTC (permalink / raw)
To: Coly Li; +Cc: linux-bcache, zoumingzhe
在 2022/9/8 01:15, Coly Li 写道:
>
>> 2022年9月7日 19:29,mingzhe.zou@easystack.cn 写道:
>>
>> From: mingzhe <mingzhe.zou@easystack.cn>
>>
>> Currently, size is specified and not checked when creating a flash device.
>> This will cause a problem, IO maybe hang when creating a flash device with
>> the actual size of the device.
>>
>> ```
>> if (attr == &sysfs_flash_vol_create) {
>> int r;
>> uint64_t v;
>>
>> strtoi_h_or_return(buf, v);
>>
>> r = bch_flash_dev_create(c, v);
>> if (r)
>> return r;
>> }
>> ```
>>
>> Because the flash device needs some space for superblock, journal and btree.
>> If the size of data reaches the available size, the new IO cannot allocate
>> space and will hang. At this time, the gc thread will be started frequently.
>>
>> Even more unreasonable, we can create flash devices larger than actual size.
>>
>> ```
>> [root@zou ~]# echo 2G > /sys/block/vdb/bcache/set/flash_vol_create
>> [root@zou ~]# lsblk /dev/vdb
>> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
>> vdb 252:16 0 1G 0 disk
>> └─bcache0 251:0 0 2G 0 disk
>> ```
>>
>> This patch will limit the size of flash device, reserving at least 5% of
>> available size for the btree.
>>
>> ```
>> [root@zou ~]# echo 2G > /sys/block/vdb/bcache/set/flash_vol_create
>> [root@zou ~]# lsblk /dev/vdb
>> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
>> vdb 252:16 0 1G 0 disk
>> └─bcache0 251:0 0 950M 0 disk
>> ```
>>
>> Signed-off-by: mingzhe <mingzhe.zou@easystack.cn>
>> ---
>> drivers/md/bcache/super.c | 13 ++++++++++++-
>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
>> index ba3909bb6bea..f41e09e0e8ee 100644
>> --- a/drivers/md/bcache/super.c
>> +++ b/drivers/md/bcache/super.c
>> @@ -1579,6 +1579,17 @@ static int flash_devs_run(struct cache_set *c)
>> return ret;
>> }
>>
>
> Hi Mingzhe,
>
>> +static inline sector_t flash_dev_max_sectors(struct cache_set *c)
>> +{
>> + size_t avail_nbuckets;
>> + struct cache *ca = c->cache;
>> + size_t first_bucket = ca->sb.first_bucket;
>> + size_t njournal_buckets = ca->sb.njournal_buckets;
>> +
>> + avail_nbuckets = c->nbuckets - first_bucket - njournal_buckets;
>> + return bucket_to_sector(c, avail_nbuckets / 100 * 95);
>> +}
> Overall I like this idea. This is really something I didn’t realize to fix, nice catch!
>
> BTW, I feel 95% is still quite high rate, how about using 90%? And you may define the rate as a macro in bcache.h.
Hi Coly
Of course, it would be better to define as a macro, I will send v2.
From my testing, 95% should be safe, no IO hung.
But 90% is better than 95%, because every write 6.25% (1/16) of the
total capacity will trigger a gc.
mingzhe
>
> Thanks for this patch.
>
> Coly Li
>
>> +
>> int bch_flash_dev_create(struct cache_set *c, uint64_t size)
>> {
>> struct uuid_entry *u;
>> @@ -1600,7 +1611,7 @@ int bch_flash_dev_create(struct cache_set *c, uint64_t size)
>> u->first_reg = u->last_reg = cpu_to_le32((u32)ktime_get_real_seconds());
>>
>> SET_UUID_FLASH_ONLY(u, 1);
>> - u->sectors = size >> 9;
>> + u->sectors = min(flash_dev_max_sectors(c), size >> 9);
>>
>> bch_uuid_write(c);
>>
>> --
>> 2.17.1
>>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-08 5:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-07 11:29 [PATCH] bcache: limit create flash device size mingzhe.zou
2022-09-07 17:15 ` Coly Li
2022-09-08 5:55 ` Zou Mingzhe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).