Date: Tue, 13 Nov 2018 11:02:25 -0700 From: Scott Long <scottl@samsco.org> To: Harry Schmalzbauer <freebsd@omnilan.de> Cc: scsi@freebsd.org, freebsd-stable <freebsd-stable@freebsd.org> Subject: Re: MSI allocation regression, still to be corrected in HEAD and please MFC before release/12.0 gets branched Message-ID: <05D94EC4-D797-4246-96B5-6DC1494E7EA9@samsco.org> In-Reply-To: <042090da-f73f-ce80-517c-0b1729d3d6e1@omnilan.de> References: <201707300653.v6U6rwLN099096@repo.freebsd.org> <597DA578.6030101@omnilan.de> <597F56A8.1060603@omnilan.de> <D18DFAD4-6E93-4AE2-BE15-EFF4D8ABCB2A@samsco.org> <59804C8C.1020003@omnilan.de> <e7d94e6a-89e8-ffa1-40da-7fb67e6bfc2b@omnilan.de> <78611650-D7A4-4B1D-A254-DB058E1AC1C6@samsco.org> <d99e383d-b09a-f3bd-f1e2-a6a808016347@omnilan.de> <A2AECF9F-2BB1-4FC0-8330-658336A3A4F0@samsco.org> <6e1e5f9f-4ece-dc9d-b059-08d52c9e6965@omnilan.de> <042090da-f73f-ce80-517c-0b1729d3d6e1@omnilan.de>
next in thread | previous in thread | raw e-mail | index | archive | help
> On Nov 12, 2018, at 10:03 AM, Harry Schmalzbauer <freebsd@omnilan.de> =
wrote:
>=20
> Am 11.06.2018 um 20:28 schrieb Harry Schmalzbauer:
>> Am 05.06.2018 um 19:54 schrieb Scott Long:
>> =E2=80=A6
>>>>>> Late in the 11.2 phase, I identified this commit as a regression =
for MSI (non-x) alloctaion.
>>>>>> I have an idea what probably causes the problem here (INTx =
allocation, although MSI (and MSI-x) capability):
>>>>>> disable_msix is not 0 (I need to disable MSI-x because of =
ESXi-passthru=E2=80=A6).
>>>>>>=20
>>>>>> Corresponding lines:
>>>>>> {
>>>>>> device_t dev;
>>>>>> int error, msgs;
>>>>>>=20
>>>>>> dev =3D sc->mps_dev;
>>>>>> error =3D 0;
>>>>>> msgs =3D 0;
>>>>>>=20
>>>>>> if ((sc->disable_msix =3D=3D 0) &&
>>>>>> ((msgs =3D pci_msix_count(dev)) >=3D MPS_MSI_COUNT))
>>>>>> error =3D mps_alloc_msix(sc, MPS_MSI_COUNT);
>>>>>> if ((error !=3D 0) && (sc->disable_msi =3D=3D 0) &&
>>>>>> ((msgs =3D pci_msi_count(dev)) >=3D MPS_MSI_COUNT))
>>>>>> error =3D mps_alloc_msi(sc, MPS_MSI_COUNT);
>>>>>> if (error !=3D 0)
>>>>>> msgs =3D 0;
>>>>>>=20
>>>>>> sc->msi_msgs =3D msgs;
>>>>>> return (error);
>>>>>> }
>>>>>>=20
> =E2=80=A6
>>>>> Hi Harry,
>>>>> You are correct about the bug. Please change the line at the top =
of the function that reads
>>>>> error =3D 0;
>>>>> to
>>>>> error =3D ENXIO;
>>>>> Let me know if that fixes the MSI problem for you.
>>=20
>> =E2=80=A6
>>=20
> =E2=80=A6
>> Index: src/sys/dev/mps/mps_pci.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- sys/dev/mps/mps_pci.c (Revision 334948)
>> +++ sys/dev/mps/mps_pci.c (Arbeitskopie)
>> @@ -244,7 +244,7 @@
>> int error, msgs;
>>=20
>> dev =3D sc->mps_dev;
>> - error =3D 0;
>> + error =3D ENXIO;
>> msgs =3D 0;
>>=20
>> if ((sc->disable_msix =3D=3D 0) &&
>>=20
>=20
> To my understanding, it's obvious that the way =
mps_pci_alloc_interrupts() currently works is unintended.
> This might not affect too many people, but is there a reason not to =
fix it?
>=20
> I already created a coresponding problem report: =
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D229267
> Anything else I should do?
>=20
Hi Harry,
Sorry for ignoring this for so long. I=E2=80=99m going to commit a fix =
today, but it won=E2=80=99t be the same one-line change.
Upon reviewing the code, I=E2=80=99d going to refactor it so it=E2=80=99s =
not so confusing and prone to these kinds of mistakes.
Thank you for the continued reminders to finish this.
Scott
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?05D94EC4-D797-4246-96B5-6DC1494E7EA9>