All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Vladimir Davydov <vdavydov@virtuozzo.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Hugh Dickins <hughd@google.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	Sasha Levin <sasha.levin@oracle.com>,
	Minchan Kim <minchan@kernel.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH 4/4] mm: prepare page_referenced() and page_idle to new THP refcounting
Date: Thu, 5 Nov 2015 14:36:06 +0200	[thread overview]
Message-ID: <20151105123606.GE7614@node.shutemov.name> (raw)
In-Reply-To: <20151105120726.GD29259@esperanza>

On Thu, Nov 05, 2015 at 03:07:26PM +0300, Vladimir Davydov wrote:
> @@ -849,30 +836,23 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
>  		if (pmd_page(*pmd) != page)
>  			goto unlock_pmd;
>  
> -		if (vma->vm_flags & VM_LOCKED) {
> -			pra->vm_flags |= VM_LOCKED;
> -			ret = SWAP_FAIL; /* To break the loop */
> -			goto unlock_pmd;
> -		}
> -
> -		if (pmdp_clear_flush_young_notify(vma, address, pmd))
> -			referenced++;
> -		spin_unlock(ptl);
> +		pte = (pte_t *)pmd;

pmd_t and pte_t are not always compatible. We shouldn't pretend they are.
And we shouldn't use pte_unmap_unlock() to unlock pmd table.

What about interface like this (I'm not sure about helper's name):

void page_check_address_transhuge(struct page *page, struct mm_struct *mm,
                                   unsigned long address,
                                   pmd_t **pmdp, pte_t **ptep,
				   spinlock_t **ptlp);

page_check_address_transhuge(page, mm, address, &pmd, &pte, &ptl);
if (pmd) {
	/* handle pmd... */
} else if (pte) {
	/* handle pte... */
} else {
	return SWAP_AGAIN;
}

/* common stuff */

if (pmd)
	spin_unlock(ptl);
else 
	pte_unmap_unlock(pte, ptl);

/* ... */

The helper shouldn't set pmd if the page is tracked to pte.

-- 
 Kirill A. Shutemov

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Vladimir Davydov <vdavydov@virtuozzo.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Hugh Dickins <hughd@google.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	Sasha Levin <sasha.levin@oracle.com>,
	Minchan Kim <minchan@kernel.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH 4/4] mm: prepare page_referenced() and page_idle to new THP refcounting
Date: Thu, 5 Nov 2015 14:36:06 +0200	[thread overview]
Message-ID: <20151105123606.GE7614@node.shutemov.name> (raw)
In-Reply-To: <20151105120726.GD29259@esperanza>

On Thu, Nov 05, 2015 at 03:07:26PM +0300, Vladimir Davydov wrote:
> @@ -849,30 +836,23 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
>  		if (pmd_page(*pmd) != page)
>  			goto unlock_pmd;
>  
> -		if (vma->vm_flags & VM_LOCKED) {
> -			pra->vm_flags |= VM_LOCKED;
> -			ret = SWAP_FAIL; /* To break the loop */
> -			goto unlock_pmd;
> -		}
> -
> -		if (pmdp_clear_flush_young_notify(vma, address, pmd))
> -			referenced++;
> -		spin_unlock(ptl);
> +		pte = (pte_t *)pmd;

pmd_t and pte_t are not always compatible. We shouldn't pretend they are.
And we shouldn't use pte_unmap_unlock() to unlock pmd table.

What about interface like this (I'm not sure about helper's name):

void page_check_address_transhuge(struct page *page, struct mm_struct *mm,
                                   unsigned long address,
                                   pmd_t **pmdp, pte_t **ptep,
				   spinlock_t **ptlp);

page_check_address_transhuge(page, mm, address, &pmd, &pte, &ptl);
if (pmd) {
	/* handle pmd... */
} else if (pte) {
	/* handle pte... */
} else {
	return SWAP_AGAIN;
}

/* common stuff */

if (pmd)
	spin_unlock(ptl);
else 
	pte_unmap_unlock(pte, ptl);

/* ... */

The helper shouldn't set pmd if the page is tracked to pte.

-- 
 Kirill A. Shutemov

  reply	other threads:[~2015-11-05 12:36 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-03 15:26 [PATCH 0/4] Bugfixes for THP refcounting Kirill A. Shutemov
2015-11-03 15:26 ` Kirill A. Shutemov
2015-11-03 15:26 ` [PATCH 1/4] mm: do not crash on PageDoubleMap() for non-head pages Kirill A. Shutemov
2015-11-03 15:26   ` Kirill A. Shutemov
2015-11-03 15:26 ` [PATCH 2/4] mm: duplicate rmap reference for hugetlb pages as compound Kirill A. Shutemov
2015-11-03 15:26   ` Kirill A. Shutemov
2015-11-03 15:26 ` [PATCH 3/4] thp: fix split vs. unmap race Kirill A. Shutemov
2015-11-03 15:26   ` Kirill A. Shutemov
2015-11-03 15:26 ` [PATCH 4/4] mm: prepare page_referenced() and page_idle to new THP refcounting Kirill A. Shutemov
2015-11-03 15:26   ` Kirill A. Shutemov
2015-11-05  9:10   ` Vladimir Davydov
2015-11-05  9:10     ` Vladimir Davydov
2015-11-05  9:24     ` Kirill A. Shutemov
2015-11-05  9:24       ` Kirill A. Shutemov
2015-11-05 12:07       ` Vladimir Davydov
2015-11-05 12:07         ` Vladimir Davydov
2015-11-05 12:36         ` Kirill A. Shutemov [this message]
2015-11-05 12:36           ` Kirill A. Shutemov
2015-11-05 12:53           ` Vladimir Davydov
2015-11-05 12:53             ` Vladimir Davydov
2015-11-05 12:58             ` Kirill A. Shutemov
2015-11-05 12:58               ` Kirill A. Shutemov
2015-11-05 16:31               ` Vladimir Davydov
2015-11-05 16:31                 ` Vladimir Davydov
2015-11-06 14:37               ` [PATCH] mm: add page_check_address_transhuge helper Vladimir Davydov
2015-11-06 14:37                 ` Vladimir Davydov
2015-11-06 15:24                 ` Kirill A. Shutemov
2015-11-06 15:24                   ` Kirill A. Shutemov
2015-11-05 16:03   ` [PATCH 4/4] mm: prepare page_referenced() and page_idle to new THP refcounting Vladimir Davydov
2015-11-05 16:03     ` Vladimir Davydov
2015-11-05 17:27     ` Kirill A. Shutemov
2015-11-05 17:27       ` Kirill A. Shutemov
2015-11-06  0:32   ` Andrew Morton
2015-11-06  0:32     ` Andrew Morton
2015-11-06 10:29     ` Kirill A. Shutemov
2015-11-06 10:29       ` Kirill A. Shutemov
2015-11-06 22:39       ` Andrew Morton
2015-11-06 22:39         ` Andrew Morton
2015-11-08 23:40         ` Kirill A. Shutemov
2015-11-08 23:40           ` Kirill A. Shutemov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151105123606.GE7614@node.shutemov.name \
    --to=kirill@shutemov.name \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=hughd@google.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=sasha.levin@oracle.com \
    --cc=vdavydov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.