Re: Question about pte_offset_map_lock

[Minchan Kim <minchan.kim@gmail.com>]

Hi, Peter.

On Thu, Dec 17, 2009 at 5:01 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, 2009-12-17 at 11:46 +0900, Minchan Kim wrote:
>> It may be a dumb question.
>>
>> As I read the code of pte_lock, I have a question.
>> Now, there is pte_offset_map_lock following as.
>>
>> #define pte_offset_map_lock(mm, pmd, address, ptlp)     \
>> ({                                                      \
>>         spinlock_t *__ptl = pte_lockptr(mm, pmd);       \
>>         pte_t *__pte = pte_offset_map(pmd, address);    \
>>         *(ptlp) = __ptl;                                \
>>         spin_lock(__ptl);                               \
>>         __pte;                                          \
>> })
>>
>> Why do we grab the lock after getting __pte?
>> Is it possible that __pte might be changed before we grab the spin_lock?
>>
>> Some codes in mm checks original pte by pte_same.
>> There are not-checked cases in proc. As looking over the cases,
>> It seems no problem. But in future, new user of pte_offset_map_lock
>> could mistake with that?
>
> I think currently mmap_sem serializes all that. Cases like faults that
> take the mmap_sem for reading sometimes need the pte validation to check
> if they didn't race with another fault etc.
>
> But since mmap_sem is held for reading the vma can't dissapear and the
> memory map is stable in the sense that the page tables will be present
> (or can be instantiated when needed), since munmap removes the
> pagetables for vmas.

Thanks for answering dumb question.

It means sometimes pte split lock depends on mmap_sem.
First of all, Shouldn't we remove this dependency by reordering spinlock(__ptl)
in pte_offset_map_lock  for range locking?

>
>



-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>