On 28/11/2022 22:48, Alex Williamson wrote:
> On Mon, 28 Nov 2022 19:22:10 +0000
> Joao Martins <joao.m.martins@xxxxxxxxxx> wrote:
>
>> On 28/11/2022 19:12, Alex Williamson wrote:
>>> On Fri, 25 Nov 2022 17:29:56 +0000
>>> Joao Martins <joao.m.martins@xxxxxxxxxx> wrote:
>>>
>>>> Commit f38044e5ef58 ("vfio/iova_bitmap: Fix PAGE_SIZE unaligned bitmaps")
>>>> had fixed the unaligned bitmaps by capping the remaining iterable set at
>>>> the start of the bitmap. Although, that mistakenly worked around
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>> iova_bitmap_set() incorrectly setting bits across page boundary.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>
>>>> Fix this by reworking the loop inside iova_bitmap_set() to iterate over a
> ^^^^^^^^^^^...
>>>> range of bits to set (cur_bit .. last_bit) which may span different pinned
>>>> pages, thus updating @page_idx and @offset as it sets the bits. The
>>>> previous cap to the first page is now adjusted to be always accounted
>>>> rather than when there's only a non-zero pgoff.
>>>>
>>>> While at it, make @page_idx , @offset and @nbits to be unsigned int given
>>>> that it won't be more than 512 and 4096 respectively (even a bigger
>>>> PAGE_SIZE or a smaller struct page size won't make this bigger than the
>>>> above 32-bit max). Also, delete the stale kdoc on Return type.
>>>>
>>>> Cc: Avihai Horon <avihaih@xxxxxxxxxx>
>>>> Co-developed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
>>>> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
>>>> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
>>>
>>> Should this have:
>>>
>>> Fixes: f38044e5ef58 ("vfio/iova_bitmap: Fix PAGE_SIZE unaligned bitmaps")
>>>
>>> ?
>>
>> I was at two minds with the Fixes tag.
>>
>> The commit you referenced above is still a fix (or workaround), this patch is a
>> better fix that superseeds as opposed to fixing a bug that commit f38044e5ef58
>> introduced. So perhaps the right one ought to be:
>>
>> Fixes: 58ccf0190d19 ("vfio: Add an IOVA bitmap support")
>
> The above highlighted text certainly suggests that there's a fix to
> f38044e5ef58 here. We might still be iterating on a problem that was
> originally introduced in 58ccf0190d19, but this more directly addresses
> the version of that problem that exists after f38044e5ef58. I think
> it's more helpful for backporters to see this progression rather than
> two patches claiming to fix the same commit with one depending on
> another. If you'd rather that stable have a different backport that
> short circuits the interim fix in f38044e5ef58, that could be posted
> separately, but imo it's better to follow the mainline progression.
OK, thanks for the explanation -- lets then use f38044e5ef58 as the Fixes tag.
