Patchwork [GIT,PULL] EFI fix

login
register
mail settings
Submitter Ingo Molnar
Date Jan. 11, 2019, 7:46 a.m.
Message ID <20190111074614.GA68053@gmail.com>
Download mbox | patch
Permalink /patch/697465/
State New
Headers show

Comments

Ingo Molnar - Jan. 11, 2019, 7:46 a.m.
Linus,

Please pull the latest efi-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git efi-urgent-for-linus

   # HEAD: b12f5440d8ca02e8f9ab4f1461f9214295cc4f66 Merge branch 'linus' into efi/urgent, to resolve conflict

A single fix that adds an annotation to resolve a kmemleak false 
positive.

 Thanks,

	Ingo

------------------>
Qian Cai (1):
      efi: Let kmemleak ignore false positives


 drivers/firmware/efi/efi.c | 3 +++
 1 file changed, 3 insertions(+)
Ard Biesheuvel - Jan. 11, 2019, 2:22 p.m.
On Fri, 11 Jan 2019 at 08:46, Ingo Molnar <mingo@kernel.org> wrote:
>
> Linus,
>
> Please pull the latest efi-urgent-for-linus git tree from:
>
>    git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git efi-urgent-for-linus
>
>    # HEAD: b12f5440d8ca02e8f9ab4f1461f9214295cc4f66 Merge branch 'linus' into efi/urgent, to resolve conflict
>
> A single fix that adds an annotation to resolve a kmemleak false
> positive.
>
>  Thanks,
>
>         Ingo
>
> ------------------>
> Qian Cai (1):
>       efi: Let kmemleak ignore false positives
>
>
>  drivers/firmware/efi/efi.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 4c46ff6f2242..7ac09dd8f268 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -31,6 +31,7 @@
>  #include <linux/acpi.h>
>  #include <linux/ucs2_string.h>
>  #include <linux/memblock.h>
> +#include <linux/kmemleak.h>
>
>  #include <asm/early_ioremap.h>
>
> @@ -1026,6 +1027,8 @@ int __ref efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
>         if (!rsv)
>                 return -ENOMEM;
>
> +       kmemleak_ignore(rsv);
> +
>         rsv->size = EFI_MEMRESERVE_COUNT(PAGE_SIZE);
>         atomic_set(&rsv->count, 1);
>         rsv->entry[0].base = addr;

I was hoping we could merge this patch (so we can backport it), but
resolve the conflict by dropping the kmemleak_ignore() again, since it
will now complain since the kmalloc() has been replaced with a
__get_free_page() in the mean time (if that makes sense)
Linus Torvalds - Jan. 11, 2019, 5:47 p.m.
On Thu, Jan 10, 2019 at 11:46 PM Ingo Molnar <mingo@kernel.org> wrote:
>
> A single fix that adds an annotation to resolve a kmemleak false
> positive.

This one is apparently obviated by commit 80424b02d42b ("efi: Reduce
the amount of memblock reservations for persistent allocations")

               Linus
Linus Torvalds - Jan. 11, 2019, 5:55 p.m.
On Fri, Jan 11, 2019 at 6:22 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> I was hoping we could merge this patch (so we can backport it), but
> resolve the conflict by dropping the kmemleak_ignore() again [..]

Well, we'd drop the new #include line also, since it would be
pointless without the kmemleak_ignore().

End result: there would be nothing left. Better not to merge it at all.

It's easy enough to backport, and just say "done differently upstream
in commit 80424b02d42b ("efi: Reduce the amount of memblock
reservations for persistent allocations").

The stable tree doesn't require that the *same* commits be upstream,
it only requires that the fixes be upstream and Greg&al want a pointer
to the upstream fix just so that they know they're not fixing
something that might still be broken upstream.

See for example (just random googling)

   https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=37435f7e80ef9adc32a69013c18f135e3f434244

which shows that "fixed differently upstream" case and points to why.

                  Linus
Ingo Molnar - Jan. 12, 2019, 8:53 a.m.
* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Fri, Jan 11, 2019 at 6:22 AM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> >
> > I was hoping we could merge this patch (so we can backport it), but
> > resolve the conflict by dropping the kmemleak_ignore() again [..]
> 
> Well, we'd drop the new #include line also, since it would be
> pointless without the kmemleak_ignore().
> 
> End result: there would be nothing left. Better not to merge it at all.

Indeed!

> It's easy enough to backport, and just say "done differently upstream
> in commit 80424b02d42b ("efi: Reduce the amount of memblock
> reservations for persistent allocations").
> 
> The stable tree doesn't require that the *same* commits be upstream,
> it only requires that the fixes be upstream and Greg&al want a pointer
> to the upstream fix just so that they know they're not fixing
> something that might still be broken upstream.
> 
> See for example (just random googling)
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=37435f7e80ef9adc32a69013c18f135e3f434244
> 
> which shows that "fixed differently upstream" case and points to why.

Thanks - I'm dropping the commit from efi/urgent.

	Ingo
Ingo Molnar - Jan. 12, 2019, 8:54 a.m.
* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Thu, Jan 10, 2019 at 11:46 PM Ingo Molnar <mingo@kernel.org> wrote:
> >
> > A single fix that adds an annotation to resolve a kmemleak false
> > positive.
> 
> This one is apparently obviated by commit 80424b02d42b ("efi: Reduce
> the amount of memblock reservations for persistent allocations")

... and it turns out I mis-merged it in tip:master not realizing this 
connection (hence the pull request) - so good thing that Ard warned about 
this and you double checked it!

Thanks,

	Ingo

Patch

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 4c46ff6f2242..7ac09dd8f268 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -31,6 +31,7 @@ 
 #include <linux/acpi.h>
 #include <linux/ucs2_string.h>
 #include <linux/memblock.h>
+#include <linux/kmemleak.h>
 
 #include <asm/early_ioremap.h>
 
@@ -1026,6 +1027,8 @@  int __ref efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
 	if (!rsv)
 		return -ENOMEM;
 
+	kmemleak_ignore(rsv);
+
 	rsv->size = EFI_MEMRESERVE_COUNT(PAGE_SIZE);
 	atomic_set(&rsv->count, 1);
 	rsv->entry[0].base = addr;