diff mbox

[resend] arm:extend the reserved memory for initrd to be page aligned

Message ID 35FD53F367049845BC99AC72306C23D103D6DB49161F@CNBJMBX05.corpusers.net (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Wang, Yalin Sept. 19, 2014, 7:09 a.m. UTC
this patch extend the start and end address of initrd to be page aligned,
so that we can free all memory including the un-page aligned head or tail
page of initrd, if the start or end address of initrd are not page
aligned, the page can't be freed by free_initrd_mem() function.

Signed-off-by: Yalin Wang <yalin.wang@sonymobile.com>
---
 arch/arm/mm/init.c   | 5 +++++
 arch/arm64/mm/init.c | 8 +++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Catalin Marinas Sept. 19, 2014, 10 a.m. UTC | #1
On Fri, Sep 19, 2014 at 08:09:47AM +0100, Wang, Yalin wrote:
> this patch extend the start and end address of initrd to be page aligned,
> so that we can free all memory including the un-page aligned head or tail
> page of initrd, if the start or end address of initrd are not page
> aligned, the page can't be freed by free_initrd_mem() function.
> 
> Signed-off-by: Yalin Wang <yalin.wang@sonymobile.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

(as I said, if Russell doesn't have any objections please send the patch
to his patch system)
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux Sept. 25, 2014, 2:31 p.m. UTC | #2
On Fri, Sep 19, 2014 at 11:00:02AM +0100, Catalin Marinas wrote:
> On Fri, Sep 19, 2014 at 08:09:47AM +0100, Wang, Yalin wrote:
> > this patch extend the start and end address of initrd to be page aligned,
> > so that we can free all memory including the un-page aligned head or tail
> > page of initrd, if the start or end address of initrd are not page
> > aligned, the page can't be freed by free_initrd_mem() function.
> > 
> > Signed-off-by: Yalin Wang <yalin.wang@sonymobile.com>
> 
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> 
> (as I said, if Russell doesn't have any objections please send the patch
> to his patch system)

I now have an objection.  The patches in the emails were properly formatted.
The patches which were submitted to the patch system (there's two of them
doing the same thing...) are not:

--- ../kernel.torvalds.git.origin/arch/arm/mm/init.c    2014-09-24 16:24:06.863759000 +0800
+++ arch/arm/mm/init.c  2014-09-24 16:27:11.455456000 +0800

This is totally broken.  Let's read the patch(1) man page:

       First, patch takes an ordered list of candidate file names as follows:

        · If the header is that of a context diff, patch takes the old and new
          file  names  in  the  header.  A name is ignored if it does not have
          enough slashes to satisfy the -pnum or --strip=num option.  The name
          /dev/null is also ignored.

        · If  there is an Index: line in the leading garbage and if either the
          old and new names are both absent  or  if  patch  is  conforming  to
          POSIX, patch takes the name in the Index: line.

        · For the purpose of the following rules, the candidate file names are
          considered to be in the order (old, new, index), regardless  of  the
          order that they appear in the header.

       Then patch selects a file name from the candidate list as follows:

        · If  some  of  the named files exist, patch selects the first name if
          conforming to POSIX, and the best name otherwise.
...
        · If no named files exist, no RCS, ClearCase, Perforce, or SCCS master
          was found, some names are given, patch is not conforming  to  POSIX,
          and  the patch appears to create a file, patch selects the best name
          requiring the creation of the fewest directories.

        · If no file name results from the above heuristics, you are asked for
          the name of the file to patch, and patch selects that name.

...

NOTES FOR PATCH SENDERS
       There are several things you should bear in mind if you are going to be
       sending out patches.
...
       If the recipient is supposed to use the -pN option, do not send  output
       that looks like this:

          diff -Naur v2.0.29/prog/README prog/README
          --- v2.0.29/prog/README   Mon Mar 10 15:13:12 1997
          +++ prog/README   Mon Mar 17 14:58:22 1997

       because  the two file names have different numbers of slashes, and dif-
       ferent versions of patch interpret  the  file  names  differently.   To
       avoid confusion, send output that looks like this instead:

          diff -Naur v2.0.29/prog/README v2.0.30/prog/README
          --- v2.0.29/prog/README   Mon Mar 10 15:13:12 1997
          +++ v2.0.30/prog/README   Mon Mar 17 14:58:22 1997

So I'm going to reject this crap.
Catalin Marinas Sept. 25, 2014, 3:44 p.m. UTC | #3
On Thu, Sep 25, 2014 at 03:31:42PM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 19, 2014 at 11:00:02AM +0100, Catalin Marinas wrote:
> > On Fri, Sep 19, 2014 at 08:09:47AM +0100, Wang, Yalin wrote:
> > > this patch extend the start and end address of initrd to be page aligned,
> > > so that we can free all memory including the un-page aligned head or tail
> > > page of initrd, if the start or end address of initrd are not page
> > > aligned, the page can't be freed by free_initrd_mem() function.
> > > 
> > > Signed-off-by: Yalin Wang <yalin.wang@sonymobile.com>
> > 
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > 
> > (as I said, if Russell doesn't have any objections please send the patch
> > to his patch system)
> 
> I now have an objection.  The patches in the emails were properly formatted.

They were so close ;)

