RE: Re: [PATCH v4 6/6] RISC-V: Free-up initrd in free_initrd_mem()
Hi, > On Wed, Feb 13, 2019 at 09:54:15AM -0800, Christoph Hellwig wrote: > > On Wed, Feb 13, 2019 at 09:38:36AM +0200, Mike Rapoport wrote: > > > memblock_free() is has no real effect at this point, no idea why arm64 > > > calls it. > > > > Looks like the call was added fairly recently by: > > > > commit 05c58752f9dce11e396676eb731a620541590ed0 > > Author: CHANDAN VN > > Date: Mon Apr 30 09:50:18 2018 +0530 > > > > arm64: To remove initrd reserved area entry from memblock > > > > which claims it is to work around the initrd being displayed in > > /sys/kernel/debug/memblock/reserved. > > > > I really think we need to have common behavior there - either do this > > for all architectures or none. I've just sent a series that > > consolidates all but a handful of the free_initrd_mem, so implementing > > any common behavior on top of that would be good. > > I've just started to look into it today :) > I'll reply on that thread. INITRD reserved area entry is not removed from memblock even though initrd reserved area is freed. The same can be checked from /sys/kernel/debug/memblock/reserved. We did not face this issue on arm32 architecture. Though the changes which i had submitted does not fix any memory leak, it does make sure that the entries freed from memblock are actually removed from the sys entry as well. Also the implementation of arm64 is quite different from arm32. I feel a generic implementation can be taken up only if its a real necessity.
RE: Re: [PATCH] Smack: Fix memory leak in smack_inode_getsecctx
>On Mon, Jun 04, 2018 at 02:01:34PM -0700, Casey Schaufler wrote: >> On 6/1/2018 10:45 AM, Casey Schaufler wrote: >> > Fix memory leak in smack_inode_getsecctx >> > >> > The implementation of smack_inode_getsecctx() made >> > incorrect assumptions about how Smack presents a security >> > context. Smack does not need to allocate memory to support >> > security contexts, so "releasing" a Smack context is a no-op. >> > The code made an unnecessary copy and returned that as a >> > context, which was never freed. The revised implementation >> > returns the context correctly. >> > >> > Signed-off-by: Casey Schaufler >> >> Tejun, does this pass your tests? >Oh, I'm not the one who reported. Chandan? Looks good to me. Leak not found.
RE: Re: [PATCH] Smack: Fix memory leak in smack_inode_getsecctx
>On Mon, Jun 04, 2018 at 02:01:34PM -0700, Casey Schaufler wrote: >> On 6/1/2018 10:45 AM, Casey Schaufler wrote: >> > Fix memory leak in smack_inode_getsecctx >> > >> > The implementation of smack_inode_getsecctx() made >> > incorrect assumptions about how Smack presents a security >> > context. Smack does not need to allocate memory to support >> > security contexts, so "releasing" a Smack context is a no-op. >> > The code made an unnecessary copy and returned that as a >> > context, which was never freed. The revised implementation >> > returns the context correctly. >> > >> > Signed-off-by: Casey Schaufler >> >> Tejun, does this pass your tests? >Oh, I'm not the one who reported. Chandan? Looks good to me. Leak not found.
RE: Re: [PATCH 1/1] Fix memory leak in kernfs_security_xattr_set and kernfs_security_xattr_set
>> I agree that the fix can be done simply by using "false" for >> smack_inode_getsecurity(), but what happens with kernfs_node_setsecdata() >> and smack_inode_notifysecctx(). kernfs_node_setsecdata() is probably >>ignorable >> but smack_inode_notifysecctx() is sending the "ctx" to >>smack_inode_setsecurity() >> and since "ctx" would be NULL because we used "false", >>smack_inode_setsecurity() >> becomes dummy. >Thank you for pointing this out. You're right, there's more >at issue here than changing the alloc flag will fix. I think >that calling smack_inode_getsecurity() from smack_inode_getsecctx() >is making the code more complicated than it needs to be. I will >have a patch shortly. If you think the patch would take time or is complicated, I suggest that the kfree() fix should go to fix the leaks for now.
RE: Re: [PATCH 1/1] Fix memory leak in kernfs_security_xattr_set and kernfs_security_xattr_set
>> I agree that the fix can be done simply by using "false" for >> smack_inode_getsecurity(), but what happens with kernfs_node_setsecdata() >> and smack_inode_notifysecctx(). kernfs_node_setsecdata() is probably >>ignorable >> but smack_inode_notifysecctx() is sending the "ctx" to >>smack_inode_setsecurity() >> and since "ctx" would be NULL because we used "false", >>smack_inode_setsecurity() >> becomes dummy. >Thank you for pointing this out. You're right, there's more >at issue here than changing the alloc flag will fix. I think >that calling smack_inode_getsecurity() from smack_inode_getsecctx() >is making the code more complicated than it needs to be. I will >have a patch shortly. If you think the patch would take time or is complicated, I suggest that the kfree() fix should go to fix the leaks for now.
RE: Re: [PATCH 1/1] Fix memory leak in kernfs_security_xattr_set and kernfs_security_xattr_set
Hi >On 5/31/2018 9:11 AM, Tejun Heo wrote: > On Thu, May 31, 2018 at 09:04:25AM -0700, Casey Schaufler wrote: >>> On 5/31/2018 8:39 AM, Tejun Heo wrote: (cc'ing more security folks and copying whole body) So, I'm sure the patch fixes the memory leak but API wise it looks super confusing. Can security folks chime in here? Is this the right fix? security_inode_getsecctx() provides a security context. Technically, this is a data blob, although both provider provide a null terminated string. security_inode_getsecurity(), on the other hand, provides a string to match an attribute name. The former releases the security context with security_release_secctx(), where the later releases the string with kfree(). When the Smack hook smack_inode_getsecctx() was added in 2009 for use by labeled NFS the alloc value passed to >>> smack_inode_getsecurity() was set incorrectly. This wasn't a >>> major issue, since labeled NFS is a fringe case. When kernfs >>> started using the hook, it became the issue you discovered. >>> >>> The reason that we have all this confusion is that SELinux >>> generates security contexts as needed, while Smack keeps them >>> around all the time. Releasing an SELinux context frees memory, >>> while releasing a Smack context is a null operation. >> Any chance this detail can be hidden behind security api? This looks >> pretty error-prone, no? >>It *is* hidden behind the security API. The problem is strictly >>within the Smack code, where the implementer of smack_inode_getsecctx() >>made an error. I agree that the fix can be done simply by using "false" for smack_inode_getsecurity(), but what happens with kernfs_node_setsecdata() and smack_inode_notifysecctx(). kernfs_node_setsecdata() is probably ignorable but smack_inode_notifysecctx() is sending the "ctx" to smack_inode_setsecurity() and since "ctx" would be NULL because we used "false", smack_inode_setsecurity() becomes dummy.
RE: Re: [PATCH 1/1] Fix memory leak in kernfs_security_xattr_set and kernfs_security_xattr_set
Hi >On 5/31/2018 9:11 AM, Tejun Heo wrote: > On Thu, May 31, 2018 at 09:04:25AM -0700, Casey Schaufler wrote: >>> On 5/31/2018 8:39 AM, Tejun Heo wrote: (cc'ing more security folks and copying whole body) So, I'm sure the patch fixes the memory leak but API wise it looks super confusing. Can security folks chime in here? Is this the right fix? security_inode_getsecctx() provides a security context. Technically, this is a data blob, although both provider provide a null terminated string. security_inode_getsecurity(), on the other hand, provides a string to match an attribute name. The former releases the security context with security_release_secctx(), where the later releases the string with kfree(). When the Smack hook smack_inode_getsecctx() was added in 2009 for use by labeled NFS the alloc value passed to >>> smack_inode_getsecurity() was set incorrectly. This wasn't a >>> major issue, since labeled NFS is a fringe case. When kernfs >>> started using the hook, it became the issue you discovered. >>> >>> The reason that we have all this confusion is that SELinux >>> generates security contexts as needed, while Smack keeps them >>> around all the time. Releasing an SELinux context frees memory, >>> while releasing a Smack context is a null operation. >> Any chance this detail can be hidden behind security api? This looks >> pretty error-prone, no? >>It *is* hidden behind the security API. The problem is strictly >>within the Smack code, where the implementer of smack_inode_getsecctx() >>made an error. I agree that the fix can be done simply by using "false" for smack_inode_getsecurity(), but what happens with kernfs_node_setsecdata() and smack_inode_notifysecctx(). kernfs_node_setsecdata() is probably ignorable but smack_inode_notifysecctx() is sending the "ctx" to smack_inode_setsecurity() and since "ctx" would be NULL because we used "false", smack_inode_setsecurity() becomes dummy.
[PATCH 1/1] Fix memory leak in kernfs_security_xattr_set and kernfs_security_xattr_set
From: "sireesha.t"
Leak is caused because smack_inode_getsecurity() is allocating memory
using kstrdup(). Though the security_release_secctx() is called, it
would not free the allocated memory. Calling security_release_secctx is
not relevant for this scenario as inode_getsecurity() does not provide a
"secctx".
Similar fix has been mainlined:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?id=57e7ba04d422c3d41c8426380303ec9b7533ded9
The fix is to replace the security_release_secctx() with a kfree()
Below is the KMEMLEAK dump:
unreferenced object 0xffc025e11c80 (size 64):
comm "systemd-tmpfile", pid 2452, jiffies 4294894464 (age 235587.492s)
hex dump (first 32 bytes):
53 79 73 74 65 6d 3a 3a 53 68 61 72 65 64 00 00 System::Shared..
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
backtrace:
[] __save_stack_trace+0x28/0x34
[] create_object+0x130/0x25c
[] kmemleak_alloc+0x30/0x5c
[] __kmalloc_track_caller+0x1cc/0x2a8
[] kstrdup+0x3c/0x6c
[] smack_inode_getsecurity+0xcc/0xec
[] smack_inode_getsecctx+0x24/0x44
[] security_inode_getsecctx+0x50/0x70
[] kernfs_security_xattr_set+0x74/0xe0
[] __vfs_setxattr+0x74/0x90
[] __vfs_setxattr_noperm+0x80/0x1ac
[] vfs_setxattr+0x84/0xac
[] setxattr+0x114/0x178
[] path_setxattr+0x74/0xb8
[] SyS_lsetxattr+0x10/0x1c
[] __sys_trace_return+0x0/0x4
Signed-off-by: sireesha.t
Signed-off-by: CHANDAN VN
---
fs/kernfs/inode.c | 3 ++-
fs/nfsd/nfs4xdr.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index a343039..53befb8 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -369,7 +369,8 @@ static int kernfs_security_xattr_set(const struct
xattr_handler *handler,
mutex_unlock(_mutex);
if (secdata)
- security_release_secctx(secdata, secdata_len);
+ kfree(secdata);
+
return error;
}
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index aaa88c1..1e0dbe9 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2911,7 +2911,7 @@ static int get_parent_attributes(struct svc_export *exp,
struct kstat *stat)
out:
#ifdef CONFIG_NFSD_V4_SECURITY_LABEL
if (context)
- security_release_secctx(context, contextlen);
+ kfree(context);
#endif /* CONFIG_NFSD_V4_SECURITY_LABEL */
kfree(acl);
if (tempfh) {
--
1.9.1
[PATCH 1/1] Fix memory leak in kernfs_security_xattr_set and kernfs_security_xattr_set
From: "sireesha.t"
Leak is caused because smack_inode_getsecurity() is allocating memory
using kstrdup(). Though the security_release_secctx() is called, it
would not free the allocated memory. Calling security_release_secctx is
not relevant for this scenario as inode_getsecurity() does not provide a
"secctx".
Similar fix has been mainlined:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?id=57e7ba04d422c3d41c8426380303ec9b7533ded9
The fix is to replace the security_release_secctx() with a kfree()
Below is the KMEMLEAK dump:
unreferenced object 0xffc025e11c80 (size 64):
comm "systemd-tmpfile", pid 2452, jiffies 4294894464 (age 235587.492s)
hex dump (first 32 bytes):
53 79 73 74 65 6d 3a 3a 53 68 61 72 65 64 00 00 System::Shared..
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
backtrace:
[] __save_stack_trace+0x28/0x34
[] create_object+0x130/0x25c
[] kmemleak_alloc+0x30/0x5c
[] __kmalloc_track_caller+0x1cc/0x2a8
[] kstrdup+0x3c/0x6c
[] smack_inode_getsecurity+0xcc/0xec
[] smack_inode_getsecctx+0x24/0x44
[] security_inode_getsecctx+0x50/0x70
[] kernfs_security_xattr_set+0x74/0xe0
[] __vfs_setxattr+0x74/0x90
[] __vfs_setxattr_noperm+0x80/0x1ac
[] vfs_setxattr+0x84/0xac
[] setxattr+0x114/0x178
[] path_setxattr+0x74/0xb8
[] SyS_lsetxattr+0x10/0x1c
[] __sys_trace_return+0x0/0x4
Signed-off-by: sireesha.t
Signed-off-by: CHANDAN VN
---
fs/kernfs/inode.c | 3 ++-
fs/nfsd/nfs4xdr.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index a343039..53befb8 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -369,7 +369,8 @@ static int kernfs_security_xattr_set(const struct
xattr_handler *handler,
mutex_unlock(_mutex);
if (secdata)
- security_release_secctx(secdata, secdata_len);
+ kfree(secdata);
+
return error;
}
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index aaa88c1..1e0dbe9 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2911,7 +2911,7 @@ static int get_parent_attributes(struct svc_export *exp,
struct kstat *stat)
out:
#ifdef CONFIG_NFSD_V4_SECURITY_LABEL
if (context)
- security_release_secctx(context, contextlen);
+ kfree(context);
#endif /* CONFIG_NFSD_V4_SECURITY_LABEL */
kfree(acl);
if (tempfh) {
--
1.9.1
Re: [PATCHv2 1/1] arm64: To remove initrd reserved area entry from memblock
Please ignore this mail. I missed replying to the thread.
I have resubmitted over the proper thread.
On Mon, 30 Apr 2018, 09:44 CHANDAN VN, <chandan...@samsung.com> wrote:
>
> INITRD reserved area entry is not removed from memblock
> even though initrd reserved area is freed. After freeing
> the memory it is released from memblock. The same can be
> checked from /sys/kernel/debug/memblock/reserved.
>
> The patch makes sure that the initrd entry is removed from
> memblock when keepinitrd is not enabled.
>
> The patch only affects accounting and debugging. This does not
> fix any memory leak.
>
> Signed-off-by: CHANDAN VN <chandan...@samsung.com>
> ---
> arch/arm64/mm/init.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 9f3c47a..1b18b47 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -646,8 +646,10 @@ void free_initmem(void)
>
> void __init free_initrd_mem(unsigned long start, unsigned long end)
> {
> - if (!keep_initrd)
> + if (!keep_initrd) {
> free_reserved_area((void *)start, (void *)end, 0, "initrd");
> + memblock_free(__virt_to_phys(start), end - start);
> + }
> }
>
> static int __init keepinitrd_setup(char *__unused)
> --
> 1.9.1
>
Re: [PATCHv2 1/1] arm64: To remove initrd reserved area entry from memblock
Please ignore this mail. I missed replying to the thread.
I have resubmitted over the proper thread.
On Mon, 30 Apr 2018, 09:44 CHANDAN VN, wrote:
>
> INITRD reserved area entry is not removed from memblock
> even though initrd reserved area is freed. After freeing
> the memory it is released from memblock. The same can be
> checked from /sys/kernel/debug/memblock/reserved.
>
> The patch makes sure that the initrd entry is removed from
> memblock when keepinitrd is not enabled.
>
> The patch only affects accounting and debugging. This does not
> fix any memory leak.
>
> Signed-off-by: CHANDAN VN
> ---
> arch/arm64/mm/init.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 9f3c47a..1b18b47 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -646,8 +646,10 @@ void free_initmem(void)
>
> void __init free_initrd_mem(unsigned long start, unsigned long end)
> {
> - if (!keep_initrd)
> + if (!keep_initrd) {
> free_reserved_area((void *)start, (void *)end, 0, "initrd");
> + memblock_free(__virt_to_phys(start), end - start);
> + }
> }
>
> static int __init keepinitrd_setup(char *__unused)
> --
> 1.9.1
>
[PATCHv2 1/1] arm64: To remove initrd reserved area entry from memblock
INITRD reserved area entry is not removed from memblock
even though initrd reserved area is freed. After freeing
the memory it is released from memblock. The same can be
checked from /sys/kernel/debug/memblock/reserved.
The patch makes sure that the initrd entry is removed from
memblock when keepinitrd is not enabled.
The patch only affects accounting and debugging. This does not
fix any memory leak.
Signed-off-by: CHANDAN VN <chandan...@samsung.com>
---
arch/arm64/mm/init.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9f3c47a..1b18b47 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -646,8 +646,10 @@ void free_initmem(void)
void __init free_initrd_mem(unsigned long start, unsigned long end)
{
- if (!keep_initrd)
+ if (!keep_initrd) {
free_reserved_area((void *)start, (void *)end, 0, "initrd");
+ memblock_free(__virt_to_phys(start), end - start);
+ }
}
static int __init keepinitrd_setup(char *__unused)
--
1.9.1
[PATCHv2 1/1] arm64: To remove initrd reserved area entry from memblock
INITRD reserved area entry is not removed from memblock
even though initrd reserved area is freed. After freeing
the memory it is released from memblock. The same can be
checked from /sys/kernel/debug/memblock/reserved.
The patch makes sure that the initrd entry is removed from
memblock when keepinitrd is not enabled.
The patch only affects accounting and debugging. This does not
fix any memory leak.
Signed-off-by: CHANDAN VN
---
arch/arm64/mm/init.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9f3c47a..1b18b47 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -646,8 +646,10 @@ void free_initmem(void)
void __init free_initrd_mem(unsigned long start, unsigned long end)
{
- if (!keep_initrd)
+ if (!keep_initrd) {
free_reserved_area((void *)start, (void *)end, 0, "initrd");
+ memblock_free(__virt_to_phys(start), end - start);
+ }
}
static int __init keepinitrd_setup(char *__unused)
--
1.9.1
[PATCHv2 1/1] arm64: To remove initrd reserved area entry from memblock
INITRD reserved area entry is not removed from memblock
even though initrd reserved area is freed. After freeing
the memory it is released from memblock. The same can be
checked from /sys/kernel/debug/memblock/reserved.
The patch makes sure that the initrd entry is removed from
memblock when keepinitrd is not enabled.
The patch only affects accounting and debugging. This does not
fix any memory leak.
Signed-off-by: CHANDAN VN <chandan...@samsung.com>
---
arch/arm64/mm/init.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9f3c47a..1b18b47 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -646,8 +646,10 @@ void free_initmem(void)
void __init free_initrd_mem(unsigned long start, unsigned long end)
{
- if (!keep_initrd)
+ if (!keep_initrd) {
free_reserved_area((void *)start, (void *)end, 0, "initrd");
+ memblock_free(__virt_to_phys(start), end - start);
+ }
}
static int __init keepinitrd_setup(char *__unused)
--
1.9.1
[PATCHv2 1/1] arm64: To remove initrd reserved area entry from memblock
INITRD reserved area entry is not removed from memblock
even though initrd reserved area is freed. After freeing
the memory it is released from memblock. The same can be
checked from /sys/kernel/debug/memblock/reserved.
The patch makes sure that the initrd entry is removed from
memblock when keepinitrd is not enabled.
The patch only affects accounting and debugging. This does not
fix any memory leak.
Signed-off-by: CHANDAN VN
---
arch/arm64/mm/init.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9f3c47a..1b18b47 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -646,8 +646,10 @@ void free_initmem(void)
void __init free_initrd_mem(unsigned long start, unsigned long end)
{
- if (!keep_initrd)
+ if (!keep_initrd) {
free_reserved_area((void *)start, (void *)end, 0, "initrd");
+ memblock_free(__virt_to_phys(start), end - start);
+ }
}
static int __init keepinitrd_setup(char *__unused)
--
1.9.1
Re: [PATCH 1/1] arm64: To remove initrd reserved area entry from memblock
Hi,
May I know when this patch would be taken for merging?
On Sat, Apr 7, 2018 at 9:58 AM, Chandan Vn <vn.chan...@gmail.com> wrote:
> On Fri, Apr 6, 2018 at 9:47 PM, Laura Abbott <labb...@redhat.com> wrote:
>> Does this have an impact on anything besides accounting
>> in memblock?
>
> Yes, the impact is only on accounting or debugging.
>
> We were trying to reduce the reserved memory by removing initrd reserved area.
> After disabling "keepinitrd", only way to check if it was removed or
> not was to check
> /sys/kernel/debug/memblock/reserved. We found the entry to be present
> irrespective of
> "keepinitrd" being enabled/disabled.
> I hope that with the fix others wont face similar issue. Also we did
> not find any such problem
> with ARM32 ARCHITECTURE.
>
> On Fri, Apr 6, 2018 at 9:47 PM, Laura Abbott <labb...@redhat.com> wrote:
>> On 04/05/2018 09:53 PM, CHANDAN VN wrote:
>>>
>>> INITRD reserved area entry is not removed from memblock
>>> even though initrd reserved area is freed. After freeing
>>> the memory it is released from memblock. The same can be
>>> checked from /sys/kernel/debug/memblock/reserved.
>>>
>>> The patch makes sure that the initrd entry is removed from
>>> memblock when keepinitrd is not enabled.
>>>
>>
>> Does this have an impact on anything besides accounting
>> in memblock?
>>
>>
>>> Signed-off-by: CHANDAN VN <chandan...@samsung.com>
>>> ---
>>> arch/arm64/mm/init.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>>> index 9f3c47a..1b18b47 100644
>>> --- a/arch/arm64/mm/init.c
>>> +++ b/arch/arm64/mm/init.c
>>> @@ -646,8 +646,10 @@ void free_initmem(void)
>>> void __init free_initrd_mem(unsigned long start, unsigned long end)
>>> {
>>> - if (!keep_initrd)
>>> + if (!keep_initrd) {
>>> free_reserved_area((void *)start, (void *)end, 0,
>>> "initrd");
>>> + memblock_free(__virt_to_phys(start), end - start);
>>> + }
>>> }
>>> static int __init keepinitrd_setup(char *__unused)
>>>
>>
Re: [PATCH 1/1] arm64: To remove initrd reserved area entry from memblock
Hi,
May I know when this patch would be taken for merging?
On Sat, Apr 7, 2018 at 9:58 AM, Chandan Vn wrote:
> On Fri, Apr 6, 2018 at 9:47 PM, Laura Abbott wrote:
>> Does this have an impact on anything besides accounting
>> in memblock?
>
> Yes, the impact is only on accounting or debugging.
>
> We were trying to reduce the reserved memory by removing initrd reserved area.
> After disabling "keepinitrd", only way to check if it was removed or
> not was to check
> /sys/kernel/debug/memblock/reserved. We found the entry to be present
> irrespective of
> "keepinitrd" being enabled/disabled.
> I hope that with the fix others wont face similar issue. Also we did
> not find any such problem
> with ARM32 ARCHITECTURE.
>
> On Fri, Apr 6, 2018 at 9:47 PM, Laura Abbott wrote:
>> On 04/05/2018 09:53 PM, CHANDAN VN wrote:
>>>
>>> INITRD reserved area entry is not removed from memblock
>>> even though initrd reserved area is freed. After freeing
>>> the memory it is released from memblock. The same can be
>>> checked from /sys/kernel/debug/memblock/reserved.
>>>
>>> The patch makes sure that the initrd entry is removed from
>>> memblock when keepinitrd is not enabled.
>>>
>>
>> Does this have an impact on anything besides accounting
>> in memblock?
>>
>>
>>> Signed-off-by: CHANDAN VN
>>> ---
>>> arch/arm64/mm/init.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>>> index 9f3c47a..1b18b47 100644
>>> --- a/arch/arm64/mm/init.c
>>> +++ b/arch/arm64/mm/init.c
>>> @@ -646,8 +646,10 @@ void free_initmem(void)
>>> void __init free_initrd_mem(unsigned long start, unsigned long end)
>>> {
>>> - if (!keep_initrd)
>>> + if (!keep_initrd) {
>>> free_reserved_area((void *)start, (void *)end, 0,
>>> "initrd");
>>> + memblock_free(__virt_to_phys(start), end - start);
>>> + }
>>> }
>>> static int __init keepinitrd_setup(char *__unused)
>>>
>>
Re: [PATCH 1/1] arm64: To remove initrd reserved area entry from memblock
On Fri, Apr 6, 2018 at 9:47 PM, Laura Abbott <labb...@redhat.com> wrote:
> Does this have an impact on anything besides accounting
> in memblock?
Yes, the impact is only on accounting or debugging.
We were trying to reduce the reserved memory by removing initrd reserved area.
After disabling "keepinitrd", only way to check if it was removed or
not was to check
/sys/kernel/debug/memblock/reserved. We found the entry to be present
irrespective of
"keepinitrd" being enabled/disabled.
I hope that with the fix others wont face similar issue. Also we did
not find any such problem
with ARM32 ARCHITECTURE.
On Fri, Apr 6, 2018 at 9:47 PM, Laura Abbott <labb...@redhat.com> wrote:
> On 04/05/2018 09:53 PM, CHANDAN VN wrote:
>>
>> INITRD reserved area entry is not removed from memblock
>> even though initrd reserved area is freed. After freeing
>> the memory it is released from memblock. The same can be
>> checked from /sys/kernel/debug/memblock/reserved.
>>
>> The patch makes sure that the initrd entry is removed from
>> memblock when keepinitrd is not enabled.
>>
>
> Does this have an impact on anything besides accounting
> in memblock?
>
>
>> Signed-off-by: CHANDAN VN <chandan...@samsung.com>
>> ---
>> arch/arm64/mm/init.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 9f3c47a..1b18b47 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -646,8 +646,10 @@ void free_initmem(void)
>> void __init free_initrd_mem(unsigned long start, unsigned long end)
>> {
>> - if (!keep_initrd)
>> + if (!keep_initrd) {
>> free_reserved_area((void *)start, (void *)end, 0,
>> "initrd");
>> + memblock_free(__virt_to_phys(start), end - start);
>> + }
>> }
>> static int __init keepinitrd_setup(char *__unused)
>>
>
Re: [PATCH 1/1] arm64: To remove initrd reserved area entry from memblock
On Fri, Apr 6, 2018 at 9:47 PM, Laura Abbott wrote:
> Does this have an impact on anything besides accounting
> in memblock?
Yes, the impact is only on accounting or debugging.
We were trying to reduce the reserved memory by removing initrd reserved area.
After disabling "keepinitrd", only way to check if it was removed or
not was to check
/sys/kernel/debug/memblock/reserved. We found the entry to be present
irrespective of
"keepinitrd" being enabled/disabled.
I hope that with the fix others wont face similar issue. Also we did
not find any such problem
with ARM32 ARCHITECTURE.
On Fri, Apr 6, 2018 at 9:47 PM, Laura Abbott wrote:
> On 04/05/2018 09:53 PM, CHANDAN VN wrote:
>>
>> INITRD reserved area entry is not removed from memblock
>> even though initrd reserved area is freed. After freeing
>> the memory it is released from memblock. The same can be
>> checked from /sys/kernel/debug/memblock/reserved.
>>
>> The patch makes sure that the initrd entry is removed from
>> memblock when keepinitrd is not enabled.
>>
>
> Does this have an impact on anything besides accounting
> in memblock?
>
>
>> Signed-off-by: CHANDAN VN
>> ---
>> arch/arm64/mm/init.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 9f3c47a..1b18b47 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -646,8 +646,10 @@ void free_initmem(void)
>> void __init free_initrd_mem(unsigned long start, unsigned long end)
>> {
>> - if (!keep_initrd)
>> + if (!keep_initrd) {
>> free_reserved_area((void *)start, (void *)end, 0,
>> "initrd");
>> + memblock_free(__virt_to_phys(start), end - start);
>> + }
>> }
>> static int __init keepinitrd_setup(char *__unused)
>>
>
[PATCH 1/1] arm64: To remove initrd reserved area entry from memblock
INITRD reserved area entry is not removed from memblock
even though initrd reserved area is freed. After freeing
the memory it is released from memblock. The same can be
checked from /sys/kernel/debug/memblock/reserved.
The patch makes sure that the initrd entry is removed from
memblock when keepinitrd is not enabled.
Signed-off-by: CHANDAN VN <chandan...@samsung.com>
---
arch/arm64/mm/init.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9f3c47a..1b18b47 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -646,8 +646,10 @@ void free_initmem(void)
void __init free_initrd_mem(unsigned long start, unsigned long end)
{
- if (!keep_initrd)
+ if (!keep_initrd) {
free_reserved_area((void *)start, (void *)end, 0, "initrd");
+ memblock_free(__virt_to_phys(start), end - start);
+ }
}
static int __init keepinitrd_setup(char *__unused)
--
1.9.1
[PATCH 1/1] arm64: To remove initrd reserved area entry from memblock
INITRD reserved area entry is not removed from memblock
even though initrd reserved area is freed. After freeing
the memory it is released from memblock. The same can be
checked from /sys/kernel/debug/memblock/reserved.
The patch makes sure that the initrd entry is removed from
memblock when keepinitrd is not enabled.
Signed-off-by: CHANDAN VN
---
arch/arm64/mm/init.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9f3c47a..1b18b47 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -646,8 +646,10 @@ void free_initmem(void)
void __init free_initrd_mem(unsigned long start, unsigned long end)
{
- if (!keep_initrd)
+ if (!keep_initrd) {
free_reserved_area((void *)start, (void *)end, 0, "initrd");
+ memblock_free(__virt_to_phys(start), end - start);
+ }
}
static int __init keepinitrd_setup(char *__unused)
--
1.9.1
