|
From: | Daniel Henrique Barboza |
Subject: | Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/5] hw/ppc: removing spapr_drc_detach_cb opaques |
Date: | Wed, 3 May 2017 10:56:57 -0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0 |
On 05/03/2017 12:26 AM, Bharata B Rao
wrote:
I wasn't clear enough in my last comment. Let me rephrase. Is there any other use for the 'spapr_lmb_release' callback function other than being called by the spapr_del_lmbs() in the flow you've stated above? Searching the master code now I've found: $ grep -R 'spapr_lmb_release' . ./spapr.c:static void spapr_lmb_release(DeviceState *dev, void *opaque) ./spapr.c: drck->detach(drc, dev, spapr_lmb_release, ds, errp); Note that all the callback is doing is asserting that a nr_lmb counter will be zero after a decrement and, if true, execute the following: hotplug_ctrl = qdev_get_hotplug_handler(dev); hotplug_handler_unplug(hotplug_ctrl, dev, &error_abort); So, if the callback spapr_lmb_release is only being called in the following for loop of spapr_del_lmbs() to clean up each LMB DRC, can't we get rid of it and do the following after this for loop? for (i = 0; i < nr_lmbs; i++) { drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB, addr / SPAPR_MEMORY_BLOCK_SIZE); g_assert(drc); drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); drck->detach(drc, dev, ds, errp); addr += SPAPR_MEMORY_BLOCK_SIZE; } if (!spapr_all_lmbs_drcs_released(PC_DIMM(dev))) { // All LMBs were cleared, proceed with detach hotplug_ctrl = qdev_get_hotplug_handler(dev); hotplug_handler_unplug(hotplug_ctrl, dev, &error_abort); } // proceed with spapr_del_lmbs code Doesn't this code does exactly the same thing that the callback does today? Note that we can even use that conditional to block the remaining spapr_del_lmbs code from executing if the LMBs weren't properly cleansed - something that today isn't done. If removing this callback is too problematic or can somehow cause problems that I am unable to foresee, then the alternative would be to either deal with the scanning inside the callback (as it is being done in this patch) or migrate the nr_lmbs information for late retrieval in the callback. I am fine with any alternative, we just need to agree on what makes more sense. Daniel
|
[Prev in Thread] | Current Thread | [Next in Thread] |