I can see three patches but none of them exactly right:

8157/1 - wrong diff format
8159/1 - correct format, does not have my ack (you can take this one if
	 you want)
8162/1 - got my ack this time but with the wrong diff format again

Maybe a pull request is a better idea.
Wang, Yalin Sept. 26, 2014, 1:56 a.m. UTC | #4
> On Fri, Sep 19, 2014 at 11:00:02AM +0100, Catalin Marinas wrote:
> > On Fri, Sep 19, 2014 at 08:09:47AM +0100, Wang, Yalin wrote:
> > > this patch extend the start and end address of initrd to be page
> > > aligned, so that we can free all memory including the un-page
> > > aligned head or tail page of initrd, if the start or end address of
> > > initrd are not page aligned, the page can't be freed by
> free_initrd_mem() function.
> > >
> > > Signed-off-by: Yalin Wang <yalin.wang@sonymobile.com>
> >
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> >
> > (as I said, if Russell doesn't have any objections please send the
> > patch to his patch system)
> 
> I now have an objection.  The patches in the emails were properly formatted.
> The patches which were submitted to the patch system (there's two of them
> doing the same thing...) are not:
> 
> --- ../kernel.torvalds.git.origin/arch/arm/mm/init.c    2014-09-24
> 16:24:06.863759000 +0800
> +++ arch/arm/mm/init.c  2014-09-24 16:27:11.455456000 +0800
> 
> This is totally broken.  Let's read the patch(1) man page:
> 
>        First, patch takes an ordered list of candidate file names as
> follows:
> 
>         · If the header is that of a context diff, patch takes the old and
> new
>           file  names  in  the  header.  A name is ignored if it does not
> have
>           enough slashes to satisfy the -pnum or --strip=num option.  The
> name
>           /dev/null is also ignored.
> 
>         · If  there is an Index: line in the leading garbage and if either
> the
>           old and new names are both absent  or  if  patch  is  conforming
> to
>           POSIX, patch takes the name in the Index: line.
> 
>         · For the purpose of the following rules, the candidate file names
> are
>           considered to be in the order (old, new, index), regardless  of
> the
>           order that they appear in the header.
> 
>        Then patch selects a file name from the candidate list as follows:
> 
>         · If  some  of  the named files exist, patch selects the first name
> if
>           conforming to POSIX, and the best name otherwise.
> ...
>         · If no named files exist, no RCS, ClearCase, Perforce, or SCCS
> master
>           was found, some names are given, patch is not conforming  to
> POSIX,
>           and  the patch appears to create a file, patch selects the best
> name
>           requiring the creation of the fewest directories.
> 
>         · If no file name results from the above heuristics, you are asked
> for
>           the name of the file to patch, and patch selects that name.
> 
> ...
> 
> NOTES FOR PATCH SENDERS
>        There are several things you should bear in mind if you are going to
> be
>        sending out patches.
> ...
>        If the recipient is supposed to use the -pN option, do not send
> output
>        that looks like this:
> 
>           diff -Naur v2.0.29/prog/README prog/README
>           --- v2.0.29/prog/README   Mon Mar 10 15:13:12 1997
>           +++ prog/README   Mon Mar 17 14:58:22 1997
> 
>        because  the two file names have different numbers of slashes, and
> dif-
>        ferent versions of patch interpret  the  file  names  differently.
> To
>        avoid confusion, send output that looks like this instead:
> 
>           diff -Naur v2.0.29/prog/README v2.0.30/prog/README
>           --- v2.0.29/prog/README   Mon Mar 10 15:13:12 1997
>           +++ v2.0.30/prog/README   Mon Mar 17 14:58:22 1997
> 
Got it ,
I will resend the patch,
By the way, how to remove my wrong patch in the patch system ?

Thanks



--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wang, Yalin Sept. 26, 2014, 2:40 a.m. UTC | #5
> They were so close ;)
> 
> I can see three patches but none of them exactly right:
> 
> 8157/1 - wrong diff format
> 8159/1 - correct format, does not have my ack (you can take this one if
> 	 you want)
> 8162/1 - got my ack this time but with the wrong diff format again
> 
> Maybe a pull request is a better idea.
> 
I am really confused, 
I read this web:
http://www.arm.linux.org.uk/developer/patches/info.php
it said use diff -urN to generate patch like this:

