| Message ID | 1652132902-27109-2-git-send-email-quic_jhugo@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | hyperv compose_msi_msg fixups | expand |
> From: Jeffrey Hugo <quic_jhugo@quicinc.com> > Sent: Monday, May 9, 2022 2:48 PM > Subject: [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in s/ITRE/IRTE. I suppose Wei can help fix this without a v2 :-) > compose_msi_msg() > ... > Currently if compose_msi_msg() is called multiple times, it will free any > previous ITRE allocation, and generate a new allocation. While nothing > prevents this from occurring, it is extranious when Linux could just reuse s/extranious/extraneous > the existing allocation and avoid a bunch of overhead. > > However, when future ITRE allocations operate on blocks of MSIs instead of s/ITRE/IRTE > a single line, freeing the allocation will impact all of the lines. This > could cause an issue where an allocation of N MSIs occurs, then some of > the lines are retargeted, and finally the allocation is freed/reallocated. > The freeing of the allocation removes all of the configuration for the > entire block, which requires all the lines to be retargeted, which might > not happen since some lines might already be unmasked/active. > > Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> Reviewed-by: Dexuan Cui <decui@microsoft.com> Tested-by: Dexuan Cui <decui@microsoft.com>
On 5/9/2022 5:13 PM, Dexuan Cui wrote: >> From: Jeffrey Hugo <quic_jhugo@quicinc.com> >> Sent: Monday, May 9, 2022 2:48 PM >> Subject: [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in > > s/ITRE/IRTE. I suppose Wei can help fix this without a v2 :-) Thanks for the review. I have no problem sending out a V2. Especially since you pointed out my mistakes on both patches. I'll wait a little bit for any additional feedback, and then send out a V2. > >> compose_msi_msg() >> ... >> Currently if compose_msi_msg() is called multiple times, it will free any >> previous ITRE allocation, and generate a new allocation. While nothing >> prevents this from occurring, it is extranious when Linux could just reuse > > s/extranious/extraneous > >> the existing allocation and avoid a bunch of overhead. >> >> However, when future ITRE allocations operate on blocks of MSIs instead of > > s/ITRE/IRTE > >> a single line, freeing the allocation will impact all of the lines. This >> could cause an issue where an allocation of N MSIs occurs, then some of >> the lines are retargeted, and finally the allocation is freed/reallocated. >> The freeing of the allocation removes all of the configuration for the >> entire block, which requires all the lines to be retargeted, which might >> not happen since some lines might already be unmasked/active. >> >> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> > > Reviewed-by: Dexuan Cui <decui@microsoft.com> > Tested-by: Dexuan Cui <decui@microsoft.com>
On Mon, May 09, 2022 at 08:29:19PM -0600, Jeffrey Hugo wrote: > On 5/9/2022 5:13 PM, Dexuan Cui wrote: > > > From: Jeffrey Hugo <quic_jhugo@quicinc.com> > > > Sent: Monday, May 9, 2022 2:48 PM > > > Subject: [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in > > > > s/ITRE/IRTE. I suppose Wei can help fix this without a v2 :-) > > Thanks for the review. > > I have no problem sending out a V2. Especially since you pointed out my > mistakes on both patches. I'll wait a little bit for any additional > feedback, and then send out a V2. Yes please send out v2. Thanks, Wei.
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c index cf2fe575..5e2e637 100644 --- a/drivers/pci/controller/pci-hyperv.c +++ b/drivers/pci/controller/pci-hyperv.c @@ -1707,6 +1707,15 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) u32 size; int ret; + /* Reuse the previous allocation */ + if (data->chip_data) { + int_desc = data->chip_data; + msg->address_hi = int_desc->address >> 32; + msg->address_lo = int_desc->address & 0xffffffff; + msg->data = int_desc->data; + return; + } + pdev = msi_desc_to_pci_dev(irq_data_get_msi_desc(data)); dest = irq_data_get_effective_affinity_mask(data); pbus = pdev->bus; @@ -1716,13 +1725,6 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) if (!hpdev) goto return_null_message; - /* Free any previous message that might have already been composed. */ - if (data->chip_data) { - int_desc = data->chip_data; - data->chip_data = NULL; - hv_int_desc_free(hpdev, int_desc); - } - int_desc = kzalloc(sizeof(*int_desc), GFP_ATOMIC); if (!int_desc) goto drop_reference;
Currently if compose_msi_msg() is called multiple times, it will free any previous ITRE allocation, and generate a new allocation. While nothing prevents this from occurring, it is extranious when Linux could just reuse the existing allocation and avoid a bunch of overhead. However, when future ITRE allocations operate on blocks of MSIs instead of a single line, freeing the allocation will impact all of the lines. This could cause an issue where an allocation of N MSIs occurs, then some of the lines are retargeted, and finally the allocation is freed/reallocated. The freeing of the allocation removes all of the configuration for the entire block, which requires all the lines to be retargeted, which might not happen since some lines might already be unmasked/active. Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> --- drivers/pci/controller/pci-hyperv.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)