All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Arcangeli <aarcange@redhat.com>
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	Vineet Gupta <vgupta@synopsys.com>,
	Russell King <linux@armlinux.org.uk>,
	Will Deacon <will.deacon@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Ralf Baechle <ralf@linux-mips.org>,
	"David S. Miller" <davem@davemloft.net>,
	"Aneesh Kumar K . V" <aneesh.kumar@linux.vnet.ibm.com>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	linux-arch@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCHv2 1/3] x86/mm: Provide pmdp_establish() helper
Date: Fri, 16 Jun 2017 15:36:00 +0200	[thread overview]
Message-ID: <20170616133600.GE11676@redhat.com> (raw)
In-Reply-To: <20170615145224.66200-2-kirill.shutemov@linux.intel.com>

Hello Krill,

On Thu, Jun 15, 2017 at 05:52:22PM +0300, Kirill A. Shutemov wrote:
> +static inline pmd_t pmdp_establish(pmd_t *pmdp, pmd_t pmd)
> +{
> +	pmd_t old;
> +
> +	/*
> +	 * We cannot assume what is value of pmd here, so there's no easy way
> +	 * to set if half by half. We have to fall back to cmpxchg64.
> +	 */
> +	{
> +		old = *pmdp;
> +	} while (cmpxchg64(&pmdp->pmd, old.pmd, pmd.pmd) != old.pmd);
> +
> +	return old;
> +}

I see further margin for optimization here (although it's only for PAE
x32..).

pmd is stable so we could do:

if (!(pmd & _PAGE_PRESENT)) {
   cast to split_pmd and use xchg on pmd_low like
   native_pmdp_get_and_clear and copy pmd_high non atomically
} else {
  the above cmpxchg64 loop
}

Now thinking about the above I had a second thought if pmdp_establish
is the right interface and if we shouldn't replace pmdp_establish with
pmdp_mknotpresent instead to skip the pmd & _PAGE_PRESENT check that
will always be true in practice, so pmdp_mknotpresent will call
internally pmd_mknotpresent and it won't have to check for pmd &
_PAGE_PRESENT and it would have no cons on x86-64.

WARNING: multiple messages have this Message-ID (diff)
From: Andrea Arcangeli <aarcange@redhat.com>
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	Vineet Gupta <vgupta@synopsys.com>,
	Russell King <linux@armlinux.org.uk>,
	Will Deacon <will.deacon@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Ralf Baechle <ralf@linux-mips.org>,
	"David S. Miller" <davem@davemloft.net>,
	"Aneesh Kumar K . V" <aneesh.kumar@linux.vnet.ibm.com>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	linux-arch@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCHv2 1/3] x86/mm: Provide pmdp_establish() helper
Date: Fri, 16 Jun 2017 15:36:00 +0200	[thread overview]
Message-ID: <20170616133600.GE11676@redhat.com> (raw)
In-Reply-To: <20170615145224.66200-2-kirill.shutemov@linux.intel.com>

Hello Krill,

On Thu, Jun 15, 2017 at 05:52:22PM +0300, Kirill A. Shutemov wrote:
> +static inline pmd_t pmdp_establish(pmd_t *pmdp, pmd_t pmd)
> +{
> +	pmd_t old;
> +
> +	/*
> +	 * We cannot assume what is value of pmd here, so there's no easy way
> +	 * to set if half by half. We have to fall back to cmpxchg64.
> +	 */
> +	{
> +		old = *pmdp;
> +	} while (cmpxchg64(&pmdp->pmd, old.pmd, pmd.pmd) != old.pmd);
> +
> +	return old;
> +}

I see further margin for optimization here (although it's only for PAE
x32..).

pmd is stable so we could do:

if (!(pmd & _PAGE_PRESENT)) {
   cast to split_pmd and use xchg on pmd_low like
   native_pmdp_get_and_clear and copy pmd_high non atomically
} else {
  the above cmpxchg64 loop
}

