Re: [PATCH v2] PCI hotplug Eq v2
On Mon, 2018-09-17 at 15:53 -0500, Bjorn Helgaas wrote: > On Thu, Aug 30, 2018 at 04:11:59PM -0600, Jon Derrick wrote: > > Hi Bjorn, > > > > Sorry for the delay on this one and pushing it after RC1. > > Feel free to queue it up for 4.20 if it looks fine. > > > > I've added comments to the git log and source explaining why > > calculate_iosize was left unchanged. Basically I could not > > synthesize a condition where it would have affected the topology. > > In other words, the only reason you didn't change the > calculate_iosize() path was because you couldn't test it? > I did unsuccessfully try to synthesize it in hardware and qemu. The firmwares didn't provide the neccessary topology to hit the flexible IO provisioning conditions > I appreciate your desire to avoid untested changes, but I think it's > very important to preserve and even improve the symmetry between > calculate_memsize() and calculate_iosize(). For example, it's not > obvious why the order is different here: > > calculate_iosize(): > size = ALIGN(size + size1, align); > if (size < old_size) > size = old_size; > I agree this part didn't make that much sense to me, which was another reason I left it as-is. Looking at it again, I think its a harmless calculation that bounds IO size tightly, but could also be reordered as below to provide for the additional IO (assuming this code ever runs). > calculate_memsize(): > if (size < old_size) > size = old_size; > size = ALIGN(size + size1, align); > > So I don't want to diverge them further unless there's a real > functional reason why we need to handle I/O port space differently > than MMIO space. > > You've tested the MMIO path, and I'm willing to take the risk of > doing the same thing in the I/O port path. > > Bjorn Great! I'll follow-up with a patch as soon as I can Jon smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH v2] PCI hotplug Eq v2
On Mon, 2018-09-17 at 15:53 -0500, Bjorn Helgaas wrote: > On Thu, Aug 30, 2018 at 04:11:59PM -0600, Jon Derrick wrote: > > Hi Bjorn, > > > > Sorry for the delay on this one and pushing it after RC1. > > Feel free to queue it up for 4.20 if it looks fine. > > > > I've added comments to the git log and source explaining why > > calculate_iosize was left unchanged. Basically I could not > > synthesize a condition where it would have affected the topology. > > In other words, the only reason you didn't change the > calculate_iosize() path was because you couldn't test it? > I did unsuccessfully try to synthesize it in hardware and qemu. The firmwares didn't provide the neccessary topology to hit the flexible IO provisioning conditions > I appreciate your desire to avoid untested changes, but I think it's > very important to preserve and even improve the symmetry between > calculate_memsize() and calculate_iosize(). For example, it's not > obvious why the order is different here: > > calculate_iosize(): > size = ALIGN(size + size1, align); > if (size < old_size) > size = old_size; > I agree this part didn't make that much sense to me, which was another reason I left it as-is. Looking at it again, I think its a harmless calculation that bounds IO size tightly, but could also be reordered as below to provide for the additional IO (assuming this code ever runs). > calculate_memsize(): > if (size < old_size) > size = old_size; > size = ALIGN(size + size1, align); > > So I don't want to diverge them further unless there's a real > functional reason why we need to handle I/O port space differently > than MMIO space. > > You've tested the MMIO path, and I'm willing to take the risk of > doing the same thing in the I/O port path. > > Bjorn Great! I'll follow-up with a patch as soon as I can Jon smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH v2] PCI hotplug Eq v2
On Thu, Aug 30, 2018 at 04:11:59PM -0600, Jon Derrick wrote: > Hi Bjorn, > > Sorry for the delay on this one and pushing it after RC1. > Feel free to queue it up for 4.20 if it looks fine. > > I've added comments to the git log and source explaining why > calculate_iosize was left unchanged. Basically I could not > synthesize a condition where it would have affected the topology. In other words, the only reason you didn't change the calculate_iosize() path was because you couldn't test it? I appreciate your desire to avoid untested changes, but I think it's very important to preserve and even improve the symmetry between calculate_memsize() and calculate_iosize(). For example, it's not obvious why the order is different here: calculate_iosize(): size = ALIGN(size + size1, align); if (size < old_size) size = old_size; calculate_memsize(): if (size < old_size) size = old_size; size = ALIGN(size + size1, align); So I don't want to diverge them further unless there's a real functional reason why we need to handle I/O port space differently than MMIO space. You've tested the MMIO path, and I'm willing to take the risk of doing the same thing in the I/O port path. Bjorn
Re: [PATCH v2] PCI hotplug Eq v2
On Thu, Aug 30, 2018 at 04:11:59PM -0600, Jon Derrick wrote: > Hi Bjorn, > > Sorry for the delay on this one and pushing it after RC1. > Feel free to queue it up for 4.20 if it looks fine. > > I've added comments to the git log and source explaining why > calculate_iosize was left unchanged. Basically I could not > synthesize a condition where it would have affected the topology. In other words, the only reason you didn't change the calculate_iosize() path was because you couldn't test it? I appreciate your desire to avoid untested changes, but I think it's very important to preserve and even improve the symmetry between calculate_memsize() and calculate_iosize(). For example, it's not obvious why the order is different here: calculate_iosize(): size = ALIGN(size + size1, align); if (size < old_size) size = old_size; calculate_memsize(): if (size < old_size) size = old_size; size = ALIGN(size + size1, align); So I don't want to diverge them further unless there's a real functional reason why we need to handle I/O port space differently than MMIO space. You've tested the MMIO path, and I'm willing to take the risk of doing the same thing in the I/O port path. Bjorn
[PATCH v2] PCI hotplug Eq v2
Hi Bjorn, Sorry for the delay on this one and pushing it after RC1. Feel free to queue it up for 4.20 if it looks fine. I've added comments to the git log and source explaining why calculate_iosize was left unchanged. Basically I could not synthesize a condition where it would have affected the topology. v1->v2: Comments Jon Derrick (1): PCI: Equalize hotplug memory for non/occupied slots drivers/pci/setup-bus.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) -- 1.8.3.1
[PATCH v2] PCI hotplug Eq v2
Hi Bjorn, Sorry for the delay on this one and pushing it after RC1. Feel free to queue it up for 4.20 if it looks fine. I've added comments to the git log and source explaining why calculate_iosize was left unchanged. Basically I could not synthesize a condition where it would have affected the topology. v1->v2: Comments Jon Derrick (1): PCI: Equalize hotplug memory for non/occupied slots drivers/pci/setup-bus.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) -- 1.8.3.1
