Pingfan Liu <kernelfans@xxxxxxxxx> writes: > On Wed, Jul 22, 2020 at 12:57 PM Michael Ellerman <mpe@xxxxxxxxxxxxxx> wrote: >> >> Pingfan Liu <kernelfans@xxxxxxxxx> writes: >> > A bug is observed on pseries by taking the following steps on rhel: >> ^ >> RHEL >> >> I assume it happens on mainline too? > Yes, it does. >> > [...] >> > diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c >> > index 1a3ac3b..def8cb3f 100644 >> > --- a/arch/powerpc/platforms/pseries/hotplug-memory.c >> > +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c >> > @@ -372,6 +372,7 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb) >> > invalidate_lmb_associativity_index(lmb); >> > lmb_clear_nid(lmb); >> > lmb->flags &= ~DRCONF_MEM_ASSIGNED; >> > + drmem_update_dt(); >> >> No error checking? > Hmm, here should be a more careful design. Please see the comment at the end. >> >> > __remove_memory(nid, base_addr, block_sz); >> > >> > @@ -607,6 +608,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb) >> > >> > lmb_set_nid(lmb); >> > lmb->flags |= DRCONF_MEM_ASSIGNED; >> > + drmem_update_dt(); >> >> And here .. >> > >> > block_sz = memory_block_size_bytes(); >> > >> > @@ -625,6 +627,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb) >> > invalidate_lmb_associativity_index(lmb); >> > lmb_clear_nid(lmb); >> > lmb->flags &= ~DRCONF_MEM_ASSIGNED; >> > + drmem_update_dt(); >> >> >> And here .. >> >> > __remove_memory(nid, base_addr, block_sz); >> > } >> > @@ -877,9 +880,6 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog) >> > break; >> > } >> > >> > - if (!rc) >> > - rc = drmem_update_dt(); >> > - >> > unlock_device_hotplug(); >> > return rc; >> >> Whereas previously we did check it. > > drmem_update_dt() fails iff allocating memory fail. That's true currently, but it might change in future. > And in the failed case, even the original code does not roll back the > effect of __add_memory()/__remove_memory(). Yeah hard to know what the desired behaviour is. If something fails we at least need to print a message though, not silently swallow it. > And I plan to do the following in V4: if drmem_update_dt() fails in > dlpar_add_lmb(), then bails out immediately. That sounds reasonable. cheers _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec
