[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <e45fa35c-315a-45ea-a14f-030c5258f533@arm.com>
Date: Mon, 23 Jun 2025 15:30:28 +0100
From: Ryan Roberts <ryan.roberts@....com>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Alexander Gordeev <agordeev@...ux.ibm.com>,
Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/vmalloc: leave lazy MMU mode on PTE mapping error
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@...el.com>
>>>>> Reported-by: Dan Carpenter <dan.carpenter@...aro.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@...ux.ibm.com>
>>>>
>>>> Ouch, sorry about that! The patch looks good to me so:
>>>>
>>>> Reviewed-by: Ryan Roberts <ryan.roberts@....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
>
Powered by blists - more mailing lists