From: Seth Jennings <sjenning@linux.vnet.ibm.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Andrew Morton <akpm@linux-foundation.org>,
Nitin Gupta <ngupta@vflare.org>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Dan Magenheimer <dan.magenheimer@oracle.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
devel@driverdev.osuosl.org, xiaoguangrong@linux.vnet.ibm.com
Subject: Re: [PATCH 0/2] revert changes to zcache_do_preload()
Date: Fri, 24 Aug 2012 15:57:44 -0500 [thread overview]
Message-ID: <5037EAC8.6080403@linux.vnet.ibm.com> (raw)
In-Reply-To: <20120823232845.GE5369@bbox>
On 08/23/2012 06:28 PM, Minchan Kim wrote:
> Okay, then, why do you think the patchsets are culprit?
> I didn't look the cleanup patch series of Xiao at that time
> so I can be wrong but as I just look through patch of
> "zcache: optimize zcache_do_preload", I can't find any fault
> because zcache_put_page checks irq_disable so we don't need
> to disable preemption so it seems that patch is correct to me.
> If the race happens by preemption, BUG_ON in zcache_put_page
> should catch it.
>
> What do you mean? Do you have any clue in your mind?
>
> The commits undermine an assumption made by tmem_put() in
> the cleancache path that preemption is disabled.
I do not have an explanation right now for why these commits
expose this issue. The patch looks like it should be fine
to me, hence my Ack at the time.
I understand and agree with you that the zcache shim
functions zcache_put_page(), zcache_get_page(),
zcache_flush_page(), and zcache_flush_object() all disable
interrupts (or make sure that interrupts are already
disabled) which implicitly disables preemption.
I'm still trying to find root cause here.
Seth
--
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: Seth Jennings <sjenning@linux.vnet.ibm.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Andrew Morton <akpm@linux-foundation.org>,
Nitin Gupta <ngupta@vflare.org>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Dan Magenheimer <dan.magenheimer@oracle.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
devel@driverdev.osuosl.org, xiaoguangrong@linux.vnet.ibm.com
Subject: Re: [PATCH 0/2] revert changes to zcache_do_preload()
Date: Fri, 24 Aug 2012 15:57:44 -0500 [thread overview]
Message-ID: <5037EAC8.6080403@linux.vnet.ibm.com> (raw)
In-Reply-To: <20120823232845.GE5369@bbox>
On 08/23/2012 06:28 PM, Minchan Kim wrote:
> Okay, then, why do you think the patchsets are culprit?
> I didn't look the cleanup patch series of Xiao at that time
> so I can be wrong but as I just look through patch of
> "zcache: optimize zcache_do_preload", I can't find any fault
> because zcache_put_page checks irq_disable so we don't need
> to disable preemption so it seems that patch is correct to me.
> If the race happens by preemption, BUG_ON in zcache_put_page
> should catch it.
>
> What do you mean? Do you have any clue in your mind?
>
> The commits undermine an assumption made by tmem_put() in
> the cleancache path that preemption is disabled.
I do not have an explanation right now for why these commits
expose this issue. The patch looks like it should be fine
to me, hence my Ack at the time.
I understand and agree with you that the zcache shim
functions zcache_put_page(), zcache_get_page(),
zcache_flush_page(), and zcache_flush_object() all disable
interrupts (or make sure that interrupts are already
disabled) which implicitly disables preemption.
I'm still trying to find root cause here.
Seth
next prev parent reply other threads:[~2012-08-24 20:58 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-23 15:33 [PATCH 0/2] revert changes to zcache_do_preload() Seth Jennings
2012-08-23 15:33 ` Seth Jennings
2012-08-23 15:33 ` [PATCH 1/2] Revert "staging: zcache: cleanup zcache_do_preload and zcache_put_page" Seth Jennings
2012-08-23 15:33 ` Seth Jennings
2012-08-23 15:33 ` [PATCH 2/2] Revert "staging: zcache: optimize zcache_do_preload" Seth Jennings
2012-08-23 15:33 ` Seth Jennings
2012-08-23 20:56 ` [PATCH 0/2] revert changes to zcache_do_preload() Minchan Kim
2012-08-23 20:56 ` Minchan Kim
2012-08-23 22:10 ` Seth Jennings
2012-08-23 22:10 ` Seth Jennings
2012-08-23 23:28 ` Minchan Kim
2012-08-23 23:28 ` Minchan Kim
2012-08-24 2:21 ` Xiao Guangrong
2012-08-24 2:21 ` Xiao Guangrong
2012-08-24 20:57 ` Seth Jennings [this message]
2012-08-24 20:57 ` Seth Jennings
2012-08-29 17:42 ` Seth Jennings
2012-08-29 17:42 ` Seth Jennings
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=5037EAC8.6080403@linux.vnet.ibm.com \
--to=sjenning@linux.vnet.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=dan.magenheimer@oracle.com \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=konrad.wilk@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=minchan@kernel.org \
--cc=ngupta@vflare.org \
--cc=xiaoguangrong@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.