From: Nick Piggin <nickpiggin@yahoo.com.au>
To: Hisashi Hifumi <hifumi.hisashi@oss.ntt.co.jp>
Cc: Hugh Dickins <hugh@veritas.com>,
akpm@linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: PageLRU can be non-atomic bit operation
Date: Tue, 24 Apr 2007 12:47:47 +1000 [thread overview]
Message-ID: <462D6FD3.2090805@yahoo.com.au> (raw)
In-Reply-To: <6.0.0.20.2.20070424100507.049670d0@172.19.0.2>
Hisashi Hifumi wrote:
>
> At 22:42 07/04/23, Hugh Dickins wrote:
> >On Mon, 23 Apr 2007, Hisashi Hifumi wrote:
> >> >No. The PG_lru flag bit is just one bit amongst many others:
> >> >what of concurrent operations changing other bits in that same
> >> >unsigned long e.g. trying to lock the page by setting PG_locked?
> >> >There are some places where such micro-optimizations can be made
> >> >(typically while first allocating the page); but in general, no.
> >>
> >> In i386 and x86_64, btsl is used to change page flag. In this case,
> if btsl
> >> without lock prefix
> >> set PG_locked and PG_lru flag concurrently, does only one operation
> >> succeed ?
> >
> >That's right: on an SMP machine, without the lock prefix, the operation
> >is no longer atomic: what's stored back may be missing the result of
> >one or the other of the racing operations.
> >
>
> In the case that changing the same bit concurrently, lock prefix or other
> spinlock is needed. But, I think that concurrent bit operation on
> different bits
> is just like OR operation , so lock prefix is not needed.
>
> AMD instruction manual says about bts that ,
>
> "Copies a bit, specified by bit index in a register or 8-bit immediate
> value (second operand), from a bit
> string (first operand), also called the bit base, to the carry flag (CF)
> of the rFLAGS register, and then
> sets the bit in the bit string to 1."
>
> BTS instruction is read-modify-write instruction on bit unit. So
> concurrent bit operation on different
> bits may be possible.
No matter what actual instruction is used, the SetPageLRU operation (ie.
without the double underscore prefix) must be atomic, and the __SetPageLRU
operation *can* be non-atomic if that would be faster.
As Hugh points out, we must have atomic ops here, so changing the generic
code to use the __ version is wrong. However if there is a faster way that
i386 can perform the atomic variant, then doing so will speed up the generic
code without breaking other architectures.
--
SUSE Labs, Novell Inc.
next prev parent reply other threads:[~2007-04-24 2:47 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-04-23 10:54 [PATCH] mm: PageLRU can be non-atomic bit operation Hisashi Hifumi
2007-04-23 11:16 ` Hugh Dickins
2007-04-23 12:34 ` Hisashi Hifumi
2007-04-23 13:42 ` Hugh Dickins
2007-04-24 1:54 ` Hisashi Hifumi
2007-04-24 2:29 ` KAMEZAWA Hiroyuki
2007-04-24 2:47 ` Nick Piggin [this message]
2007-04-24 8:12 ` Hisashi Hifumi
2007-04-24 10:13 ` Nick Piggin
2007-04-24 13:40 ` Hugh Dickins
2007-04-24 14:22 ` Andi Kleen
2007-04-25 8:56 ` Fernando Luis Vázquez Cao
2007-04-25 8:59 ` Andi Kleen
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=462D6FD3.2090805@yahoo.com.au \
--to=nickpiggin@yahoo.com.au \
--cc=akpm@linux-foundation.org \
--cc=hifumi.hisashi@oss.ntt.co.jp \
--cc=hugh@veritas.com \
--cc=linux-kernel@vger.kernel.org \
/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.