All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Rapoport <rppt@kernel.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: linux-xtensa@linux-xtensa.org,
	Florian Fainelli <f.fainelli@gmail.com>,
	Chris Zankel <chris@zankel.net>,
	Linus Walleij <linus.walleij@linaro.org>,
	Russell King - ARM Linux admin <linux@armlinux.org.uk>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Max Filippov <jcmvbkbc@gmail.com>,
	Mike Rapoport <rppt@linux.ibm.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] ARM, xtensa: highmem: avoid clobbering non-page aligned memory reservations
Date: Sat, 31 Oct 2020 17:45:28 +0200	[thread overview]
Message-ID: <20201031154528.GA14628@kernel.org> (raw)
In-Reply-To: <CAMj1kXED+Ry7FEtZm-5tL5ZD2c4+nbYtCWrzQmd158+jXHNqPQ@mail.gmail.com>

On Sat, Oct 31, 2020 at 12:21:24PM +0100, Ard Biesheuvel wrote:
> On Sat, 31 Oct 2020 at 12:04, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Sat, Oct 31, 2020 at 11:47:42AM +0100, Ard Biesheuvel wrote:
> > > On Sat, 31 Oct 2020 at 11:33, Russell King - ARM Linux admin
> > > <linux@armlinux.org.uk> wrote:
> > > >
> > > > On Sat, Oct 31, 2020 at 11:43:45AM +0200, Mike Rapoport wrote:
> > > > > From: Ard Biesheuvel <ardb@kernel.org>
> > > > >
> > > > > free_highpages() iterates over the free memblock regions in high
> > > > > memory, and marks each page as available for the memory management
> > > > > system.
> > > > >
> > > > > Until commit cddb5ddf2b76 ("arm, xtensa: simplify initialization of
> > > > > high memory pages") it rounded beginning of each region upwards and end of
> > > > > each region downwards.
> > > > >
> > > > > However, after that commit free_highmem() rounds the beginning and end of
> > > > > each region downwards, and we may end up freeing a page that is
> > > > > memblock_reserve()d, resulting in memory corruption.
> > > > >
> > > > > Restore the original rounding of the region boundaries to avoid freeing
> > > > > reserved pages.
> > > > >
> > > > > Fixes: cddb5ddf2b76 ("arm, xtensa: simplify initialization of high memory pages")
> > > > > Link: https://lore.kernel.org/r/20201029110334.4118-1-ardb@kernel.org/
> > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > > Co-developed-by:  Mike Rapoport <rppt@linux.ibm.com>
> > > > > Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> > > > > ---
> > > > >
> > > > > Max, Russell,
> > > > >
> > > > > Please let me know how do you prefer to take it upstream.
> > > > > If needed this can go via memblock tree.
> > > > >
> > > > > v2: fix words order in the commit message
> > > >
> > > > I really don't understand what is going on here; there seems to be a
> > > > total disconnect of communication between yourself and Ard. Ard has
> > > > already submitted a different patch for this to the patch system
> > > > already, sent yesterday.
> > > >
> > > > https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9021/1
> > > >
> > > > Please discuss between yourselves how you want to solve the problem,
> > > > and then submit an agreed and tested patch to those of us upstream;
> > > > please don't make it for those upstream to pick one of your patches
> > > > as you are at present.
> > > >
> > >
> > > Apologies for creating this confusion. I posted a patch and dropped it
> > > into the patch system when I found the bug.
> > >
> > > However, only when Florian asked about a 'fixes' tag, I went back to
> > > the history, and realized that the issue was introduced by Mike during
> > > the most recent merge window, and affects xtensa as well.
> >
> > So why does Mike's patch have:
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >
> > in it? It seems you haven't been directly involved in Mike's patch.
> >
> 
> Because I cc'ed him on the discussion following the patch that is now
> in your patch system. So he took that patch and modified it, but
> retained the original S-o-b and authorship.

Right, that's exactly what happened.

> > There's something /really/ not right with the process behind this
> > patch.
> >
> > > I don't have a preference which patch gets applied, though, so please
> > > indicate your preference, and we will adapt accordingly.
> >
> > I asked for you both to come to a concensus about how you want to
> > proceed, and now you're throwing it back on to me to solve your(pl)
> > mis-communication issue. We haven't heard from Mike yet.
> >
> 
> I am not throwing it back to you. I merely indicated that I have no
> preference, and since Mike is the one who introduced this issue in the
> first place, I am expecting him to drive this. And indeed, we haven't
> heard from him yet.

I didn't know that Ard's patch was already in the patch system. I took
the patch from the list, updated it, added a fix for xtensa and resend
while retaining Ard's authourship and s-o-b.

> > Clearly, I wasn't blunt and stroppy enough to be properly understood.
> > Sort it out between yourselves and tell me which patch you want me to
> > apply.
> >
> 
> I would like you to ack this version of the patch, and disregard the
> one in the patch system, so that Mike can take this one through the
> memblock tree where the issue originated in the first place.

Agree.

-- 
Sincerely yours,
Mike.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Mike Rapoport <rppt@kernel.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>,
	linux-xtensa@linux-xtensa.org,
	Florian Fainelli <f.fainelli@gmail.com>,
	Chris Zankel <chris@zankel.net>,
	Linus Walleij <linus.walleij@linaro.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Mike Rapoport <rppt@linux.ibm.com>,
	Max Filippov <jcmvbkbc@gmail.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] ARM, xtensa: highmem: avoid clobbering non-page aligned memory reservations
