On 29/11/2022 11:28, Joao Martins wrote:
> 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.
Meanwhile I've sent v2 with the tags I got here, as I'm going to be out the rest
of the week.
