Kirill A. Shutemov wrote:
> On Fri, Mar 24, 2017 at 11:09:25AM -0500, Zi Yan wrote:
>> Kirill A. Shutemov wrote:
>>> On Mon, Mar 13, 2017 at 11:45:02AM -0400, Zi Yan wrote:
>>> Again. That's doesn't look right..
>> It will be changed:
>>
>> ptl = pmd_lock(mm, pmd);
>> +retry_locked:
>> + if (unlikely(!pmd_present(*pmd))) {
>> + if (likely(!(flags & FOLL_MIGRATION))) {
>> + spin_unlock(ptl);
>> + return no_page_table(vma, flags);
>> + }
>> + pmd_migration_entry_wait(mm, pmd);
>> + goto retry_locked;
>
> Nope. pmd_migration_entry_wait() unlocks the ptl.
Right. This chunk is wrong. pmd_migrtion_entry_wait() actually locks
pmd, then unlocks it and waits on the page if it is suitable.
An simple fix could be:
+retry_locked:
ptl = pmd_lock(mm, pmd);
+ if (unlikely(!pmd_present(*pmd))) {
+ spin_unlock(ptl);
+ if (likely(!(flags & FOLL_MIGRATION)))
+ return no_page_table(vma, flags);
+ pmd_migration_entry_wait(mm, pmd);
+ goto retry_locked;
+ }
Or is it better to change pmd_migration_entry_wait() to
void pmd_migration_entry_wait(struct mm_struct *mm, pmd_t *pmd,
spinlock_t *ptl)? So that if ptl is NULL, then it takes the pmd lock and
unlocks it; if ptl is specified, it only unlocks it. This can avoid the
redundant unlock and lock in the code above, when
pmd_migration_entry_wait() is called.
Thanks.
--
Best Regards,
Yan Zi
