From: Si-Wei Liu <si-wei.liu@oracle.com>

The starting iova address to iterate iotlb map entry within a range
was set to an irrelevant value when passing to the itree_next()
iterator, although luckily it doesn't affect the outcome of finding
out the granule of the smallest iotlb map size. Fix the code to make
it consistent with the following for-loop.

Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 drivers/vdpa/mlx5/core/mr.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
index 7d0c83b5b071..8455f08f5d40 100644
--- a/drivers/vdpa/mlx5/core/mr.c
+++ b/drivers/vdpa/mlx5/core/mr.c
@@ -368,7 +368,6 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr
 	unsigned long lgcd = 0;
 	int log_entity_size;
 	unsigned long size;
-	u64 start = 0;
 	int err;
 	struct page *pg;
 	unsigned int nsg;
@@ -379,10 +378,9 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr
 	struct device *dma = mvdev->vdev.dma_dev;
 
 	for (map = vhost_iotlb_itree_first(iotlb, mr->start, mr->end - 1);
-	     map; map = vhost_iotlb_itree_next(map, start, mr->end - 1)) {
+	     map; map = vhost_iotlb_itree_next(map, mr->start, mr->end - 1)) {
 		size = maplen(map, mr);
 		lgcd = gcd(lgcd, size);
-		start += size;
 	}
 	log_entity_size = ilog2(lgcd);
 
-- 
2.46.1

On Mon, Oct 21, 2024 at 04:40:40PM +0300, Dragos Tatulea wrote:
> From: Si-Wei Liu <si-wei.liu@oracle.com>
> 
> The starting iova address to iterate iotlb map entry within a range
> was set to an irrelevant value when passing to the itree_next()
> iterator, although luckily it doesn't affect the outcome of finding
> out the granule of the smallest iotlb map size. Fix the code to make
> it consistent with the following for-loop.
> 
> Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")


But the cover letter says "that's why it does not have a fixes tag".
Confused.

> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
> ---
>  drivers/vdpa/mlx5/core/mr.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> index 7d0c83b5b071..8455f08f5d40 100644
> --- a/drivers/vdpa/mlx5/core/mr.c
> +++ b/drivers/vdpa/mlx5/core/mr.c
> @@ -368,7 +368,6 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr
>  	unsigned long lgcd = 0;
>  	int log_entity_size;
>  	unsigned long size;
> -	u64 start = 0;
>  	int err;
>  	struct page *pg;
>  	unsigned int nsg;
> @@ -379,10 +378,9 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr
>  	struct device *dma = mvdev->vdev.dma_dev;
>  
>  	for (map = vhost_iotlb_itree_first(iotlb, mr->start, mr->end - 1);
> -	     map; map = vhost_iotlb_itree_next(map, start, mr->end - 1)) {
> +	     map; map = vhost_iotlb_itree_next(map, mr->start, mr->end - 1)) {
>  		size = maplen(map, mr);
>  		lgcd = gcd(lgcd, size);
> -		start += size;
>  	}
>  	log_entity_size = ilog2(lgcd);
>  
> -- 
> 2.46.1

On 13.11.24 07:32, Michael S. Tsirkin wrote:
> On Mon, Oct 21, 2024 at 04:40:40PM +0300, Dragos Tatulea wrote:
>> From: Si-Wei Liu <si-wei.liu@oracle.com>
>>
>> The starting iova address to iterate iotlb map entry within a range
>> was set to an irrelevant value when passing to the itree_next()
>> iterator, although luckily it doesn't affect the outcome of finding
>> out the granule of the smallest iotlb map size. Fix the code to make
>> it consistent with the following for-loop.
>>
>> Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")
> 
> 
> But the cover letter says "that's why it does not have a fixes tag".
> Confused.
Sorry about that. Patch is fine with fixes tag, I forgot to drop that
part of the sentence from the cover letter.

Let me know if I need to resend something.

Thanks,
Dragos

