All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Magenheimer <dan.magenheimer@oracle.com>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	jeremy@goop.org, hughd@google.com, ngupta@vflare.org,
	Konrad Wilk <konrad.wilk@oracle.com>,
	JBeulich@novell.com, Kurt Hackel <kurt.hackel@oracle.com>,
	npiggin@kernel.dk, akpm@linux-foundation.org, riel@redhat.com,
	hannes@cmpxchg.org, matthew@wil.cx,
	Chris Mason <chris.mason@oracle.com>,
	sjenning@linux.vnet.ibm.com, jackdachef@gmail.com,
	cyclonusj@gmail.com
Subject: RE: Subject: [PATCH V7 2/4] mm: frontswap: core code
Date: Mon, 29 Aug 2011 08:47:59 -0700 (PDT)	[thread overview]
Message-ID: <4be6abb6-7b82-4e64-9e27-cd0fe0c1e1b1@default> (raw)
In-Reply-To: <a2fc3885-b98d-4918-afcc-5eac083c7eb0@default>

> > My concern was race in counters. Even you allow race in frontswap_succ_puts++,
> >
> > Don't you need some lock for
> > 	sis->frontswap_pages++
> > 	sis->frontswap_pages--
> 
> Hmmm... OK, you've convinced me.  If this counter should be one and
> a race leaves it as zero, I think data corruption could result on
> a swapoff or partial swapoff.  And after thinking about it, I
> think I also need to check for locking on frontswap_set/clear
> as I don't think these bitfield modifiers are atomic.
> 
> Thanks for pointing this out.  Good catch!  I will need to
> play with this and test it so probably will not submit V8 until
> next week as today is a vacation day for me.

Silly me: Of course set_bit and clear_bit ARE atomic.  I will
post V8 later today with the only changes being frontswap_pages
is now a type atomic_t.

Thanks again for catching this, Kame!

Thanks,
Dan

WARNING: multiple messages have this Message-ID (diff)
From: Dan Magenheimer <dan.magenheimer@oracle.com>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	jeremy@goop.org, hughd@google.com, ngupta@vflare.org,
	Konrad Wilk <konrad.wilk@oracle.com>,
	JBeulich@novell.com, Kurt Hackel <kurt.hackel@oracle.com>,
	npiggin@kernel.dk, akpm@linux-foundation.org, riel@redhat.com,
	hannes@cmpxchg.org, matthew@wil.cx,
	Chris Mason <chris.mason@oracle.com>,
	sjenning@linux.vnet.ibm.com, jackdachef@gmail.com,
	cyclonusj@gmail.com
Subject: RE: Subject: [PATCH V7 2/4] mm: frontswap: core code
Date: Mon, 29 Aug 2011 08:47:59 -0700 (PDT)	[thread overview]
Message-ID: <4be6abb6-7b82-4e64-9e27-cd0fe0c1e1b1@default> (raw)
In-Reply-To: <a2fc3885-b98d-4918-afcc-5eac083c7eb0@default>

> > My concern was race in counters. Even you allow race in frontswap_succ_puts++,
> >
> > Don't you need some lock for
> > 	sis->frontswap_pages++
> > 	sis->frontswap_pages--
> 
> Hmmm... OK, you've convinced me.  If this counter should be one and
> a race leaves it as zero, I think data corruption could result on
> a swapoff or partial swapoff.  And after thinking about it, I
> think I also need to check for locking on frontswap_set/clear
> as I don't think these bitfield modifiers are atomic.
> 
> Thanks for pointing this out.  Good catch!  I will need to
> play with this and test it so probably will not submit V8 until
> next week as today is a vacation day for me.

Silly me: Of course set_bit and clear_bit ARE atomic.  I will
post V8 later today with the only changes being frontswap_pages
is now a type atomic_t.

Thanks again for catching this, Kame!

Thanks,
Dan

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-08-29 15:49 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-23 14:58 Subject: [PATCH V7 2/4] mm: frontswap: core code Dan Magenheimer
2011-08-23 14:58 ` Dan Magenheimer
2011-08-25  6:05 ` KAMEZAWA Hiroyuki
2011-08-25  6:05   ` KAMEZAWA Hiroyuki
2011-08-25 13:29   ` Seth Jennings
2011-08-25 13:29     ` Seth Jennings
2011-08-25 17:52     ` Dan Magenheimer
2011-08-25 17:52       ` Dan Magenheimer
2011-08-25 17:37   ` Dan Magenheimer
2011-08-25 17:37     ` Dan Magenheimer
2011-08-26  0:16     ` KAMEZAWA Hiroyuki
2011-08-26  0:16       ` KAMEZAWA Hiroyuki
2011-08-26 14:28       ` Dan Magenheimer
2011-08-26 14:28         ` Dan Magenheimer
2011-08-29 15:47         ` Dan Magenheimer [this message]
2011-08-29 15:47           ` Dan Magenheimer
2011-08-26 14:53       ` Dan Magenheimer
2011-08-26 14:53         ` Dan Magenheimer

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=4be6abb6-7b82-4e64-9e27-cd0fe0c1e1b1@default \
    --to=dan.magenheimer@oracle.com \
    --cc=JBeulich@novell.com \
    --cc=akpm@linux-foundation.org \
    --cc=chris.mason@oracle.com \
    --cc=cyclonusj@gmail.com \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=jackdachef@gmail.com \
    --cc=jeremy@goop.org \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=konrad.wilk@oracle.com \
    --cc=kurt.hackel@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=matthew@wil.cx \
    --cc=ngupta@vflare.org \
    --cc=npiggin@kernel.dk \
    --cc=riel@redhat.com \
    --cc=sjenning@linux.vnet.ibm.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.