diff mbox series

[09/14] bcache: avoid data copy between cache_set->sb and cache->sb

Message ID 20200815041043.45116-10-colyli@suse.de (mailing list archive)
State New, archived
Headers show
Series bcache: remove multiple caches code framework | expand

Commit Message

Coly Li Aug. 15, 2020, 4:10 a.m. UTC
struct cache_sb embedded in struct cache_set is only partial used and
not a real copy from cache's in-memory super block. When removing the
embedded cache_set->sb, it is unncessary to copy data between these two
in-memory super blocks (cache_set->sb and cache->sb), it is sufficient
to just use cache->sb.

This patch removes the data copy between these two in-memory super
blocks in bch_cache_set_alloc() and bcache_write_super(). In future
except for set_uuid, cache's super block will be referenced by cache
set, no copy any more.

Signed-off-by: Coly Li <colyli@suse.de>
---
 drivers/md/bcache/super.c | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

Comments

Hannes Reinecke Aug. 17, 2020, 6:22 a.m. UTC | #1
On 8/15/20 6:10 AM, Coly Li wrote:
> struct cache_sb embedded in struct cache_set is only partial used and
> not a real copy from cache's in-memory super block. When removing the
> embedded cache_set->sb, it is unncessary to copy data between these two
> in-memory super blocks (cache_set->sb and cache->sb), it is sufficient
> to just use cache->sb.
> 
> This patch removes the data copy between these two in-memory super
> blocks in bch_cache_set_alloc() and bcache_write_super(). In future
> except for set_uuid, cache's super block will be referenced by cache
> set, no copy any more.
> 
> Signed-off-by: Coly Li <colyli@suse.de>
> ---
>   drivers/md/bcache/super.c | 22 +++-------------------
>   1 file changed, 3 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 748b08ab4f11..05c5a7e867bb 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -350,16 +350,10 @@ void bcache_write_super(struct cache_set *c)
>   	down(&c->sb_write_mutex);
>   	closure_init(cl, &c->cl);
>   
> -	c->sb.seq++;
> +	ca->sb.seq++;
>   
> -	if (c->sb.version > version)
> -		version = c->sb.version;
> -
> -	ca->sb.version		= version;
> -	ca->sb.seq		= c->sb.seq;
> -	ca->sb.last_mount	= c->sb.last_mount;
> -
> -	SET_CACHE_SYNC(&ca->sb, CACHE_SYNC(&c->sb));
> +	if (ca->sb.version < version)
> +		ca->sb.version = version;
>   
>   	bio_init(bio, ca->sb_bv, 1);
>   	bio_set_dev(bio, ca->bdev);
> @@ -1860,16 +1854,6 @@ struct cache_set *bch_cache_set_alloc(struct cache_sb *sb)
>   	bch_cache_accounting_init(&c->accounting, &c->cl);
>   
>   	memcpy(c->set_uuid, sb->set_uuid, 16);
> -	c->sb.block_size	= sb->block_size;
> -	c->sb.bucket_size	= sb->bucket_size;
> -	c->sb.nr_in_set		= sb->nr_in_set;
> -	c->sb.last_mount	= sb->last_mount;
> -	c->sb.version		= sb->version;
> -	if (c->sb.version >= BCACHE_SB_VERSION_CDEV_WITH_FEATURES) {
> -		c->sb.feature_compat = sb->feature_compat;
> -		c->sb.feature_ro_compat = sb->feature_ro_compat;
> -		c->sb.feature_incompat = sb->feature_incompat;
> -	}
>   
>   	c->bucket_bits		= ilog2(sb->bucket_size);
>   	c->block_bits		= ilog2(sb->block_size);
> 
Please fold it into patch 13, as then it's obvious why we don't need 
this copy actions anymore.

Cheers,