On Wed, Nov 13, 2024 at 03:33:35PM +0100, Dragos Tatulea wrote:
> 
> 
> On 13.11.24 07:32, Michael S. Tsirkin wrote:
> > On Mon, Oct 21, 2024 at 04:40:40PM +0300, Dragos Tatulea wrote:
> >> From: Si-Wei Liu <si-wei.liu@oracle.com>
> >>
> >> The starting iova address to iterate iotlb map entry within a range
> >> was set to an irrelevant value when passing to the itree_next()
> >> iterator, although luckily it doesn't affect the outcome of finding
> >> out the granule of the smallest iotlb map size. Fix the code to make
> >> it consistent with the following for-loop.
> >>
> >> Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")
> > 
> > 
> > But the cover letter says "that's why it does not have a fixes tag".
> > Confused.
> Sorry about that. Patch is fine with fixes tag, I forgot to drop that
> part of the sentence from the cover letter.
> 
> Let me know if I need to resend something.
> 
> Thanks,
> Dragos

But why does it need the fixes tag? That one means "if you have
that hash, you need this patch". Pls do not abuse it for
optimizations.

On 13.11.24 15:49, Michael S. Tsirkin wrote:
> On Wed, Nov 13, 2024 at 03:33:35PM +0100, Dragos Tatulea wrote:
>>
>>
>> On 13.11.24 07:32, Michael S. Tsirkin wrote:
>>> On Mon, Oct 21, 2024 at 04:40:40PM +0300, Dragos Tatulea wrote:
>>>> From: Si-Wei Liu <si-wei.liu@oracle.com>
>>>>
>>>> The starting iova address to iterate iotlb map entry within a range
>>>> was set to an irrelevant value when passing to the itree_next()
>>>> iterator, although luckily it doesn't affect the outcome of finding
>>>> out the granule of the smallest iotlb map size. Fix the code to make
>>>> it consistent with the following for-loop.
>>>>
>>>> Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")
>>>
>>>
>>> But the cover letter says "that's why it does not have a fixes tag".
>>> Confused.
>> Sorry about that. Patch is fine with fixes tag, I forgot to drop that
>> part of the sentence from the cover letter.
>>
>> Let me know if I need to resend something.
>>
>> Thanks,
>> Dragos
> 
> But why does it need the fixes tag? That one means "if you have
> that hash, you need this patch". Pls do not abuse it for
> optimizations.
> 
Well, it is a fix but it happens that the code around still works without
this fix. I figured that it would be better to take it into older stable kernels
just like the other one. But if you consider it an improvement I will send a v2
without the Fixes tag.

Thanks,
Dragos

On Wed, Nov 13, 2024 at 04:01:05PM +0100, Dragos Tatulea wrote:
> 
> 
> On 13.11.24 15:49, Michael S. Tsirkin wrote:
> > On Wed, Nov 13, 2024 at 03:33:35PM +0100, Dragos Tatulea wrote:
> >>
> >>
> >> On 13.11.24 07:32, Michael S. Tsirkin wrote:
> >>> On Mon, Oct 21, 2024 at 04:40:40PM +0300, Dragos Tatulea wrote:
> >>>> From: Si-Wei Liu <si-wei.liu@oracle.com>
> >>>>
> >>>> The starting iova address to iterate iotlb map entry within a range
> >>>> was set to an irrelevant value when passing to the itree_next()
> >>>> iterator, although luckily it doesn't affect the outcome of finding
> >>>> out the granule of the smallest iotlb map size. Fix the code to make
> >>>> it consistent with the following for-loop.
> >>>>
> >>>> Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")
> >>>
> >>>
> >>> But the cover letter says "that's why it does not have a fixes tag".
> >>> Confused.
> >> Sorry about that. Patch is fine with fixes tag, I forgot to drop that
> >> part of the sentence from the cover letter.
> >>
> >> Let me know if I need to resend something.
> >>
> >> Thanks,
> >> Dragos
> > 
> > But why does it need the fixes tag? That one means "if you have
> > that hash, you need this patch". Pls do not abuse it for
> > optimizations.
> > 
> Well, it is a fix but it happens that the code around still works without
> this fix. I figured that it would be better to take it into older stable kernels
> just like the other one. But if you consider it an improvement I will send a v2
> without the Fixes tag.
> 
> Thanks,
> Dragos

No need.

On Mon, Oct 21, 2024 at 9:41 PM Dragos Tatulea <dtatulea@nvidia.com> wrote:
>
> From: Si-Wei Liu <si-wei.liu@oracle.com>
>
> The starting iova address to iterate iotlb map entry within a range
> was set to an irrelevant value when passing to the itree_next()
> iterator, although luckily it doesn't affect the outcome of finding
> out the granule of the smallest iotlb map size. Fix the code to make
> it consistent with the following for-loop.
>
> Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
> ---

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks