From: Baoquan He <bhe@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: cl@linux-foundation.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] perpuc: check pcpu_first_chunk and pcpu_reserved_chunk to avoid handling them twice
Date: Wed, 22 Jul 2015 08:03:57 +0800 [thread overview]
Message-ID: <20150722000357.GA1834@dhcp-17-102.nay.redhat.com> (raw)
In-Reply-To: <20150721152840.GG15934@mtj.duckdns.org>
Hi Tejun,
On 07/21/15 at 11:28am, Tejun Heo wrote:
> On Mon, Jul 20, 2015 at 10:55:29PM +0800, Baoquan He wrote:
> > In pcpu_setup_first_chunk() pcpu_reserved_chunk is assigned to point to
> > static chunk. While pcpu_first_chunk is got from below code:
> >
> > pcpu_first_chunk = dchunk ?: schunk;
> >
> > Then it could point to static chunk too if dynamic chunk doesn't exist. So
> > in this patch adding a check in percpu_init_late() to see if pcpu_first_chunk
> > is equal to pcpu_reserved_chunk. Only if they are not equal we add
> > pcpu_reserved_chunk to the target array.
>
> So, I don't think this is actually possible. dyn_size can't be zero
> so if reserved chunk is created, dyn chunk is also always created and
> thus first chunk can't equal reserved chunk. It might be useful to
> add some comments explaining this or maybe WARN_ON() but I don't think
> this path is necessary.
Thanks for your reviewing.
Yes, dyn_size can't be zero. But in pcpu_setup_first_chunk(), the local
variable dyn_size could be zero caused by below code:
if (ai->reserved_size) {
schunk->free_size = ai->reserved_size;
pcpu_reserved_chunk = schunk;
pcpu_reserved_chunk_limit = ai->static_size +
ai->reserved_size;
} else {
schunk->free_size = dyn_size;
dyn_size = 0; /* dynamic area covered
*/
}
So if no reserved_size dyn_size is assigned to zero, and is checked to
see if dchunk need be created in below code:
/* init dynamic chunk if necessary */
if (dyn_size) {
...
}
I think v1 patch is a little ugly, so made a v2 like this:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
next prev parent reply other threads:[~2015-07-22 0:04 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-20 14:55 [PATCH 1/3] percpu: clean up of schunk->map[] assignment in pcpu_setup_first_chunk Baoquan He
2015-07-20 14:55 ` [PATCH 2/3] perpuc: check pcpu_first_chunk and pcpu_reserved_chunk to avoid handling them twice Baoquan He
2015-07-21 15:28 ` Tejun Heo
2015-07-22 0:03 ` Baoquan He [this message]
2015-07-22 13:52 ` Tejun Heo
2015-07-22 14:29 ` Baoquan He
2015-07-22 14:37 ` Tejun Heo
2015-07-23 1:56 ` Baoquan He
2015-07-23 1:53 ` [PATCH v3 2/3] percpu: Add WARN_ON into percpu_init_late Baoquan He
2015-07-20 14:55 ` [PATCH 3/3] percpu: add macro PCPU_CHUNK_AREA_IN_USE Baoquan He
2015-07-20 15:35 ` Christoph Lameter
2015-07-22 0:25 ` Baoquan He
2015-07-21 15:30 ` Tejun Heo
2015-07-22 0:28 ` Baoquan He
2015-07-22 13:53 ` Tejun Heo
2015-07-23 1:55 ` Baoquan He
2015-07-23 1:50 ` [PATCH v2 3/3] percpu: add macro PCPU_MAP_BUSY Baoquan He
2015-07-20 15:32 ` [PATCH 1/3] percpu: clean up of schunk->map[] assignment in pcpu_setup_first_chunk Christoph Lameter
2015-07-21 15:33 ` Tejun Heo
2015-07-22 0:56 ` Baoquan He
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=20150722000357.GA1834@dhcp-17-102.nay.redhat.com \
--to=bhe@redhat.com \
--cc=cl@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=tj@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox