[PATCH 1/3] bcache: Remove redundant parameter for cache_alloc()

[PATCH 1/3] bcache: Remove redundant parameter for cache_alloc()

[Yijing Wang]

Cache_sb is not used in cache_alloc, and we have copied
sb info to cache->sb already, remove it.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/md/bcache/super.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index a296425..1ccb9be 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1803,7 +1803,7 @@ void bch_cache_release(struct kobject *kobj)
 	module_put(THIS_MODULE);
 }
 
-static int cache_alloc(struct cache_sb *sb, struct cache *ca)
+static int cache_alloc(struct cache *ca)
 {
 	size_t free;
 	struct bucket *b;
@@ -1858,7 +1858,7 @@ static int register_cache(struct cache_sb *sb, struct page *sb_page,
 	if (blk_queue_discard(bdev_get_queue(ca->bdev)))
 		ca->discard = CACHE_DISCARD(&ca->sb);
 
-	ret = cache_alloc(sb, ca);
+	ret = cache_alloc(ca);
 	if (ret != 0)
 		goto err;
 
-- 
1.7.1

[PATCH 2/3] bcache: update document info

[Yijing Wang]

There is no return in continue_at(), update the documentation.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/md/bcache/closure.c |    2 +-
 drivers/md/bcache/closure.h |    3 ---
 2 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c
index 9eaf1d6..864e673 100644
--- a/drivers/md/bcache/closure.c
+++ b/drivers/md/bcache/closure.c
@@ -112,7 +112,7 @@ bool closure_wait(struct closure_waitlist *waitlist, struct closure *cl)
 EXPORT_SYMBOL(closure_wait);
 
 /**
- * closure_sync - sleep until a closure a closure has nothing left to wait on
+ * closure_sync - sleep until a closure has nothing left to wait on
  *
  * Sleeps until the refcount hits 1 - the thread that's running the closure owns
  * the last refcount.
diff --git a/drivers/md/bcache/closure.h b/drivers/md/bcache/closure.h
index 782cc2c..f51188d 100644
--- a/drivers/md/bcache/closure.h
+++ b/drivers/md/bcache/closure.h
@@ -31,9 +31,6 @@
  * passing it, as you might expect, the function to run when nothing is pending
  * and the workqueue to run that function out of.
  *
- * continue_at() also, critically, is a macro that returns the calling function.
- * There's good reason for this.
- *
  * To use safely closures asynchronously, they must always have a refcount while
  * they are running owned by the thread that is running them. Otherwise, suppose
  * you submit some bios and wish to have a function run when they all complete:
-- 
1.7.1

[PATCH 3/3] bcache: Remove redundant block_size assignment

[Yijing Wang]

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/md/bcache/super.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 1ccb9be..64d0be6 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -134,7 +134,6 @@ static const char *read_super(struct cache_sb *sb, struct block_device *bdev,
 	case BCACHE_SB_VERSION_CDEV:
 	case BCACHE_SB_VERSION_CDEV_WITH_UUID:
 		sb->nbuckets	= le64_to_cpu(s->nbuckets);
-		sb->block_size	= le16_to_cpu(s->block_size);
 		sb->bucket_size	= le16_to_cpu(s->bucket_size);
 
 		sb->nr_in_set	= le16_to_cpu(s->nr_in_set);
-- 
1.7.1

Re: [PATCH 3/3] bcache: Remove redundant block_size assignment

[Coly Li]

在 16/5/10 下午4:38, Yijing Wang 写道:
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
>  drivers/md/bcache/super.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 1ccb9be..64d0be6 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -134,7 +134,6 @@ static const char *read_super(struct cache_sb *sb, struct block_device *bdev,
>  	case BCACHE_SB_VERSION_CDEV:
>  	case BCACHE_SB_VERSION_CDEV_WITH_UUID:
>  		sb->nbuckets	= le64_to_cpu(s->nbuckets);
> -		sb->block_size	= le16_to_cpu(s->block_size);
>  		sb->bucket_size	= le16_to_cpu(s->bucket_size);
>  
>  		sb->nr_in_set	= le16_to_cpu(s->nr_in_set);
> 

Yes, this assignment is redundant. Could you please to add some comments
to describe this patch before your Signed-off-by ? This will be
informative and helpful for others who may back port the patches.

Thanks.

-- 
Coly Li

Re: [PATCH 3/3] bcache: Remove redundant block_size assignment

[wangyijing]

在 2016/5/10 22:51, Coly Li 写道:
> 在 16/5/10 下午4:38, Yijing Wang 写道:
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> ---
>>  drivers/md/bcache/super.c |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
>> index 1ccb9be..64d0be6 100644
>> --- a/drivers/md/bcache/super.c
>> +++ b/drivers/md/bcache/super.c
>> @@ -134,7 +134,6 @@ static const char *read_super(struct cache_sb *sb, struct block_device *bdev,
>>  	case BCACHE_SB_VERSION_CDEV:
>>  	case BCACHE_SB_VERSION_CDEV_WITH_UUID:
>>  		sb->nbuckets	= le64_to_cpu(s->nbuckets);
>> -		sb->block_size	= le16_to_cpu(s->block_size);
>>  		sb->bucket_size	= le16_to_cpu(s->bucket_size);
>>  
>>  		sb->nr_in_set	= le16_to_cpu(s->nr_in_set);
>>
> 
> Yes, this assignment is redundant. Could you please to add some comments
> to describe this patch before your Signed-off-by ? This will be
> informative and helpful for others who may back port the patches.

OK.

Thanks!
Yijing.

> 
> Thanks.
> 

Re: [PATCH 3/3] bcache: Remove redundant block_size assignment

[Eric Wheeler]

On Tue, 10 May 2016, Yijing Wang wrote:

> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
>  drivers/md/bcache/super.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 1ccb9be..64d0be6 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -134,7 +134,6 @@ static const char *read_super(struct cache_sb *sb, struct block_device *bdev,
>  	case BCACHE_SB_VERSION_CDEV:
>  	case BCACHE_SB_VERSION_CDEV_WITH_UUID:
>  		sb->nbuckets	= le64_to_cpu(s->nbuckets);
> -		sb->block_size	= le16_to_cpu(s->block_size);
>  		sb->bucket_size	= le16_to_cpu(s->bucket_size);


Are you sure we want to do that?  

The bcache superblock allows us to assign the block size presented by 
bcache and we wouldn't want 4k users to suddenly revert to 512b.  


--
Eric Wheeler



>  
>  		sb->nr_in_set	= le16_to_cpu(s->nr_in_set);
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 3/3] bcache: Remove redundant block_size assignment

[wangyijing]

在 2016/5/11 9:41, Eric Wheeler 写道:
> On Tue, 10 May 2016, Yijing Wang wrote:
> 
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> ---
>>  drivers/md/bcache/super.c |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
>> index 1ccb9be..64d0be6 100644
>> --- a/drivers/md/bcache/super.c
>> +++ b/drivers/md/bcache/super.c
>> @@ -134,7 +134,6 @@ static const char *read_super(struct cache_sb *sb, struct block_device *bdev,
>>  	case BCACHE_SB_VERSION_CDEV:
>>  	case BCACHE_SB_VERSION_CDEV_WITH_UUID:
>>  		sb->nbuckets	= le64_to_cpu(s->nbuckets);
>> -		sb->block_size	= le16_to_cpu(s->block_size);
>>  		sb->bucket_size	= le16_to_cpu(s->bucket_size);
> 
> 
> Are you sure we want to do that?  
> 
> The bcache superblock allows us to assign the block size presented by 
> bcache and we wouldn't want 4k users to suddenly revert to 512b.  

Hi, I have nothing functional to change here, we have assigned block_size already before here.

	sb->block_size	= le16_to_cpu(s->block_size);    //first assignment

	err = "Superblock block size smaller than device block size";
	if (sb->block_size << 9 < bdev_logical_block_size(bdev))
		goto err;

	switch (sb->version) {
	case BCACHE_SB_VERSION_BDEV:
		sb->data_offset	= BDEV_DATA_START_DEFAULT;
		break;
	case BCACHE_SB_VERSION_BDEV_WITH_OFFSET:
		sb->data_offset	= le64_to_cpu(s->data_offset);

		err = "Bad data offset";
		if (sb->data_offset < BDEV_DATA_START_DEFAULT)
			goto err;

		break;
	case BCACHE_SB_VERSION_CDEV:
	case BCACHE_SB_VERSION_CDEV_WITH_UUID:
		sb->nbuckets	= le64_to_cpu(s->nbuckets);
		sb->block_size	= le16_to_cpu(s->block_size);       //redundant one

Thanks!
Yijing.


> 
> 
> --
> Eric Wheeler
> 
> 
> 
>>  
>>  		sb->nr_in_set	= le16_to_cpu(s->nr_in_set);
>> -- 
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> .
> 

Re: [PATCH 3/3] bcache: Remove redundant block_size assignment

[Eric Wheeler]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2302 bytes --]

On Wed, 11 May 2016, wangyijing wrote:

> 
> 
> 在 2016/5/11 9:41, Eric Wheeler 写道:
> > On Tue, 10 May 2016, Yijing Wang wrote:
> > 
> >> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> >> ---
> >>  drivers/md/bcache/super.c |    1 -
> >>  1 files changed, 0 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> >> index 1ccb9be..64d0be6 100644
> >> --- a/drivers/md/bcache/super.c
> >> +++ b/drivers/md/bcache/super.c
> >> @@ -134,7 +134,6 @@ static const char *read_super(struct cache_sb *sb, struct block_device *bdev,
> >>  	case BCACHE_SB_VERSION_CDEV:
> >>  	case BCACHE_SB_VERSION_CDEV_WITH_UUID:
> >>  		sb->nbuckets	= le64_to_cpu(s->nbuckets);
> >> -		sb->block_size	= le16_to_cpu(s->block_size);
> >>  		sb->bucket_size	= le16_to_cpu(s->bucket_size);
> > 
> > 
> > Are you sure we want to do that?  
> > 
> > The bcache superblock allows us to assign the block size presented by 
> > bcache and we wouldn't want 4k users to suddenly revert to 512b.  
> 
> Hi, I have nothing functional to change here, we have assigned block_size already before here.
> 
> 	sb->block_size	= le16_to_cpu(s->block_size);    //first assignment

I agree. Ack'ed.   


--
Eric Wheeler


> 
> 	err = "Superblock block size smaller than device block size";
> 	if (sb->block_size << 9 < bdev_logical_block_size(bdev))
> 		goto err;
> 
> 	switch (sb->version) {
> 	case BCACHE_SB_VERSION_BDEV:
> 		sb->data_offset	= BDEV_DATA_START_DEFAULT;
> 		break;
> 	case BCACHE_SB_VERSION_BDEV_WITH_OFFSET:
> 		sb->data_offset	= le64_to_cpu(s->data_offset);
> 
> 		err = "Bad data offset";
> 		if (sb->data_offset < BDEV_DATA_START_DEFAULT)
> 			goto err;
> 
> 		break;
> 	case BCACHE_SB_VERSION_CDEV:
> 	case BCACHE_SB_VERSION_CDEV_WITH_UUID:
> 		sb->nbuckets	= le64_to_cpu(s->nbuckets);
> 		sb->block_size	= le16_to_cpu(s->block_size);       //redundant one
> 
> Thanks!
> Yijing.
> 
> 
> > 
> > 
> > --
> > Eric Wheeler
> > 
> > 
> > 
> >>  
> >>  		sb->nr_in_set	= le16_to_cpu(s->nr_in_set);
> >> -- 
> >> 1.7.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> > 
> > .
> > 
> 
>