[RFC] virtio-mem: virtio_mem_init: Access bb_size just in BBM mode

[RFC] virtio-mem: virtio_mem_init: Access bb_size just in BBM mode

[Hui Zhu]

From: Hui Zhu <teawaterz@linux.alibaba.com>

/* In BBM, we also want at least two big blocks. */
vm->offline_threshold = max_t(uint64_t, 2 * vm->bbm.bb_size,
			      vm->offline_threshold);
This line does not modify vm->offline_threshold depending on the data in
vm->sbm that shares this address is 0 in SBM mode.
I think it might be difficult to make sure when we change this in the
future.

This commit adds an if to make sure that this line just be executed in
BBM mode.

Signed-off-by: Hui Zhu <teawaterz@linux.alibaba.com>
---
 drivers/virtio/virtio_mem.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 10ec60d..8185916 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -2472,8 +2472,9 @@ static int virtio_mem_init(struct virtio_mem *vm)
 	vm->offline_threshold = max_t(uint64_t, 2 * memory_block_size_bytes(),
 				      VIRTIO_MEM_DEFAULT_OFFLINE_THRESHOLD);
 	/* In BBM, we also want at least two big blocks. */
-	vm->offline_threshold = max_t(uint64_t, 2 * vm->bbm.bb_size,
-				      vm->offline_threshold);
+	if (!vm->in_sbm)
+		vm->offline_threshold = max_t(uint64_t, 2 * vm->bbm.bb_size,
+					      vm->offline_threshold);
 
 	dev_info(&vm->vdev->dev, "start address: 0x%llx", vm->addr);
 	dev_info(&vm->vdev->dev, "region size: 0x%llx", vm->region_size);
-- 
1.8.3.1

Re: [RFC] virtio-mem: virtio_mem_init: Access bb_size just in BBM mode

[David Hildenbrand]

On 15.06.21 08:59, Hui Zhu wrote:
> From: Hui Zhu <teawaterz@linux.alibaba.com>
> 
> /* In BBM, we also want at least two big blocks. */
> vm->offline_threshold = max_t(uint64_t, 2 * vm->bbm.bb_size,
> 			      vm->offline_threshold);
> This line does not modify vm->offline_threshold depending on the data in
> vm->sbm that shares this address is 0 in SBM mode.
> I think it might be difficult to make sure when we change this in the
> future.
> 
> This commit adds an if to make sure that this line just be executed in
> BBM mode.
> 
> Signed-off-by: Hui Zhu <teawaterz@linux.alibaba.com>
> ---
>   drivers/virtio/virtio_mem.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> index 10ec60d..8185916 100644
> --- a/drivers/virtio/virtio_mem.c
> +++ b/drivers/virtio/virtio_mem.c
> @@ -2472,8 +2472,9 @@ static int virtio_mem_init(struct virtio_mem *vm)
>   	vm->offline_threshold = max_t(uint64_t, 2 * memory_block_size_bytes(),
>   				      VIRTIO_MEM_DEFAULT_OFFLINE_THRESHOLD);
>   	/* In BBM, we also want at least two big blocks. */
> -	vm->offline_threshold = max_t(uint64_t, 2 * vm->bbm.bb_size,
> -				      vm->offline_threshold);
> +	if (!vm->in_sbm)
> +		vm->offline_threshold = max_t(uint64_t, 2 * vm->bbm.bb_size,
> +					      vm->offline_threshold);
>   
>   	dev_info(&vm->vdev->dev, "start address: 0x%llx", vm->addr);
>   	dev_info(&vm->vdev->dev, "region size: 0x%llx", vm->region_size);
> 

Good that you also spotted it, I already have a fix pending for that :)

https://lkml.kernel.org/r/20210602185720.31821-2-david@redhat.com

-- 
Thanks,

David / dhildenb

Re: [RFC] virtio-mem: virtio_mem_init: Access bb_size just in BBM mode

[teawater]

> 2021年6月15日 15:43，David Hildenbrand <david@redhat.com> 写道：
> 
> On 15.06.21 08:59, Hui Zhu wrote:
>> From: Hui Zhu <teawaterz@linux.alibaba.com>
>> /* In BBM, we also want at least two big blocks. */
>> vm->offline_threshold = max_t(uint64_t, 2 * vm->bbm.bb_size,
>> 			      vm->offline_threshold);
>> This line does not modify vm->offline_threshold depending on the data in
>> vm->sbm that shares this address is 0 in SBM mode.
>> I think it might be difficult to make sure when we change this in the
>> future.
>> This commit adds an if to make sure that this line just be executed in
>> BBM mode.
>> Signed-off-by: Hui Zhu <teawaterz@linux.alibaba.com>
>> ---
>>  drivers/virtio/virtio_mem.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>> index 10ec60d..8185916 100644
>> --- a/drivers/virtio/virtio_mem.c
>> +++ b/drivers/virtio/virtio_mem.c
>> @@ -2472,8 +2472,9 @@ static int virtio_mem_init(struct virtio_mem *vm)
>>  	vm->offline_threshold = max_t(uint64_t, 2 * memory_block_size_bytes(),
>>  				      VIRTIO_MEM_DEFAULT_OFFLINE_THRESHOLD);
>>  	/* In BBM, we also want at least two big blocks. */
>> -	vm->offline_threshold = max_t(uint64_t, 2 * vm->bbm.bb_size,
>> -				      vm->offline_threshold);
>> +	if (!vm->in_sbm)
>> +		vm->offline_threshold = max_t(uint64_t, 2 * vm->bbm.bb_size,
>> +					      vm->offline_threshold);
>>    	dev_info(&vm->vdev->dev, "start address: 0x%llx", vm->addr);
>>  	dev_info(&vm->vdev->dev, "region size: 0x%llx", vm->region_size);
> 
> Good that you also spotted it, I already have a fix pending for that :)
> 
> https://lkml.kernel.org/r/20210602185720.31821-2-david@redhat.com

Oops!  Please ignore this RFC.

Best,
Hui

> 
> -- 
> Thanks,
> 
> David / dhildenb