Hi Marek,
On Wed, 11 Oct 2017 23:34:33 +0200
Marek Vasut <marek.vasut(a)gmail.com> wrote:
> On 10/11/2017 03:54 PM, Arnd Bergmann wrote:
> > The map_word_() functions, dating back to linux-2.6.8, try to perform
> > bitwise operations on a 'map_word' structure. This may have worked
> > with compilers that were current then (gcc-3.4 or earlier), but end
> > up being rather inefficient on any version I could try now (gcc-4.4 or
> > higher). Specifically we hit a problem analyzed in gcc PR81715 where we
> > fail to reuse the stack space for local variables.
> >
> > This can be seen immediately in the stack consumption for
> > cfi_staa_erase_varsize() and other functions that (with CONFIG_KASAN)
> > can be up to 2200 bytes. Changing the inline functions into macros brings
> > this down to 1280 bytes. Without KASAN, the same problem exists, but
> > the stack consumption is lower to start with, my patch shrinks it from
> > 920 to 496 bytes on with arm-linux-gnueabi-gcc-5.4, and saves around
> > 1KB in .text size for cfi_cmdset_0020.c, as it avoids copying map_word
> > structures for each call to one of these helpers.
> >
> > With the latest gcc-8 snapshot, the problem is fixed in upstream gcc,
> > but nobody uses that yet, so we should still work around it in mainline
> > kernels and probably backport the workaround to stable kernels as well.
> > We had a couple of other functions that suffered from the same gcc bug,
> > and all of those had a simpler workaround involving dummy variables
> > in the inline function. Unfortunately that did not work here, the
> > macro hack was the best I could come up with.
> >
> > It would also be helpful to have someone to a little performance testing
> > on the patch, to see how much it helps in terms of CPU utilitzation.
> >
> > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715
> > Cc: stable(a)vger.kernel.org
> > Signed-off-by: Arnd Bergmann <arnd(a)arndb.de>
>
> Don't you lose type-checking with this conversion to macros ?
>
Yes, we loose strict type checking, but if you look at the code, you'll
see that the macros do (valN).x[i], so, if valN is not a struct or
a union containing a field named x, the compiler will complain. That
should save us from devs passing random arguments to those macros.
Anyway, this code is not seeing a lot of changes lately, so I wouldn't
be so worried by the lack of strict type-checking implied by this
transition to macros.
Regards,
Boris
On Sat, Dec 16, 2017 at 2:51 AM, Brian King <brking(a)linux.vnet.ibm.com> wrote:
> This fixes a hang issue seen when changing the MTU size from 1500 MTU
> to 9000 MTU on both 5717 and 5719 chips. In discussion with Broadcom,
> they've indicated that these chipsets have the same phy as the 57766
> chipset, so the same workarounds apply. This has been tested by IBM
> on both Power 8 and Power 9 systems as well as by Broadcom on x86
> hardware and has been confirmed to resolve the hang issue.
Thanks for the patch. We need more time to review.
>
> Cc: stable <stable(a)vger.kernel.org>
> Signed-off-by: Brian King <brking(a)linux.vnet.ibm.com>
> ---
> drivers/net/ethernet/broadcom/tg3.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index de51c21..d09c5a9 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -14225,7 +14225,9 @@ static int tg3_change_mtu(struct net_device *dev, int new_mtu)
> /* Reset PHY, otherwise the read DMA engine will be in a mode that
> * breaks all requests to 256 bytes.
> */
> - if (tg3_asic_rev(tp) == ASIC_REV_57766)
> + if (tg3_asic_rev(tp) == ASIC_REV_57766 ||
> + tg3_asic_rev(tp) == ASIC_REV_5717 ||
> + tg3_asic_rev(tp) == ASIC_REV_5719)
> reset_phy = true;
>
> err = tg3_restart_hw(tp, reset_phy);
> --
> 1.8.3.1
>
syzbot reported we have a use-after-free when mqueue_evict_inode()
is called on __cleanup_mnt() path, where the ipc ns is already
freed by the previous exit_task_namespaces(). We can just move
it after after exit_task_work() to avoid this use-after-free.
Reported-by: syzbot <syzkaller(a)googlegroups.com>
Cc: Ingo Molnar <mingo(a)kernel.org>
Cc: Al Viro <viro(a)zeniv.linux.org.uk>
Cc: Andrew Morton <akpm(a)linux-foundation.org>
Cc: Linus Torvalds <torvalds(a)linux-foundation.org>
Cc: stable(a)vger.kernel.org
Signed-off-by: Cong Wang <xiyou.wangcong(a)gmail.com>
---
kernel/exit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/exit.c b/kernel/exit.c
index 6b4298a41167..909e43c45158 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -861,8 +861,8 @@ void __noreturn do_exit(long code)
exit_fs(tsk);
if (group_dead)
disassociate_ctty(1);
- exit_task_namespaces(tsk);
exit_task_work(tsk);
+ exit_task_namespaces(tsk);
exit_thread(tsk);
/*
--
2.13.0