diff -Nru linux.orig/lib/string.c linux/lib/string.c

but I see other developers use git format-patch to generate patch and
submit to the patch system.
Git format-patch format can also be accepted by the patch system correctly ?
If yes, I think this web should update,
Use git format-patch to generate patch is more convenient than use diff -urN 

Thanks



--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wang, Yalin Sept. 28, 2014, 1:50 a.m. UTC | #6
> -----Original Message-----
> On Thu, Sep 25, 2014 at 03:31:42PM +0100, Russell King - ARM Linux wrote:
> > On Fri, Sep 19, 2014 at 11:00:02AM +0100, Catalin Marinas wrote:
> > > On Fri, Sep 19, 2014 at 08:09:47AM +0100, Wang, Yalin wrote:
> > > > this patch extend the start and end address of initrd to be page
> > > > aligned, so that we can free all memory including the un-page
> > > > aligned head or tail page of initrd, if the start or end address
> > > > of initrd are not page aligned, the page can't be freed by
> free_initrd_mem() function.
> > > >
> > > > Signed-off-by: Yalin Wang <yalin.wang@sonymobile.com>
> > >
> > > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > >
> > > (as I said, if Russell doesn't have any objections please send the
> > > patch to his patch system)
> >
> > I now have an objection.  The patches in the emails were properly
> formatted.
> 
> They were so close ;)
> 
> I can see three patches but none of them exactly right:
> 
> 8157/1 - wrong diff format
> 8159/1 - correct format, does not have my ack (you can take this one if
> 	 you want)
> 8162/1 - got my ack this time but with the wrong diff format again
> 
> Maybe a pull request is a better idea.
> 
I have resend the 2 patches:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8167/1
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8168/1
 
please have a look.

Thanks



--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Valdis Klētnieks Sept. 29, 2014, 7:21 p.m. UTC | #7
On Fri, 26 Sep 2014 10:40:54 +0800, "Wang, Yalin" said:

> I am really confused,
> I read this web:
> http://www.arm.linux.org.uk/developer/patches/info.php
> it said use diff -urN to generate patch like this:
>
> diff -Nru linux.orig/lib/string.c linux/lib/string.c
>
> but I see other developers use git format-patch to generate patch and
> submit to the patch system.
> Git format-patch format can also be accepted by the patch system correctly ?
> If yes, I think this web should update,
> Use git format-patch to generate patch is more convenient than use diff -urN

'diff -urN' has the advantage that it will work against a tree extracted
from a release tarball, and doesn't have a requirement that you have git
installed.  Having said that, somebody who has access to the website probably
should update it to mention that both methods are acceptable....
Uwe Kleine-König Sept. 30, 2014, 7:02 a.m. UTC | #8
Hello,

On Mon, Sep 29, 2014 at 03:21:58PM -0400, Valdis.Kletnieks@vt.edu wrote:
> On Fri, 26 Sep 2014 10:40:54 +0800, "Wang, Yalin" said:
> 
> > I am really confused,
> > I read this web:
> > http://www.arm.linux.org.uk/developer/patches/info.php
> > it said use diff -urN to generate patch like this:
> >
> > diff -Nru linux.orig/lib/string.c linux/lib/string.c
> >
> > but I see other developers use git format-patch to generate patch and
> > submit to the patch system.
> > Git format-patch format can also be accepted by the patch system correctly ?
> > If yes, I think this web should update,
> > Use git format-patch to generate patch is more convenient than use diff -urN
> 
> 'diff -urN' has the advantage that it will work against a tree extracted
> from a release tarball, and doesn't have a requirement that you have git
That's wrong, patches generated by git-format-patch are also applicable
just fine on top of an extracted tar ball by patch(1).

Best regards
Uwe
Valdis Klētnieks Sept. 30, 2014, 9:24 a.m. UTC | #9
On Tue, 30 Sep 2014 09:02:09 +0200, Uwe Kleine-König said:

> > 'diff -urN' has the advantage that it will work against a tree extracted
> > from a release tarball, and doesn't have a requirement that you have git

> That's wrong, patches generated by git-format-patch are also applicable
> just fine on top of an extracted tar ball by patch(1).

How you going to do a git-format-patch without git and a git tree? :)

