| Message ID | 1356030217-5472-1-git-send-email-kgene.kim@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Thu, Dec 20, 2012 at 11:03 AM, Kukjin Kim <kgene.kim@samsung.com> wrote: > The size of memory bank should be under 256MB, because current > section size is 256MB on EXYNOS SoCs. This patch fixes it. This makes no sense. You don't have to split up memory ranges, the code should be made to handle it instead. What's the actual bug caused by this? The description is vague. -Olof
Hi Olof, On Thursday 20 of December 2012 11:56:59 Olof Johansson wrote: > Hi, > > On Thu, Dec 20, 2012 at 11:03 AM, Kukjin Kim <kgene.kim@samsung.com> wrote: > > The size of memory bank should be under 256MB, because current > > section size is 256MB on EXYNOS SoCs. This patch fixes it. > > This makes no sense. You don't have to split up memory ranges, the > code should be made to handle it instead. It's not Exynos code which causes the problem. Sparsemem initialization relies on the fact that initial amount of structures to described memory equals to maximum section size which is defined per arch (e.g. ARCH_EXYNOS). > What's the actual bug caused by this? The description is vague. The kernel panics early on NULL pointer dereference in memory initialization. Best regards, Tomasz Figa
I would like to ask a question here. Do we need to have sparse even if the physical memory is contiguous? All the recent exynos machines come with physical banks without any holes, and I am thinking why not drop it and use flat mem instead. With LPAE these sections sizes wont be useful, and I dont like to keep different section sizes for different configurations. Any suggestions/opinions are very much helpful to me. Regards, Subash On Thursday 20 December 2012 01:14 PM, Tomasz Figa wrote: > Hi Olof, > > On Thursday 20 of December 2012 11:56:59 Olof Johansson wrote: >> Hi, >> >> On Thu, Dec 20, 2012 at 11:03 AM, Kukjin Kim <kgene.kim@samsung.com> > wrote: >>> The size of memory bank should be under 256MB, because current >>> section size is 256MB on EXYNOS SoCs. This patch fixes it. >> >> This makes no sense. You don't have to split up memory ranges, the >> code should be made to handle it instead. > > It's not Exynos code which causes the problem. Sparsemem initialization > relies on the fact that initial amount of structures to described memory > equals to maximum section size which is defined per arch (e.g. > ARCH_EXYNOS). > >> What's the actual bug caused by this? The description is vague. > > The kernel panics early on NULL pointer dereference in memory > initialization. > > Best regards, > Tomasz Figa > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Hi Subash, On Thursday 20 of December 2012 14:19:48 Subash Patel wrote: > I would like to ask a question here. Do we need to have sparse even if > the physical memory is contiguous? All the recent exynos machines come > with physical banks without any holes, and I am thinking why not drop it > and use flat mem instead. With LPAE these sections sizes wont be useful, > and I dont like to keep different section sizes for different > configurations. Any suggestions/opinions are very much helpful to me. Since sparse memory is the only option available on Exynos currently in kernel configuration, I assume there was a reason for it. However I am not an expert in memory management, so please correct me if I am wrong. Best regards, Tomasz Figa
On Thu, Dec 20, 2012 at 10:14:26PM +0100, Tomasz Figa wrote: > Hi Olof, > > On Thursday 20 of December 2012 11:56:59 Olof Johansson wrote: > > Hi, > > > > On Thu, Dec 20, 2012 at 11:03 AM, Kukjin Kim <kgene.kim@samsung.com> > wrote: > > > The size of memory bank should be under 256MB, because current > > > section size is 256MB on EXYNOS SoCs. This patch fixes it. > > > > This makes no sense. You don't have to split up memory ranges, the > > code should be made to handle it instead. > > It's not Exynos code which causes the problem. Sparsemem initialization > relies on the fact that initial amount of structures to described memory > equals to maximum section size which is defined per arch (e.g. > ARCH_EXYNOS). > > > What's the actual bug caused by this? The description is vague. > > The kernel panics early on NULL pointer dereference in memory > initialization. And of course the debugging information is included in the commit log so that others can see what problem you're experiencing and make a decision whether the proposed solution is the right one...
Russell King - ARM Linux wrote: > [...] > > > > > What's the actual bug caused by this? The description is vague. > > > > The kernel panics early on NULL pointer dereference in memory > > initialization. > (Cc'ed KyongHo Cho) Yeah, correct. The size of memory bank should be under section size and current section size is 256MiB on EXYNOS. So if we don't change the section size, each memory bank should be under 256MiB. If not, as Tomasz said, kernel panic happens in mem_init(). > And of course the debugging information is included in the commit log so > that others can see what problem you're experiencing and make a decision > whether the proposed solution is the right one... Yeah, why not, I will add it in next version. Thanks. - Kukjin
On Thu, Dec 20, 2012 at 4:32 PM, Kukjin Kim <kgene.kim@samsung.com> wrote: > Russell King - ARM Linux wrote: >> > > [...] > >> > >> > > What's the actual bug caused by this? The description is vague. >> > >> > The kernel panics early on NULL pointer dereference in memory >> > initialization. >> > > (Cc'ed KyongHo Cho) > > Yeah, correct. The size of memory bank should be under section size and > current section size is 256MiB on EXYNOS. So if we don't change the section > size, each memory bank should be under 256MiB. If not, as Tomasz said, > kernel panic happens in mem_init(). That's a bug somewhere, the device tree should describe the hardware, and not necessarily the massaged format that the kernel wants it in to not trigger a bug. So I don't want to see this patch merged, I want to see a proper fix instead, please. There are also already other exynos platforms and device trees with more than 256MB in the memory node. Changing all of them this way seems unreasonable. -Olof
Tomasz Figa wrote: > > On Thursday 20 of December 2012 14:19:48 Subash Patel wrote: > > I would like to ask a question here. Do we need to have sparse even if > > the physical memory is contiguous? All the recent exynos machines come > > with physical banks without any holes, and I am thinking why not drop it > > and use flat mem instead. With LPAE these sections sizes wont be useful, > > and I dont like to keep different section sizes for different > > configurations. Any suggestions/opinions are very much helpful to me. > > Since sparse memory is the only option available on Exynos currently in > kernel configuration, I assume there was a reason for it. However I am not > an expert in memory management, so please correct me if I am wrong. > Hmm...yeah, as Subash said, it's no problem to disable sparsemem on EXYNOS for now but memory hole can be existing on upcoming EXYNOS such as S5PV210 so keeping sparsemem would be better I think. - Kukjin
> From: Olof Johansson [mailto:olof@lixom.net] > Sent: Friday, December 21, 2012 9:36 AM > > On Thu, Dec 20, 2012 at 4:32 PM, Kukjin Kim <kgene.kim@samsung.com> wrote: > > Russell King - ARM Linux wrote: > >> > > > > [...] > > > >> > > >> > > What's the actual bug caused by this? The description is vague. > >> > > >> > The kernel panics early on NULL pointer dereference in memory > >> > initialization. > >> > > > > (Cc'ed KyongHo Cho) > > > > Yeah, correct. The size of memory bank should be under section size and > > current section size is 256MiB on EXYNOS. So if we don't change the section > > size, each memory bank should be under 256MiB. If not, as Tomasz said, > > kernel panic happens in mem_init(). > > That's a bug somewhere, the device tree should describe the hardware, > and not necessarily the massaged format that the kernel wants it in to > not trigger a bug. > > So I don't want to see this patch merged, I want to see a proper fix > instead, please. > > There are also already other exynos platforms and device trees with > more than 256MB in the memory node. Changing all of them this way > seems unreasonable. > As Kukjin said earlier, the kernel panic may happen while gathering memory statistics in mem_init()(arch/arm/mm/init.c). Russell King does not agreed to modify mem_init() instead he told that a memory bank must not cross section boundaries.
diff --git a/arch/arm/boot/dts/exynos5440-ssdk5440.dts b/arch/arm/boot/dts/exynos5440-ssdk5440.dts index 44d4d24..79d02cc 100644 --- a/arch/arm/boot/dts/exynos5440-ssdk5440.dts +++ b/arch/arm/boot/dts/exynos5440-ssdk5440.dts @@ -17,7 +17,14 @@ compatible = "samsung,ssdk5440", "samsung,exynos5440"; memory { - reg = <0x80000000 0x80000000>; + reg = <0x80000000 0x10000000 + 0x90000000 0x10000000 + 0xA0000000 0x10000000 + 0xB0000000 0x10000000 + 0xC0000000 0x10000000 + 0xD0000000 0x10000000 + 0xE0000000 0x10000000 + 0xF0000000 0x10000000>; }; chosen {
The size of memory bank should be under 256MB, because current section size is 256MB on EXYNOS SoCs. This patch fixes it. Signed-off-by: Kukjin Kim <kgene.kim@samsung.com> --- arch/arm/boot/dts/exynos5440-ssdk5440.dts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)