Re: [PATCHv5 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
Hello, On Wed, Apr 14, 2021 at 11:08:19AM +0800, Pingfan Liu wrote: > On Sat, Apr 10, 2021 at 12:33 AM Michal Suchánek wrote: > > > > Hello, > > > > On Fri, Aug 28, 2020 at 04:10:09PM +0800, Pingfan Liu wrote: > > > On Thu, Aug 27, 2020 at 3:53 PM Laurent Dufour > > > wrote: > > > > > > > > Le 10/08/2020 à 10:52, Pingfan Liu a écrit : > > > > > A bug is observed on pseries by taking the following steps on rhel: > > > > > -1. drmgr -c mem -r -q 5 > > > > > -2. echo c > /proc/sysrq-trigger > > > > > > > > > > And then, the failure looks like: > > > > > kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/ > > > > > kdump: saving vmcore-dmesg.txt > > > > > kdump: saving vmcore-dmesg.txt complete > > > > > kdump: saving vmcore > > > > > Checking for memory holes : [ 0.0 %] / > > > > > Checking for memory holes : > > > > > [100.0 %] | Excluding unnecessary pages > > > > > : [100.0 %] \ Copying data > > > > > : [ 0.3 %] - eta: 38s[ 44.337636] > > > > > hash-mmu: mm: Hashing failure ! EA=0x7fffba40 > > > > > access=0x8004 current=makedumpfile > > > > > [ 44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base > > > > > psize=2 psize 2 pte=0xc0005504 > > > > > [ 44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 > > > > > access=0x8004 current=makedumpfile > > > > > [ 44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base > > > > > psize=2 psize 2 pte=0xc0005504 > > > > > [ 44.337708] makedumpfile[469]: unhandled signal 7 at > > > > > 7fffba40 nip 7fffbbc4d7fc lr 00011356ca3c code 2 > > > > > [ 44.338548] Core dump to |/bin/false pipe failed > > > > > /lib/kdump-lib-initramfs.sh: line 98: 469 Bus error > > > > > $CORE_COLLECTOR /proc/vmcore > > > > > $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete > > > > > kdump: saving vmcore failed > > > > > > > > > > * Root cause * > > > > >After analyzing, it turns out that in the current implementation, > > > > > when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt > > > > > updating as > > > > > the code __remove_memory() comes before drmem_update_dt(). > > > > > So in kdump kernel, when read_from_oldmem() resorts to > > > > > pSeries_lpar_hpte_insert() to install hpte, but fails with -2 due to > > > > > non-exist pfn. And finally, low_hash_fault() raise SIGBUS to process, > > > > > as it > > > > > can be observed "Bus error" > > > > > > > > > > From a viewpoint of listener and publisher, the publisher notifies > > > > > the > > > > > listener before data is ready. This introduces a problem where udev > > > > > launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before > > > > > updating. And in capture kernel, makedumpfile will access the memory > > > > > based > > > > > on the stale dt info, and hit a SIGBUS error due to an un-existed lmb. > > > > > > > > > > * Fix * > > > > > This bug is introduced by commit 063b8b1251fd > > > > > ("powerpc/pseries/memory-hotplug: Only update DT once per memory DLPAR > > > > > request"), which tried to combine all the dt updating into one. > > > > > > > > > > To fix this issue, meanwhile not to introduce a quadratic runtime > > > > > complexity by the model: > > > > >dlpar_memory_add_by_count > > > > > for_each_drmem_lmb <-- > > > > >dlpar_add_lmb > > > > > drmem_update_dt(_v1|_v2) > > > > >for_each_drmem_lmb <-- > > > > > The dt should still be only updated once, and just before the last > > > > > memory > > > > > online/offline event is ejected to user space. Achieve this by > > > > > tracing the > > > > > num of lmb added or removed. > > > > > > > > > > Signed-off-by: Pingfan Liu > > > > > Cc: Michael Ellerman > > > > > Cc: Hari Bathini > > > > > Cc: Nathan Lynch > > > > > Cc: Nathan Fontenot > > > > > Cc: Laurent Dufour > > > > > To: linuxppc-dev@lists.ozlabs.org > > > > > Cc: ke...@lists.infradead.org > > > > > --- > > > > > v4 -> v5: change dlpar_add_lmb()/dlpar_remove_lmb() prototype to > > > > > report > > > > >whether dt is updated successfully. > > > > >Fix a condition boundary check bug > > > > > v3 -> v4: resolve a quadratic runtime complexity issue. > > > > >This series is applied on next-test branch > > > > > arch/powerpc/platforms/pseries/hotplug-memory.c | 102 > > > > > +++- > > > > > 1 file changed, 80 insertions(+), 22 deletions(-) > > > > > > > > > > diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c > > > > > b/arch/powerpc/platforms/pseries/hotplug-memory.c > > > > > index 46cbcd1..1567d9f 100644 > > > > > --- a/arch/powerpc/platforms/pseries/hotplug-memory.c > > > > > +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c > > > >
Re: [PATCHv5 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
On Sat, Apr 10, 2021 at 12:33 AM Michal Suchánek wrote: > > Hello, > > On Fri, Aug 28, 2020 at 04:10:09PM +0800, Pingfan Liu wrote: > > On Thu, Aug 27, 2020 at 3:53 PM Laurent Dufour > > wrote: > > > > > > Le 10/08/2020 à 10:52, Pingfan Liu a écrit : > > > > A bug is observed on pseries by taking the following steps on rhel: > > > > -1. drmgr -c mem -r -q 5 > > > > -2. echo c > /proc/sysrq-trigger > > > > > > > > And then, the failure looks like: > > > > kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/ > > > > kdump: saving vmcore-dmesg.txt > > > > kdump: saving vmcore-dmesg.txt complete > > > > kdump: saving vmcore > > > > Checking for memory holes : [ 0.0 %] / > > > > Checking for memory holes : [100.0 > > > > %] | Excluding unnecessary pages > > > > : [100.0 %] \ Copying data > > > >: [ 0.3 %] - eta: 38s[ 44.337636] hash-mmu: mm: > > > > Hashing failure ! EA=0x7fffba40 access=0x8004 > > > > current=makedumpfile > > > > [ 44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base > > > > psize=2 psize 2 pte=0xc0005504 > > > > [ 44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 > > > > access=0x8004 current=makedumpfile > > > > [ 44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base > > > > psize=2 psize 2 pte=0xc0005504 > > > > [ 44.337708] makedumpfile[469]: unhandled signal 7 at > > > > 7fffba40 nip 7fffbbc4d7fc lr 00011356ca3c code 2 > > > > [ 44.338548] Core dump to |/bin/false pipe failed > > > > /lib/kdump-lib-initramfs.sh: line 98: 469 Bus error > > > > $CORE_COLLECTOR /proc/vmcore > > > > $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete > > > > kdump: saving vmcore failed > > > > > > > > * Root cause * > > > >After analyzing, it turns out that in the current implementation, > > > > when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt > > > > updating as > > > > the code __remove_memory() comes before drmem_update_dt(). > > > > So in kdump kernel, when read_from_oldmem() resorts to > > > > pSeries_lpar_hpte_insert() to install hpte, but fails with -2 due to > > > > non-exist pfn. And finally, low_hash_fault() raise SIGBUS to process, > > > > as it > > > > can be observed "Bus error" > > > > > > > > From a viewpoint of listener and publisher, the publisher notifies the > > > > listener before data is ready. This introduces a problem where udev > > > > launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before > > > > updating. And in capture kernel, makedumpfile will access the memory > > > > based > > > > on the stale dt info, and hit a SIGBUS error due to an un-existed lmb. > > > > > > > > * Fix * > > > > This bug is introduced by commit 063b8b1251fd > > > > ("powerpc/pseries/memory-hotplug: Only update DT once per memory DLPAR > > > > request"), which tried to combine all the dt updating into one. > > > > > > > > To fix this issue, meanwhile not to introduce a quadratic runtime > > > > complexity by the model: > > > >dlpar_memory_add_by_count > > > > for_each_drmem_lmb <-- > > > >dlpar_add_lmb > > > > drmem_update_dt(_v1|_v2) > > > >for_each_drmem_lmb <-- > > > > The dt should still be only updated once, and just before the last > > > > memory > > > > online/offline event is ejected to user space. Achieve this by tracing > > > > the > > > > num of lmb added or removed. > > > > > > > > Signed-off-by: Pingfan Liu > > > > Cc: Michael Ellerman > > > > Cc: Hari Bathini > > > > Cc: Nathan Lynch > > > > Cc: Nathan Fontenot > > > > Cc: Laurent Dufour > > > > To: linuxppc-dev@lists.ozlabs.org > > > > Cc: ke...@lists.infradead.org > > > > --- > > > > v4 -> v5: change dlpar_add_lmb()/dlpar_remove_lmb() prototype to report > > > >whether dt is updated successfully. > > > >Fix a condition boundary check bug > > > > v3 -> v4: resolve a quadratic runtime complexity issue. > > > >This series is applied on next-test branch > > > > arch/powerpc/platforms/pseries/hotplug-memory.c | 102 > > > > +++- > > > > 1 file changed, 80 insertions(+), 22 deletions(-) > > > > > > > > diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c > > > > b/arch/powerpc/platforms/pseries/hotplug-memory.c > > > > index 46cbcd1..1567d9f 100644 > > > > --- a/arch/powerpc/platforms/pseries/hotplug-memory.c > > > > +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c > > > > @@ -350,13 +350,22 @@ static bool lmb_is_removable(struct drmem_lmb > > > > *lmb) > > > > return true; > > > > } > > > > > > > > -static int dlpar_add_lmb(struct drmem_lmb *); > > > > +enum dt_update_status { > > > > + DT_NOUPDATE, > > > > + DT_TOUPDATE, > > > > +
Re: [PATCHv5 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
Hello, On Fri, Aug 28, 2020 at 04:10:09PM +0800, Pingfan Liu wrote: > On Thu, Aug 27, 2020 at 3:53 PM Laurent Dufour wrote: > > > > Le 10/08/2020 à 10:52, Pingfan Liu a écrit : > > > A bug is observed on pseries by taking the following steps on rhel: > > > -1. drmgr -c mem -r -q 5 > > > -2. echo c > /proc/sysrq-trigger > > > > > > And then, the failure looks like: > > > kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/ > > > kdump: saving vmcore-dmesg.txt > > > kdump: saving vmcore-dmesg.txt complete > > > kdump: saving vmcore > > > Checking for memory holes : [ 0.0 %] / > > > Checking for memory holes : [100.0 %] | > > > Excluding unnecessary pages : > > > [100.0 %] \ Copying data > > > : [ 0.3 %] - eta: 38s[ 44.337636] hash-mmu: mm: Hashing > > > failure ! EA=0x7fffba40 access=0x8004 current=makedumpfile > > > [ 44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base > > > psize=2 psize 2 pte=0xc0005504 > > > [ 44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 > > > access=0x8004 current=makedumpfile > > > [ 44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base > > > psize=2 psize 2 pte=0xc0005504 > > > [ 44.337708] makedumpfile[469]: unhandled signal 7 at 7fffba40 > > > nip 7fffbbc4d7fc lr 00011356ca3c code 2 > > > [ 44.338548] Core dump to |/bin/false pipe failed > > > /lib/kdump-lib-initramfs.sh: line 98: 469 Bus error > > > $CORE_COLLECTOR /proc/vmcore > > > $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete > > > kdump: saving vmcore failed > > > > > > * Root cause * > > >After analyzing, it turns out that in the current implementation, > > > when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt > > > updating as > > > the code __remove_memory() comes before drmem_update_dt(). > > > So in kdump kernel, when read_from_oldmem() resorts to > > > pSeries_lpar_hpte_insert() to install hpte, but fails with -2 due to > > > non-exist pfn. And finally, low_hash_fault() raise SIGBUS to process, as > > > it > > > can be observed "Bus error" > > > > > > From a viewpoint of listener and publisher, the publisher notifies the > > > listener before data is ready. This introduces a problem where udev > > > launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before > > > updating. And in capture kernel, makedumpfile will access the memory based > > > on the stale dt info, and hit a SIGBUS error due to an un-existed lmb. > > > > > > * Fix * > > > This bug is introduced by commit 063b8b1251fd > > > ("powerpc/pseries/memory-hotplug: Only update DT once per memory DLPAR > > > request"), which tried to combine all the dt updating into one. > > > > > > To fix this issue, meanwhile not to introduce a quadratic runtime > > > complexity by the model: > > >dlpar_memory_add_by_count > > > for_each_drmem_lmb <-- > > >dlpar_add_lmb > > > drmem_update_dt(_v1|_v2) > > >for_each_drmem_lmb <-- > > > The dt should still be only updated once, and just before the last memory > > > online/offline event is ejected to user space. Achieve this by tracing the > > > num of lmb added or removed. > > > > > > Signed-off-by: Pingfan Liu > > > Cc: Michael Ellerman > > > Cc: Hari Bathini > > > Cc: Nathan Lynch > > > Cc: Nathan Fontenot > > > Cc: Laurent Dufour > > > To: linuxppc-dev@lists.ozlabs.org > > > Cc: ke...@lists.infradead.org > > > --- > > > v4 -> v5: change dlpar_add_lmb()/dlpar_remove_lmb() prototype to report > > >whether dt is updated successfully. > > >Fix a condition boundary check bug > > > v3 -> v4: resolve a quadratic runtime complexity issue. > > >This series is applied on next-test branch > > > arch/powerpc/platforms/pseries/hotplug-memory.c | 102 > > > +++- > > > 1 file changed, 80 insertions(+), 22 deletions(-) > > > > > > diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c > > > b/arch/powerpc/platforms/pseries/hotplug-memory.c > > > index 46cbcd1..1567d9f 100644 > > > --- a/arch/powerpc/platforms/pseries/hotplug-memory.c > > > +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c > > > @@ -350,13 +350,22 @@ static bool lmb_is_removable(struct drmem_lmb *lmb) > > > return true; > > > } > > > > > > -static int dlpar_add_lmb(struct drmem_lmb *); > > > +enum dt_update_status { > > > + DT_NOUPDATE, > > > + DT_TOUPDATE, > > > + DT_UPDATED, > > > +}; > > > + > > > +/* "*dt_update" returns DT_UPDATED if updated */ > > > +static int dlpar_add_lmb(struct drmem_lmb *lmb, > > > + enum dt_update_status *dt_update); > > > > > > -static int dlpar_remove_lmb(struct drmem_lmb *lmb) > > > +static int dlpar_remove_lmb(struct
Re: [PATCHv5 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
On Thu, Aug 27, 2020 at 3:53 PM Laurent Dufour wrote: > > Le 10/08/2020 à 10:52, Pingfan Liu a écrit : > > A bug is observed on pseries by taking the following steps on rhel: > > -1. drmgr -c mem -r -q 5 > > -2. echo c > /proc/sysrq-trigger > > > > And then, the failure looks like: > > kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/ > > kdump: saving vmcore-dmesg.txt > > kdump: saving vmcore-dmesg.txt complete > > kdump: saving vmcore > > Checking for memory holes : [ 0.0 %] / > > Checking for memory holes : [100.0 %] | > > Excluding unnecessary pages : [100.0 %] > > \ Copying data : [ > > 0.3 %] - eta: 38s[ 44.337636] hash-mmu: mm: Hashing failure ! > > EA=0x7fffba40 access=0x8004 current=makedumpfile > > [ 44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 > > psize 2 pte=0xc0005504 > > [ 44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 > > access=0x8004 current=makedumpfile > > [ 44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 > > psize 2 pte=0xc0005504 > > [ 44.337708] makedumpfile[469]: unhandled signal 7 at 7fffba40 > > nip 7fffbbc4d7fc lr 00011356ca3c code 2 > > [ 44.338548] Core dump to |/bin/false pipe failed > > /lib/kdump-lib-initramfs.sh: line 98: 469 Bus error > > $CORE_COLLECTOR /proc/vmcore > > $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete > > kdump: saving vmcore failed > > > > * Root cause * > >After analyzing, it turns out that in the current implementation, > > when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt updating > > as > > the code __remove_memory() comes before drmem_update_dt(). > > So in kdump kernel, when read_from_oldmem() resorts to > > pSeries_lpar_hpte_insert() to install hpte, but fails with -2 due to > > non-exist pfn. And finally, low_hash_fault() raise SIGBUS to process, as it > > can be observed "Bus error" > > > > From a viewpoint of listener and publisher, the publisher notifies the > > listener before data is ready. This introduces a problem where udev > > launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before > > updating. And in capture kernel, makedumpfile will access the memory based > > on the stale dt info, and hit a SIGBUS error due to an un-existed lmb. > > > > * Fix * > > This bug is introduced by commit 063b8b1251fd > > ("powerpc/pseries/memory-hotplug: Only update DT once per memory DLPAR > > request"), which tried to combine all the dt updating into one. > > > > To fix this issue, meanwhile not to introduce a quadratic runtime > > complexity by the model: > >dlpar_memory_add_by_count > > for_each_drmem_lmb <-- > >dlpar_add_lmb > > drmem_update_dt(_v1|_v2) > >for_each_drmem_lmb <-- > > The dt should still be only updated once, and just before the last memory > > online/offline event is ejected to user space. Achieve this by tracing the > > num of lmb added or removed. > > > > Signed-off-by: Pingfan Liu > > Cc: Michael Ellerman > > Cc: Hari Bathini > > Cc: Nathan Lynch > > Cc: Nathan Fontenot > > Cc: Laurent Dufour > > To: linuxppc-dev@lists.ozlabs.org > > Cc: ke...@lists.infradead.org > > --- > > v4 -> v5: change dlpar_add_lmb()/dlpar_remove_lmb() prototype to report > >whether dt is updated successfully. > >Fix a condition boundary check bug > > v3 -> v4: resolve a quadratic runtime complexity issue. > >This series is applied on next-test branch > > arch/powerpc/platforms/pseries/hotplug-memory.c | 102 > > +++- > > 1 file changed, 80 insertions(+), 22 deletions(-) > > > > diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c > > b/arch/powerpc/platforms/pseries/hotplug-memory.c > > index 46cbcd1..1567d9f 100644 > > --- a/arch/powerpc/platforms/pseries/hotplug-memory.c > > +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c > > @@ -350,13 +350,22 @@ static bool lmb_is_removable(struct drmem_lmb *lmb) > > return true; > > } > > > > -static int dlpar_add_lmb(struct drmem_lmb *); > > +enum dt_update_status { > > + DT_NOUPDATE, > > + DT_TOUPDATE, > > + DT_UPDATED, > > +}; > > + > > +/* "*dt_update" returns DT_UPDATED if updated */ > > +static int dlpar_add_lmb(struct drmem_lmb *lmb, > > + enum dt_update_status *dt_update); > > > > -static int dlpar_remove_lmb(struct drmem_lmb *lmb) > > +static int dlpar_remove_lmb(struct drmem_lmb *lmb, > > + enum dt_update_status *dt_update) > > { > > unsigned long block_sz; > > phys_addr_t base_addr; > > - int rc, nid; > > + int rc, ret, nid; > > > > if (!lmb_is_removable(lmb)) > > return -EINVAL; > > @@ -372,6 +381,13
Re: [PATCHv5 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
Le 10/08/2020 à 10:52, Pingfan Liu a écrit : A bug is observed on pseries by taking the following steps on rhel: -1. drmgr -c mem -r -q 5 -2. echo c > /proc/sysrq-trigger And then, the failure looks like: kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/ kdump: saving vmcore-dmesg.txt kdump: saving vmcore-dmesg.txt complete kdump: saving vmcore Checking for memory holes : [ 0.0 %] / Checking for memory holes : [100.0 %] | Excluding unnecessary pages : [100.0 %] \ Copying data : [ 0.3 %] - eta: 38s[ 44.337636] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 access=0x8004 current=makedumpfile [ 44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc0005504 [ 44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 access=0x8004 current=makedumpfile [ 44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc0005504 [ 44.337708] makedumpfile[469]: unhandled signal 7 at 7fffba40 nip 7fffbbc4d7fc lr 00011356ca3c code 2 [ 44.338548] Core dump to |/bin/false pipe failed /lib/kdump-lib-initramfs.sh: line 98: 469 Bus error $CORE_COLLECTOR /proc/vmcore $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete kdump: saving vmcore failed * Root cause * After analyzing, it turns out that in the current implementation, when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt updating as the code __remove_memory() comes before drmem_update_dt(). So in kdump kernel, when read_from_oldmem() resorts to pSeries_lpar_hpte_insert() to install hpte, but fails with -2 due to non-exist pfn. And finally, low_hash_fault() raise SIGBUS to process, as it can be observed "Bus error" From a viewpoint of listener and publisher, the publisher notifies the listener before data is ready. This introduces a problem where udev launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before updating. And in capture kernel, makedumpfile will access the memory based on the stale dt info, and hit a SIGBUS error due to an un-existed lmb. * Fix * This bug is introduced by commit 063b8b1251fd ("powerpc/pseries/memory-hotplug: Only update DT once per memory DLPAR request"), which tried to combine all the dt updating into one. To fix this issue, meanwhile not to introduce a quadratic runtime complexity by the model: dlpar_memory_add_by_count for_each_drmem_lmb <-- dlpar_add_lmb drmem_update_dt(_v1|_v2) for_each_drmem_lmb <-- The dt should still be only updated once, and just before the last memory online/offline event is ejected to user space. Achieve this by tracing the num of lmb added or removed. Signed-off-by: Pingfan Liu Cc: Michael Ellerman Cc: Hari Bathini Cc: Nathan Lynch Cc: Nathan Fontenot Cc: Laurent Dufour To: linuxppc-dev@lists.ozlabs.org Cc: ke...@lists.infradead.org --- v4 -> v5: change dlpar_add_lmb()/dlpar_remove_lmb() prototype to report whether dt is updated successfully. Fix a condition boundary check bug v3 -> v4: resolve a quadratic runtime complexity issue. This series is applied on next-test branch arch/powerpc/platforms/pseries/hotplug-memory.c | 102 +++- 1 file changed, 80 insertions(+), 22 deletions(-) diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c index 46cbcd1..1567d9f 100644 --- a/arch/powerpc/platforms/pseries/hotplug-memory.c +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -350,13 +350,22 @@ static bool lmb_is_removable(struct drmem_lmb *lmb) return true; } -static int dlpar_add_lmb(struct drmem_lmb *); +enum dt_update_status { + DT_NOUPDATE, + DT_TOUPDATE, + DT_UPDATED, +}; + +/* "*dt_update" returns DT_UPDATED if updated */ +static int dlpar_add_lmb(struct drmem_lmb *lmb, + enum dt_update_status *dt_update); -static int dlpar_remove_lmb(struct drmem_lmb *lmb) +static int dlpar_remove_lmb(struct drmem_lmb *lmb, + enum dt_update_status *dt_update) { unsigned long block_sz; phys_addr_t base_addr; - int rc, nid; + int rc, ret, nid; if (!lmb_is_removable(lmb)) return -EINVAL; @@ -372,6 +381,13 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb) invalidate_lmb_associativity_index(lmb); lmb_clear_nid(lmb); lmb->flags &= ~DRCONF_MEM_ASSIGNED; + if (*dt_update) { That test is wrong, you should do: if (*dt_update && *dt_update == DT_TOUPDATE) { With the current code, the device tree is updated all the time. Another option would be to pass a valid pointer (!= NULL) only when DT update is required, this way you
Re: [PATCHv5 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
Hello guys. Do you have further comments on this version? Thanks, Pingfan On Mon, Aug 10, 2020 at 4:53 PM Pingfan Liu wrote: > > A bug is observed on pseries by taking the following steps on rhel: > -1. drmgr -c mem -r -q 5 > -2. echo c > /proc/sysrq-trigger > > And then, the failure looks like: > kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/ > kdump: saving vmcore-dmesg.txt > kdump: saving vmcore-dmesg.txt complete > kdump: saving vmcore > Checking for memory holes : [ 0.0 %] / > Checking for memory holes : [100.0 %] | > Excluding unnecessary pages : [100.0 %] \ > Copying data : [ 0.3 %] - > eta: 38s[ 44.337636] hash-mmu: mm: Hashing failure ! > EA=0x7fffba40 access=0x8004 current=makedumpfile > [ 44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 > psize 2 pte=0xc0005504 > [ 44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 > access=0x8004 current=makedumpfile > [ 44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 > psize 2 pte=0xc0005504 > [ 44.337708] makedumpfile[469]: unhandled signal 7 at 7fffba40 nip > 7fffbbc4d7fc lr 00011356ca3c code 2 > [ 44.338548] Core dump to |/bin/false pipe failed > /lib/kdump-lib-initramfs.sh: line 98: 469 Bus error > $CORE_COLLECTOR /proc/vmcore > $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete > kdump: saving vmcore failed > > * Root cause * > After analyzing, it turns out that in the current implementation, > when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt updating as > the code __remove_memory() comes before drmem_update_dt(). > So in kdump kernel, when read_from_oldmem() resorts to > pSeries_lpar_hpte_insert() to install hpte, but fails with -2 due to > non-exist pfn. And finally, low_hash_fault() raise SIGBUS to process, as it > can be observed "Bus error" > > From a viewpoint of listener and publisher, the publisher notifies the > listener before data is ready. This introduces a problem where udev > launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before > updating. And in capture kernel, makedumpfile will access the memory based > on the stale dt info, and hit a SIGBUS error due to an un-existed lmb. > > * Fix * > This bug is introduced by commit 063b8b1251fd > ("powerpc/pseries/memory-hotplug: Only update DT once per memory DLPAR > request"), which tried to combine all the dt updating into one. > > To fix this issue, meanwhile not to introduce a quadratic runtime > complexity by the model: > dlpar_memory_add_by_count > for_each_drmem_lmb <-- > dlpar_add_lmb > drmem_update_dt(_v1|_v2) > for_each_drmem_lmb <-- > The dt should still be only updated once, and just before the last memory > online/offline event is ejected to user space. Achieve this by tracing the > num of lmb added or removed. > > Signed-off-by: Pingfan Liu > Cc: Michael Ellerman > Cc: Hari Bathini > Cc: Nathan Lynch > Cc: Nathan Fontenot > Cc: Laurent Dufour > To: linuxppc-dev@lists.ozlabs.org > Cc: ke...@lists.infradead.org > --- > v4 -> v5: change dlpar_add_lmb()/dlpar_remove_lmb() prototype to report > whether dt is updated successfully. > Fix a condition boundary check bug > v3 -> v4: resolve a quadratic runtime complexity issue. > This series is applied on next-test branch > arch/powerpc/platforms/pseries/hotplug-memory.c | 102 > +++- > 1 file changed, 80 insertions(+), 22 deletions(-) > > diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c > b/arch/powerpc/platforms/pseries/hotplug-memory.c > index 46cbcd1..1567d9f 100644 > --- a/arch/powerpc/platforms/pseries/hotplug-memory.c > +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c > @@ -350,13 +350,22 @@ static bool lmb_is_removable(struct drmem_lmb *lmb) > return true; > } > > -static int dlpar_add_lmb(struct drmem_lmb *); > +enum dt_update_status { > + DT_NOUPDATE, > + DT_TOUPDATE, > + DT_UPDATED, > +}; > + > +/* "*dt_update" returns DT_UPDATED if updated */ > +static int dlpar_add_lmb(struct drmem_lmb *lmb, > + enum dt_update_status *dt_update); > > -static int dlpar_remove_lmb(struct drmem_lmb *lmb) > +static int dlpar_remove_lmb(struct drmem_lmb *lmb, > + enum dt_update_status *dt_update) > { > unsigned long block_sz; > phys_addr_t base_addr; > - int rc, nid; > + int rc, ret, nid; > > if (!lmb_is_removable(lmb)) > return -EINVAL; > @@ -372,6 +381,13 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb) > invalidate_lmb_associativity_index(lmb); > lmb_clear_nid(lmb); > lmb->flags &= ~DRCONF_MEM_ASSIGNED; > +
[PATCHv5 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
A bug is observed on pseries by taking the following steps on rhel: -1. drmgr -c mem -r -q 5 -2. echo c > /proc/sysrq-trigger And then, the failure looks like: kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/ kdump: saving vmcore-dmesg.txt kdump: saving vmcore-dmesg.txt complete kdump: saving vmcore Checking for memory holes : [ 0.0 %] / Checking for memory holes : [100.0 %] | Excluding unnecessary pages : [100.0 %] \ Copying data : [ 0.3 %] - eta: 38s[ 44.337636] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 access=0x8004 current=makedumpfile [ 44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc0005504 [ 44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 access=0x8004 current=makedumpfile [ 44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc0005504 [ 44.337708] makedumpfile[469]: unhandled signal 7 at 7fffba40 nip 7fffbbc4d7fc lr 00011356ca3c code 2 [ 44.338548] Core dump to |/bin/false pipe failed /lib/kdump-lib-initramfs.sh: line 98: 469 Bus error $CORE_COLLECTOR /proc/vmcore $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete kdump: saving vmcore failed * Root cause * After analyzing, it turns out that in the current implementation, when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt updating as the code __remove_memory() comes before drmem_update_dt(). So in kdump kernel, when read_from_oldmem() resorts to pSeries_lpar_hpte_insert() to install hpte, but fails with -2 due to non-exist pfn. And finally, low_hash_fault() raise SIGBUS to process, as it can be observed "Bus error" >From a viewpoint of listener and publisher, the publisher notifies the listener before data is ready. This introduces a problem where udev launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before updating. And in capture kernel, makedumpfile will access the memory based on the stale dt info, and hit a SIGBUS error due to an un-existed lmb. * Fix * This bug is introduced by commit 063b8b1251fd ("powerpc/pseries/memory-hotplug: Only update DT once per memory DLPAR request"), which tried to combine all the dt updating into one. To fix this issue, meanwhile not to introduce a quadratic runtime complexity by the model: dlpar_memory_add_by_count for_each_drmem_lmb <-- dlpar_add_lmb drmem_update_dt(_v1|_v2) for_each_drmem_lmb <-- The dt should still be only updated once, and just before the last memory online/offline event is ejected to user space. Achieve this by tracing the num of lmb added or removed. Signed-off-by: Pingfan Liu Cc: Michael Ellerman Cc: Hari Bathini Cc: Nathan Lynch Cc: Nathan Fontenot Cc: Laurent Dufour To: linuxppc-dev@lists.ozlabs.org Cc: ke...@lists.infradead.org --- v4 -> v5: change dlpar_add_lmb()/dlpar_remove_lmb() prototype to report whether dt is updated successfully. Fix a condition boundary check bug v3 -> v4: resolve a quadratic runtime complexity issue. This series is applied on next-test branch arch/powerpc/platforms/pseries/hotplug-memory.c | 102 +++- 1 file changed, 80 insertions(+), 22 deletions(-) diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c index 46cbcd1..1567d9f 100644 --- a/arch/powerpc/platforms/pseries/hotplug-memory.c +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -350,13 +350,22 @@ static bool lmb_is_removable(struct drmem_lmb *lmb) return true; } -static int dlpar_add_lmb(struct drmem_lmb *); +enum dt_update_status { + DT_NOUPDATE, + DT_TOUPDATE, + DT_UPDATED, +}; + +/* "*dt_update" returns DT_UPDATED if updated */ +static int dlpar_add_lmb(struct drmem_lmb *lmb, + enum dt_update_status *dt_update); -static int dlpar_remove_lmb(struct drmem_lmb *lmb) +static int dlpar_remove_lmb(struct drmem_lmb *lmb, + enum dt_update_status *dt_update) { unsigned long block_sz; phys_addr_t base_addr; - int rc, nid; + int rc, ret, nid; if (!lmb_is_removable(lmb)) return -EINVAL; @@ -372,6 +381,13 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb) invalidate_lmb_associativity_index(lmb); lmb_clear_nid(lmb); lmb->flags &= ~DRCONF_MEM_ASSIGNED; + if (*dt_update) { + ret = drmem_update_dt(); + if (ret) + pr_warn("%s fail to update dt, but continue\n", __func__); + else + *dt_update = DT_UPDATED; + } __remove_memory(nid, base_addr, block_sz); @@ -387,6 +403,7 @@ static int