From: Dave Hansen <dave@linux.vnet.ibm.com>
To: Nathan Fontenot <nfont@austin.ibm.com>
Cc: linux-mm@kvack.org, Greg KH <greg@kroah.com>,
linux-kernel@vger.kernel.org,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
linuxppc-dev@ozlabs.org
Subject: Re: [PATCH 0/8] De-couple sysfs memory directories from memory sections
Date: Wed, 22 Sep 2010 11:58:49 -0700 [thread overview]
Message-ID: <1285181929.3292.6287.camel@nimitz> (raw)
In-Reply-To: <4C9A4DBB.6080500@austin.ibm.com>
On Wed, 2010-09-22 at 13:40 -0500, Nathan Fontenot wrote:
> On 09/22/2010 10:20 AM, Dave Hansen wrote:
> > and phys_index's calculation needs to be:
> >
> > mem->start_phys_index * SECTION_SIZE / memory_block_size_bytes()
>
> I'm not sure if I follow where you suggest using this formula. Is this
> instead of what is used now, the base_memory_block_id() calculation?
>
> If so, then I'm not sure it would work. The formula used in base_memory_block_id()
> is done because the memory sections are not guaranteed to be added to the
> memory block starting with the first section of the block.
>
> If you meant somewhere else let me know.
My point was just that if we change the "block_size_bytes" contents,
then we have to scale down the "memoryXXXX/phys_index" by that same
amount.
It *used* to be in numbers of SECTION_SIZE units, and I think it still
is:
- mem->start_phys_index = __section_nr(section);
+ mem->start_phys_index = base_memory_block_id(__section_nr(section));
+ mem->end_phys_index = mem->start_phys_index + sections_per_block - 1;
but now it needs to be changed to be in memory_block_size_bytes() units,
*NOT* SECTION_SIZE units.
Let's say we have a system with 4 16MB sections starting at 0x0.
Before, we would have:
block_size_bytes: 16777216
memory0/phys_index: 0
memory1/phys_index: 1
memory2/phys_index: 2
memory3/phys_index: 3
Now, we change memory_block_size_bytes() to be 32MB instead. We reduce
the number of sections in half, and I think the right thing to get is:
block_size_bytes: 33554432
memory0/phys_index: 0
memory1/phys_index: 1
I think, with your code (as it stands in these patches, no fixes) that
we'd instead get this:
block_size_bytes: 16777216
memory0/phys_index: 0
memory1/phys_index: 2
Without consulting "end_phys_index" (which isn't and can't be a part of
the existing ABI), we'd think that we have two 16MB banks instead of
four.
-- Dave
WARNING: multiple messages have this Message-ID (diff)
From: Dave Hansen <dave@linux.vnet.ibm.com>
To: Nathan Fontenot <nfont@austin.ibm.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
linuxppc-dev@ozlabs.org, Greg KH <greg@kroah.com>,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Subject: Re: [PATCH 0/8] De-couple sysfs memory directories from memory sections
Date: Wed, 22 Sep 2010 11:58:49 -0700 [thread overview]
Message-ID: <1285181929.3292.6287.camel@nimitz> (raw)
In-Reply-To: <4C9A4DBB.6080500@austin.ibm.com>
On Wed, 2010-09-22 at 13:40 -0500, Nathan Fontenot wrote:
> On 09/22/2010 10:20 AM, Dave Hansen wrote:
> > and phys_index's calculation needs to be:
> >
> > mem->start_phys_index * SECTION_SIZE / memory_block_size_bytes()
>
> I'm not sure if I follow where you suggest using this formula. Is this
> instead of what is used now, the base_memory_block_id() calculation?
>
> If so, then I'm not sure it would work. The formula used in base_memory_block_id()
> is done because the memory sections are not guaranteed to be added to the
> memory block starting with the first section of the block.
>
> If you meant somewhere else let me know.
My point was just that if we change the "block_size_bytes" contents,
then we have to scale down the "memoryXXXX/phys_index" by that same
amount.
It *used* to be in numbers of SECTION_SIZE units, and I think it still
is:
- mem->start_phys_index = __section_nr(section);
+ mem->start_phys_index = base_memory_block_id(__section_nr(section));
+ mem->end_phys_index = mem->start_phys_index + sections_per_block - 1;
but now it needs to be changed to be in memory_block_size_bytes() units,
*NOT* SECTION_SIZE units.
Let's say we have a system with 4 16MB sections starting at 0x0.
Before, we would have:
block_size_bytes: 16777216
memory0/phys_index: 0
memory1/phys_index: 1
memory2/phys_index: 2
memory3/phys_index: 3
Now, we change memory_block_size_bytes() to be 32MB instead. We reduce
the number of sections in half, and I think the right thing to get is:
block_size_bytes: 33554432
memory0/phys_index: 0
memory1/phys_index: 1
I think, with your code (as it stands in these patches, no fixes) that
we'd instead get this:
block_size_bytes: 16777216
memory0/phys_index: 0
memory1/phys_index: 2
Without consulting "end_phys_index" (which isn't and can't be a part of
the existing ABI), we'd think that we have two 16MB banks instead of
four.
-- Dave
WARNING: multiple messages have this Message-ID (diff)
From: Dave Hansen <dave@linux.vnet.ibm.com>
To: Nathan Fontenot <nfont@austin.ibm.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
linuxppc-dev@ozlabs.org, Greg KH <greg@kroah.com>,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Subject: Re: [PATCH 0/8] De-couple sysfs memory directories from memory sections
Date: Wed, 22 Sep 2010 11:58:49 -0700 [thread overview]
Message-ID: <1285181929.3292.6287.camel@nimitz> (raw)
In-Reply-To: <4C9A4DBB.6080500@austin.ibm.com>
On Wed, 2010-09-22 at 13:40 -0500, Nathan Fontenot wrote:
> On 09/22/2010 10:20 AM, Dave Hansen wrote:
> > and phys_index's calculation needs to be:
> >
> > mem->start_phys_index * SECTION_SIZE / memory_block_size_bytes()
>
> I'm not sure if I follow where you suggest using this formula. Is this
> instead of what is used now, the base_memory_block_id() calculation?
>
> If so, then I'm not sure it would work. The formula used in base_memory_block_id()
> is done because the memory sections are not guaranteed to be added to the
> memory block starting with the first section of the block.
>
> If you meant somewhere else let me know.
My point was just that if we change the "block_size_bytes" contents,
then we have to scale down the "memoryXXXX/phys_index" by that same
amount.
It *used* to be in numbers of SECTION_SIZE units, and I think it still
is:
- mem->start_phys_index = __section_nr(section);
+ mem->start_phys_index = base_memory_block_id(__section_nr(section));
+ mem->end_phys_index = mem->start_phys_index + sections_per_block - 1;
but now it needs to be changed to be in memory_block_size_bytes() units,
*NOT* SECTION_SIZE units.
Let's say we have a system with 4 16MB sections starting at 0x0.
Before, we would have:
block_size_bytes: 16777216
memory0/phys_index: 0
memory1/phys_index: 1
memory2/phys_index: 2
memory3/phys_index: 3
Now, we change memory_block_size_bytes() to be 32MB instead. We reduce
the number of sections in half, and I think the right thing to get is:
block_size_bytes: 33554432
memory0/phys_index: 0
memory1/phys_index: 1
I think, with your code (as it stands in these patches, no fixes) that
we'd instead get this:
block_size_bytes: 16777216
memory0/phys_index: 0
memory1/phys_index: 2
Without consulting "end_phys_index" (which isn't and can't be a part of
the existing ABI), we'd think that we have two 16MB banks instead of
four.
-- Dave
--
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>
next prev parent reply other threads:[~2010-09-22 18:58 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-22 14:15 [PATCH 0/8] De-couple sysfs memory directories from memory sections Nathan Fontenot
2010-09-22 14:15 ` Nathan Fontenot
2010-09-22 14:28 ` [PATCH 1/8] Move find_memory_block() routine Nathan Fontenot
2010-09-22 14:28 ` Nathan Fontenot
2010-09-22 14:29 ` [PATCH 2/8] Update memory block struct to have start and end phys index Nathan Fontenot
2010-09-22 14:29 ` Nathan Fontenot
2010-09-22 14:30 ` [PATCH 3/8] Add section count to memory_block struct Nathan Fontenot
2010-09-22 14:30 ` Nathan Fontenot
2010-09-22 14:32 ` [PATCH 4/8] Add mutex for adding/removing memory blocks Nathan Fontenot
2010-09-22 14:32 ` Nathan Fontenot
2010-09-22 14:33 ` [PATCH 5/8] Allow a memory block to span multiple memory sections Nathan Fontenot
2010-09-22 14:33 ` Nathan Fontenot
2010-09-22 14:34 ` [PATCH 6/8] Update node sysfs code Nathan Fontenot
2010-09-22 14:34 ` Nathan Fontenot
2010-09-22 14:35 ` [PATCH 7/8] Define memory_block_size_bytes() for powerpc/pseries Nathan Fontenot
2010-09-22 14:35 ` Nathan Fontenot
2010-09-22 14:36 ` [PATCH 8/8] Update memory hotplug documentation Nathan Fontenot
2010-09-22 14:36 ` Nathan Fontenot
2010-09-22 15:20 ` [PATCH 0/8] De-couple sysfs memory directories from memory sections Dave Hansen
2010-09-22 15:20 ` Dave Hansen
2010-09-22 15:20 ` Dave Hansen
2010-09-22 18:40 ` Nathan Fontenot
2010-09-22 18:40 ` Nathan Fontenot
2010-09-22 18:40 ` Nathan Fontenot
2010-09-22 18:58 ` Dave Hansen [this message]
2010-09-22 18:58 ` Dave Hansen
2010-09-22 18:58 ` Dave Hansen
2010-09-23 18:40 ` Balbir Singh
2010-09-23 18:40 ` Balbir Singh
2010-09-23 18:40 ` Balbir Singh
2010-09-24 14:35 ` Nathan Fontenot
2010-09-24 14:35 ` Nathan Fontenot
2010-09-24 14:35 ` Nathan Fontenot
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=1285181929.3292.6287.camel@nimitz \
--to=dave@linux.vnet.ibm.com \
--cc=greg@kroah.com \
--cc=kamezawa.hiroyu@jp.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linuxppc-dev@ozlabs.org \
--cc=nfont@austin.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.