Date: Sat, 31 Oct 2020 17:45:28 +0200	[thread overview]
Message-ID: <20201031154528.GA14628@kernel.org> (raw)
In-Reply-To: <CAMj1kXED+Ry7FEtZm-5tL5ZD2c4+nbYtCWrzQmd158+jXHNqPQ@mail.gmail.com>

On Sat, Oct 31, 2020 at 12:21:24PM +0100, Ard Biesheuvel wrote:
> On Sat, 31 Oct 2020 at 12:04, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Sat, Oct 31, 2020 at 11:47:42AM +0100, Ard Biesheuvel wrote:
> > > On Sat, 31 Oct 2020 at 11:33, Russell King - ARM Linux admin
> > > <linux@armlinux.org.uk> wrote:
> > > >
> > > > On Sat, Oct 31, 2020 at 11:43:45AM +0200, Mike Rapoport wrote:
> > > > > From: Ard Biesheuvel <ardb@kernel.org>
> > > > >
> > > > > free_highpages() iterates over the free memblock regions in high
> > > > > memory, and marks each page as available for the memory management
> > > > > system.
> > > > >
> > > > > Until commit cddb5ddf2b76 ("arm, xtensa: simplify initialization of
> > > > > high memory pages") it rounded beginning of each region upwards and end of
> > > > > each region downwards.
> > > > >
> > > > > However, after that commit free_highmem() rounds the beginning and end of
> > > > > each region downwards, and we may end up freeing a page that is
> > > > > memblock_reserve()d, resulting in memory corruption.
> > > > >
> > > > > Restore the original rounding of the region boundaries to avoid freeing
> > > > > reserved pages.
> > > > >
> > > > > Fixes: cddb5ddf2b76 ("arm, xtensa: simplify initialization of high memory pages")
> > > > > Link: https://lore.kernel.org/r/20201029110334.4118-1-ardb@kernel.org/
> > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > > Co-developed-by:  Mike Rapoport <rppt@linux.ibm.com>
> > > > > Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> > > > > ---
> > > > >
> > > > > Max, Russell,
> > > > >
> > > > > Please let me know how do you prefer to take it upstream.
> > > > > If needed this can go via memblock tree.
> > > > >
> > > > > v2: fix words order in the commit message
> > > >
> > > > I really don't understand what is going on here; there seems to be a
> > > > total disconnect of communication between yourself and Ard. Ard has
> > > > already submitted a different patch for this to the patch system
> > > > already, sent yesterday.
> > > >
> > > > https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9021/1
> > > >
> > > > Please discuss between yourselves how you want to solve the problem,
> > > > and then submit an agreed and tested patch to those of us upstream;
> > > > please don't make it for those upstream to pick one of your patches
> > > > as you are at present.
> > > >
> > >
> > > Apologies for creating this confusion. I posted a patch and dropped it
> > > into the patch system when I found the bug.
> > >
> > > However, only when Florian asked about a 'fixes' tag, I went back to
> > > the history, and realized that the issue was introduced by Mike during
> > > the most recent merge window, and affects xtensa as well.
> >
> > So why does Mike's patch have:
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >
> > in it? It seems you haven't been directly involved in Mike's patch.
> >
> 
> Because I cc'ed him on the discussion following the patch that is now
> in your patch system. So he took that patch and modified it, but
> retained the original S-o-b and authorship.

Right, that's exactly what happened.

> > There's something /really/ not right with the process behind this
> > patch.
> >
> > > I don't have a preference which patch gets applied, though, so please
> > > indicate your preference, and we will adapt accordingly.
> >
> > I asked for you both to come to a concensus about how you want to
> > proceed, and now you're throwing it back on to me to solve your(pl)
> > mis-communication issue. We haven't heard from Mike yet.
> >
> 
> I am not throwing it back to you. I merely indicated that I have no
> preference, and since Mike is the one who introduced this issue in the
> first place, I am expecting him to drive this. And indeed, we haven't
> heard from him yet.

I didn't know that Ard's patch was already in the patch system. I took
the patch from the list, updated it, added a fix for xtensa and resend
while retaining Ard's authourship and s-o-b.

> > Clearly, I wasn't blunt and stroppy enough to be properly understood.
> > Sort it out between yourselves and tell me which patch you want me to
> > apply.
> >
> 
> I would like you to ack this version of the patch, and disregard the
> one in the patch system, so that Mike can take this one through the
> memblock tree where the issue originated in the first place.

Agree.

-- 
Sincerely yours,
Mike.

  reply	other threads:[~2020-10-31 15:46 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-31  9:43 [PATCH] ARM, xtensa: highmem: avoid clobbering non-page aligned memory reservations Mike Rapoport
2020-10-31  9:43 ` Mike Rapoport
2020-10-31 10:33 ` Russell King - ARM Linux admin
2020-10-31 10:33   ` Russell King - ARM Linux admin
2020-10-31 10:47   ` Ard Biesheuvel
2020-10-31 10:47     ` Ard Biesheuvel
2020-10-31 11:03     ` Russell King - ARM Linux admin
2020-10-31 11:03       ` Russell King - ARM Linux admin
2020-10-31 11:21       ` Ard Biesheuvel
2020-10-31 11:21         ` Ard Biesheuvel
2020-10-31 15:45         ` Mike Rapoport [this message]
2020-10-31 15:45           ` Mike Rapoport
2020-11-04  8:51           ` Mike Rapoport
2020-11-04  8:51             ` Mike Rapoport
2020-10-31 16:37 ` Max Filippov
2020-10-31 16:37   ` Max Filippov
2020-10-31 17:16   ` Mike Rapoport
2020-10-31 17:16     ` Mike Rapoport
2020-10-31 17:44     ` Max Filippov
2020-10-31 17:44       ` Max Filippov
2020-11-04  8:35       ` Ard Biesheuvel
2020-11-04  8:35         ` Ard Biesheuvel
2020-11-04  8:49         ` Mike Rapoport
2020-11-04  8:49           ` Mike Rapoport

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=20201031154528.GA14628@kernel.org \
    --to=rppt@kernel.org \
    --cc=ardb@kernel.org \
    --cc=chris@zankel.net \
    --cc=f.fainelli@gmail.com \
    --cc=jcmvbkbc@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xtensa@linux-xtensa.org \
    --cc=linux@armlinux.org.uk \
    --cc=rppt@linux.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.