(Remember, we're talking about *generating* patches, not applying them)
Russell King - ARM Linux Sept. 30, 2014, 9:59 a.m. UTC | #10
On Mon, Sep 29, 2014 at 03:21:58PM -0400, Valdis.Kletnieks@vt.edu wrote:
> On Fri, 26 Sep 2014 10:40:54 +0800, "Wang, Yalin" said:
> 
> > I am really confused,
> > I read this web:
> > http://www.arm.linux.org.uk/developer/patches/info.php
> > it said use diff -urN to generate patch like this:
> >
> > diff -Nru linux.orig/lib/string.c linux/lib/string.c
> >
> > but I see other developers use git format-patch to generate patch and
> > submit to the patch system.
> > Git format-patch format can also be accepted by the patch system correctly ?
> > If yes, I think this web should update,
> > Use git format-patch to generate patch is more convenient than use diff -urN
> 
> 'diff -urN' has the advantage that it will work against a tree extracted
> from a release tarball, and doesn't have a requirement that you have git
> installed.  Having said that, somebody who has access to the website probably
> should update it to mention that both methods are acceptable....

As the website includes *examples* of git formatted patches, no change
is necessary.

I've learned through writing those instructions that it is a bad idea to
_add_ additional words, because it gives people _more_ things to argue
about.  One form of words or examples is fine for one group of people,
but completely confuses another group of people.  You can't win.

That is one of the biggest reasons I hate writing user documentation.
Geert Uytterhoeven Oct. 8, 2014, 11:54 a.m. UTC | #11
On Fri, Sep 19, 2014 at 9:09 AM, Wang, Yalin <Yalin.Wang@sonymobile.com> wrote:
> this patch extend the start and end address of initrd to be page aligned,
> so that we can free all memory including the un-page aligned head or tail
> page of initrd, if the start or end address of initrd are not page
> aligned, the page can't be freed by free_initrd_mem() function.
>
> Signed-off-by: Yalin Wang <yalin.wang@sonymobile.com>
> ---
>  arch/arm/mm/init.c   | 5 +++++
>  arch/arm64/mm/init.c | 8 +++++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> index 659c75d..9221645 100644
> --- a/arch/arm/mm/init.c
> +++ b/arch/arm/mm/init.c
> @@ -636,6 +636,11 @@ static int keep_initrd;
>  void free_initrd_mem(unsigned long start, unsigned long end)
>  {
>         if (!keep_initrd) {
> +               if (start == initrd_start)
> +                       start = round_down(start, PAGE_SIZE);
> +               if (end == initrd_end)
> +                       end = round_up(end, PAGE_SIZE);
> +
>                 poison_init_mem((void *)start, PAGE_ALIGN(end) - start);
>                 free_reserved_area((void *)start, (void *)end, -1, "initrd");
>         }

Who guarantees there's no valuable data in [start, initrd_start)
and [initrd_end, end) being corrupted?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wang, Yalin Oct. 9, 2014, 7:25 a.m. UTC | #12
PiBXaG8gZ3VhcmFudGVlcyB0aGVyZSdzIG5vIHZhbHVhYmxlIGRhdGEgaW4gW3N0YXJ0LCBpbml0
cmRfc3RhcnQpIGFuZA0KPiBbaW5pdHJkX2VuZCwgZW5kKSBiZWluZyBjb3JydXB0ZWQ/DQo+IA0K
bW0uLg0KSSBhbSBub3Qgc3VyZSBpZiB0aGUgbWVtYmxvY2tfcmVzZXJ2ZSB3aWxsIHJlc2VydmUN
Ck1lbW9yeSBmcm9tIHBhZ2UgYWxpZ25lZCBhZGRyZXNzPw0KSWYgbm90LCBkbyB3ZSBuZWVkIGFs
c28gbWFrZSBtZW1ibG9ja19yZXNlcnZlIHRoZSBpbml0cmQgbWVtb3J5DQpGcm9tIHBhZ2UgYWxp
Z25lZCBzdGFydChyb3VuZCBkb3duKSB0byBwYWdlIGFsaWduZWQgZW5kIGFkZHJlc3Mocm91bmQg
dXApID8gDQoNClRoYW5rcw0K
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 659c75d..9221645 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -636,6 +636,11 @@  static int keep_initrd;
 void free_initrd_mem(unsigned long start, unsigned long end)
 {
 	if (!keep_initrd) {
+		if (start == initrd_start)
+			start = round_down(start, PAGE_SIZE);
+		if (end == initrd_end)
+			end = round_up(end, PAGE_SIZE);
+
 		poison_init_mem((void *)start, PAGE_ALIGN(end) - start);
 		free_reserved_area((void *)start, (void *)end, -1, "initrd");
 	}
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 5472c24..c5512f6 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -334,8 +334,14 @@  static int keep_initrd;
 
 void free_initrd_mem(unsigned long start, unsigned long end)
 {
-	if (!keep_initrd)
+	if (!keep_initrd) {
+		if (start == initrd_start)
+			start = round_down(start, PAGE_SIZE);
+		if (end == initrd_end)
+			end = round_up(end, PAGE_SIZE);
+
 		free_reserved_area((void *)start, (void *)end, 0, "initrd");
+	}
 }
 
 static int __init keepinitrd_setup(char *__unused)