Now thinking about the above I had a second thought if pmdp_establish
is the right interface and if we shouldn't replace pmdp_establish with
pmdp_mknotpresent instead to skip the pmd & _PAGE_PRESENT check that
will always be true in practice, so pmdp_mknotpresent will call
internally pmd_mknotpresent and it won't have to check for pmd &
_PAGE_PRESENT and it would have no cons on x86-64.

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

  reply	other threads:[~2017-06-16 13:36 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-15 14:52 [HELP-NEEDED, PATCHv2 0/3] Do not loose dirty bit on THP pages Kirill A. Shutemov
2017-06-15 14:52 ` Kirill A. Shutemov
2017-06-15 14:52 ` [PATCHv2 1/3] x86/mm: Provide pmdp_establish() helper Kirill A. Shutemov
2017-06-15 14:52   ` Kirill A. Shutemov
2017-06-16 13:36   ` Andrea Arcangeli [this message]
2017-06-16 13:36     ` Andrea Arcangeli
2017-06-19 12:46     ` Kirill A. Shutemov
2017-06-19 12:46       ` Kirill A. Shutemov
2017-06-19  5:48   ` Martin Schwidefsky
2017-06-19  5:48     ` Martin Schwidefsky
2017-06-19  5:48     ` Martin Schwidefsky
2017-06-19  5:48     ` Martin Schwidefsky
2017-06-19 12:48     ` Kirill A. Shutemov
2017-06-19 12:48       ` Kirill A. Shutemov
2017-06-19 13:04       ` Martin Schwidefsky
2017-06-19 13:04         ` Martin Schwidefsky
2017-06-19 15:22   ` Catalin Marinas
2017-06-19 15:22     ` Catalin Marinas
2017-06-19 16:00     ` Kirill A. Shutemov
2017-06-19 16:00       ` Kirill A. Shutemov
2017-06-19 17:09       ` Catalin Marinas
2017-06-19 17:09         ` Catalin Marinas
2017-06-19 21:52         ` Kirill A. Shutemov
2017-06-19 21:52           ` Kirill A. Shutemov
2017-06-20 15:54           ` Catalin Marinas
2017-06-20 15:54             ` Catalin Marinas
2017-06-21  9:53             ` Kirill A. Shutemov
2017-06-21  9:53               ` Kirill A. Shutemov
2017-06-21 10:40               ` Catalin Marinas
2017-06-21 10:40                 ` Catalin Marinas
2017-06-21 11:27               ` Catalin Marinas
2017-06-21 11:27                 ` Catalin Marinas
2017-06-21 12:04                 ` Kirill A. Shutemov
2017-06-21 12:04                   ` Kirill A. Shutemov
2017-06-21 15:49                 ` Vineet Gupta
2017-06-21 15:49                   ` Vineet Gupta
2017-06-21 17:15                   ` Kirill A. Shutemov
2017-06-21 17:15                     ` Kirill A. Shutemov
2017-06-21 17:20                     ` Vineet Gupta
2017-06-21 17:20                       ` Vineet Gupta
2017-06-21 17:52                       ` Kirill A. Shutemov
2017-06-21 17:52                         ` Kirill A. Shutemov
2017-06-21 17:52                         ` Kirill A. Shutemov
2017-06-19 17:11   ` Nadav Amit
2017-06-19 17:11     ` Nadav Amit
2017-06-19 21:57     ` Kirill A. Shutemov
2017-06-19 21:57       ` Kirill A. Shutemov
2017-06-15 14:52 ` [PATCHv2 2/3] mm: Do not loose dirty and access bits in pmdp_invalidate() Kirill A. Shutemov
2017-06-15 14:52   ` Kirill A. Shutemov
2017-06-15 22:44   ` kbuild test robot
2017-06-15 22:44     ` kbuild test robot
2017-06-15 22:44     ` kbuild test robot
2017-06-16 13:40   ` Andrea Arcangeli
2017-06-16 13:40     ` Andrea Arcangeli
2017-06-19 13:29     ` Kirill A. Shutemov
2017-06-19 13:29       ` Kirill A. Shutemov
2017-06-15 14:52 ` [PATCHv2 3/3] mm: Use updated pmdp_invalidate() inteface to track dirty/accessed bits Kirill A. Shutemov
2017-06-15 14:52   ` Kirill A. Shutemov
2017-06-15 21:54   ` kbuild test robot
2017-06-15 21:54     ` kbuild test robot
2017-06-15 21:54     ` kbuild test robot
2017-06-15 23:02   ` kbuild test robot
2017-06-15 23:02     ` kbuild test robot
2017-06-15 23:02     ` kbuild test robot
2017-06-16  3:02   ` Minchan Kim
2017-06-16  3:02     ` Minchan Kim
2017-06-16 13:19     ` Kirill A. Shutemov
2017-06-16 13:19       ` Kirill A. Shutemov
2017-06-16 13:52       ` Minchan Kim
2017-06-16 13:52         ` Minchan Kim
2017-06-16 14:27         ` Andrea Arcangeli
2017-06-16 14:27           ` Andrea Arcangeli
2017-06-16 14:53           ` Minchan Kim
2017-06-16 14:53             ` Minchan Kim
2017-06-19 14:03             ` Kirill A. Shutemov
2017-06-19 14:03               ` Kirill A. Shutemov
2017-06-20  2:52               ` Minchan Kim
2017-06-20  2:52                 ` Minchan Kim
2017-06-20  9:57                 ` Minchan Kim
2017-06-20  9:57                   ` Minchan Kim
2017-06-16 11:31   ` Aneesh Kumar K.V
2017-06-16 11:31     ` Aneesh Kumar K.V
2017-06-16 11:31     ` Aneesh Kumar K.V
2017-06-16 11:31     ` Aneesh Kumar K.V
2017-06-16 13:21     ` Kirill A. Shutemov
2017-06-16 13:21       ` Kirill A. Shutemov
2017-06-16 15:57       ` Aneesh Kumar K.V
2017-06-16 15:57         ` Aneesh Kumar K.V

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=20170616133600.GE11676@redhat.com \
    --to=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=catalin.marinas@arm.com \
    --cc=davem@davemloft.net \
    --cc=heiko.carstens@de.ibm.com \
    --cc=hpa@zytor.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux@armlinux.org.uk \
    --cc=mingo@kernel.org \
    --cc=ralf@linux-mips.org \
    --cc=schwidefsky@de.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=vbabka@suse.cz \
    --cc=vgupta@synopsys.com \
    --cc=will.deacon@arm.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.