| Message ID | 20241021134040.975221-3-dtatulea@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | vdpa/mlx5: Iova mapping related fixes | expand |
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
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.
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);