Hannes
Coly Li Aug. 17, 2020, 6:30 a.m. UTC | #2
On 2020/8/17 14:22, Hannes Reinecke wrote:
> On 8/15/20 6:10 AM, Coly Li wrote:
>> struct cache_sb embedded in struct cache_set is only partial used and
>> not a real copy from cache's in-memory super block. When removing the
>> embedded cache_set->sb, it is unncessary to copy data between these two
>> in-memory super blocks (cache_set->sb and cache->sb), it is sufficient
>> to just use cache->sb.
>>
>> This patch removes the data copy between these two in-memory super
>> blocks in bch_cache_set_alloc() and bcache_write_super(). In future
>> except for set_uuid, cache's super block will be referenced by cache
>> set, no copy any more.
>>
>> Signed-off-by: Coly Li <colyli@suse.de>
>> ---
>>   drivers/md/bcache/super.c | 22 +++-------------------
>>   1 file changed, 3 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
>> index 748b08ab4f11..05c5a7e867bb 100644
>> --- a/drivers/md/bcache/super.c
>> +++ b/drivers/md/bcache/super.c
>> @@ -350,16 +350,10 @@ void bcache_write_super(struct cache_set *c)
>>       down(&c->sb_write_mutex);
>>       closure_init(cl, &c->cl);
>>   -    c->sb.seq++;
>> +    ca->sb.seq++;
>>   -    if (c->sb.version > version)
>> -        version = c->sb.version;
>> -
>> -    ca->sb.version        = version;
>> -    ca->sb.seq        = c->sb.seq;
>> -    ca->sb.last_mount    = c->sb.last_mount;
>> -
>> -    SET_CACHE_SYNC(&ca->sb, CACHE_SYNC(&c->sb));
>> +    if (ca->sb.version < version)
>> +        ca->sb.version = version;
>>         bio_init(bio, ca->sb_bv, 1);
>>       bio_set_dev(bio, ca->bdev);
>> @@ -1860,16 +1854,6 @@ struct cache_set *bch_cache_set_alloc(struct
>> cache_sb *sb)
>>       bch_cache_accounting_init(&c->accounting, &c->cl);
>>         memcpy(c->set_uuid, sb->set_uuid, 16);
>> -    c->sb.block_size    = sb->block_size;
>> -    c->sb.bucket_size    = sb->bucket_size;
>> -    c->sb.nr_in_set        = sb->nr_in_set;
>> -    c->sb.last_mount    = sb->last_mount;
>> -    c->sb.version        = sb->version;
>> -    if (c->sb.version >= BCACHE_SB_VERSION_CDEV_WITH_FEATURES) {
>> -        c->sb.feature_compat = sb->feature_compat;
>> -        c->sb.feature_ro_compat = sb->feature_ro_compat;
>> -        c->sb.feature_incompat = sb->feature_incompat;
>> -    }
>>         c->bucket_bits        = ilog2(sb->bucket_size);
>>       c->block_bits        = ilog2(sb->block_size);
>>
> Please fold it into patch 13, as then it's obvious why we don't need
> this copy actions anymore.

Sure, I will do it in next version series.

Thanks.

Coly Li
diff mbox series

Patch

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 748b08ab4f11..05c5a7e867bb 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -350,16 +350,10 @@  void bcache_write_super(struct cache_set *c)
 	down(&c->sb_write_mutex);
 	closure_init(cl, &c->cl);
 
-	c->sb.seq++;
+	ca->sb.seq++;
 
-	if (c->sb.version > version)
-		version = c->sb.version;
-
-	ca->sb.version		= version;
-	ca->sb.seq		= c->sb.seq;
-	ca->sb.last_mount	= c->sb.last_mount;
-
-	SET_CACHE_SYNC(&ca->sb, CACHE_SYNC(&c->sb));
+	if (ca->sb.version < version)
+		ca->sb.version = version;
 
 	bio_init(bio, ca->sb_bv, 1);
 	bio_set_dev(bio, ca->bdev);
@@ -1860,16 +1854,6 @@  struct cache_set *bch_cache_set_alloc(struct cache_sb *sb)
 	bch_cache_accounting_init(&c->accounting, &c->cl);
 
 	memcpy(c->set_uuid, sb->set_uuid, 16);
-	c->sb.block_size	= sb->block_size;
-	c->sb.bucket_size	= sb->bucket_size;
-	c->sb.nr_in_set		= sb->nr_in_set;
-	c->sb.last_mount	= sb->last_mount;
-	c->sb.version		= sb->version;
-	if (c->sb.version >= BCACHE_SB_VERSION_CDEV_WITH_FEATURES) {
-		c->sb.feature_compat = sb->feature_compat;
-		c->sb.feature_ro_compat = sb->feature_ro_compat;
-		c->sb.feature_incompat = sb->feature_incompat;
-	}
 
 	c->bucket_bits		= ilog2(sb->bucket_size);
 	c->block_bits		= ilog2(sb->block_size);