Re: [PATCH] mm/vmalloc: leave lazy MMU mode on PTE mapping error

[Ryan Roberts <ryan.roberts@arm.com>]

On 23/06/2025 14:53, Dan Carpenter wrote:
> On Mon, Jun 23, 2025 at 02:31:48PM +0100, Ryan Roberts wrote:
>> On 23/06/2025 14:03, Alexander Gordeev wrote:
>>> On Mon, Jun 23, 2025 at 01:37:11PM +0100, Ryan Roberts wrote:
>>>> On 23/06/2025 08:57, Alexander Gordeev wrote:
>>>>> Function vmap_pages_pte_range() enters the lazy MMU mode,
>>>>> but fails to leave it in case an error is encountered.
>>>>>
>>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>>> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
>>>>> Closes: https://lore.kernel.org/r/202506132017.T1l1l6ME-lkp@intel.com/
>>>>> Fixes: 44562c71e2cf ("mm/vmalloc: Enter lazy mmu mode while manipulating vmalloc ptes")
>>>>> Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
>>>>
>>>> Ouch, sorry about that! The patch looks good to me so:
>>>>
>>>> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
>>>>
>>>> I wonder an aditional Fixes: should be added for Commit 2ba3e6947aed
>>>> ("mm/vmalloc: track which page-table levels were modified") though? That's the
>>>> one that added the "*mask |= PGTBL_PTE_MODIFIED;" which would have also been
>>>> skipped if an error occured before this patch.
>>>
>>> Good catch! I think it certainly needs to be reported with Fixes
>>> and I even doubt whether your commit should be mentioned at all?
>>
>> Well I would certainly argue that my patch is broken as is. So happy to have 2
>> Fixes: tags. But I'm not really sure what the rules are here...
> 
> I would only list the older commit 2ba3e6947aed ("mm/vmalloc: track
> which page-table levels were modified").  The static checker warning
> came later, but it's not really the important bit.  It's just one bug.

Given smatch caught the locking bug, I wonder if it could be taught to look for
lazy_mmu issues? i.e. unbalanced enter/leave, nesting and read hazards. I think
Alexander previously found a read hazard so I wouldn't be surprised if there are
more.

> 
> We'll have to hand edit the commit if we want to backport it so that's
> a separate issue.
> 
> regards,
> dan carpenter
>