Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 15 Nov 2020 10:22:10 -0700
From:      Scott Long <scottl@samsco.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        Alexander Motin <mav@freebsd.org>, "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org>
Subject:   Re: MAXPHYS bump for FreeBSD 13
Message-ID:  <0255CCBE-35C3-499F-80B6-44B57679C403@samsco.org>
In-Reply-To: <46AB39AF-B09A-4694-AA86-30EC82A1EB9E@samsco.org>
References:  <aac44f21-3a6d-c697-bb52-1a669b11aa3b@FreeBSD.org> <X7Aj0a6ZtIQfBscC@kib.kiev.ua> <ae6861cf-108f-19f7-9525-c89cac850164@FreeBSD.org> <X7EJ8K0Jaw6/dkjl@kib.kiev.ua> <46AB39AF-B09A-4694-AA86-30EC82A1EB9E@samsco.org>

next in thread | previous in thread | raw e-mail | index | archive | help


> On Nov 15, 2020, at 10:11 AM, Scott Long <scottl@samsco.org> wrote:
>=20
>=20
>=20
>> On Nov 15, 2020, at 3:58 AM, Konstantin Belousov =
<kostikbel@gmail.com> wrote:
>>=20
>> On Sat, Nov 14, 2020 at 06:40:44PM -0500, Alexander Motin wrote:
>>> On 14.11.2020 13:37, Konstantin Belousov wrote:
>>>> On Sat, Nov 14, 2020 at 10:01:05AM -0500, Alexander Motin wrote:
>>>> 4. My larger concern is, in fact, cam and drivers.
>>>=20
>>> I am actually the least concerned about this part.  I've already
>>> reviewed/cleaned it once, and can do again if needed.  We have some
>>> drivers unaware about MAXPHYS, and they should safely be limited to
>>> DFLTPHYS, the others should properly adapt.  And if you like to make
>>> MAXPHYS tunable -- I'd be happy to take this part.
>> Well, I looked at ahci(4) as a first driver, and it is already =
problematic.
>> It sizes internal structures, which are really some hardware command
>> structures, based on MAXPHYS.  Same for siis(4), but this seems to be =
even
>> worse due to +1.
>>=20
>=20
> The MAXPHYS use for AHCI is related to the maximum size of a PRD =
needed
> for a command.  AHCI specifies that you can have up to 65536 PRD =
entries,
> which translates into 2^16 + 2^12 =3D 256MB maximum I/O size.  =
However, the
> AHCI driver seems inconsistent already in that it defines AHCI_PRD_MAX=20=

> (via an indirection through AHCI_PRD_MASK) to be 2^22, or 4MB.  This =
is
> a pretty reasonable max default for AHCI, and I don=E2=80=99t think we =
need to jump
> through hoops to make it dynamic or make it larger for the future.  My
> recommendation is to abandon the partial changes you have for AHCI and
> use AHCI_PRD_MAX in place of MAXPHYS for the structure sizing and for
> the cpi->maxio attribute.
>=20
> I have less of an opinion on the SIIS driver since that hardware is =
pretty
> low-performance and mostly obsolete at this point.  I guess we should =
do the
> same as AHCI, but I don=E2=80=99t know if there are details about SIIS =
that diverge
> in significant ways.


I forgot to add that I=E2=80=99m ok with a static sizing of 4MB for the =
PRD array because
an AHCI device is only ever going to have 32 outstanding commands.  The
memory increase here is going to be pretty small compared to NVME or
MPR/MPS that has the potential for a much deeper queue, and the size of =
a
PRD element is much smaller than a vm_page_t element from the buf =
discussion.
Even a system with a lot of AHCI devices will still not grow the memory =
footprint
that much.

Scott




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?0255CCBE-35C3-499F-80B6-44B57679C403>