On Thu, Jun 20, 2019 at 08:47:12AM -0500, Bjorn Helgaas wrote: > On Wed, Jun 19, 2019 at 07:35:21PM -0600, Logan Gunthorpe wrote: > > On 2019-06-19 6:56 p.m., Nicholas Johnson wrote: > > > On Wed, Jun 19, 2019 at 10:45:38AM -0600, Logan Gunthorpe wrote: > > >> On 2019-06-19 8:01 a.m., Nicholas Johnson wrote: > > >>> ----- Forwarded message from Nicholas Johnson <nicholas.johnson-opensource@xxxxxxxxxxxxxx> ----- > > >>> > > >>> Date: Thu, 23 May 2019 06:29:28 +0800 > > >>> From: Nicholas Johnson <nicholas.johnson-opensource@xxxxxxxxxxxxxx> > > >>> To: linux-kernel@xxxxxxxxxxxxxxx > > >>> Cc: linux-pci@xxxxxxxxxxxxxxx, bhelgaas@xxxxxxxxxx, mika.westerberg@xxxxxxxxxxxxxxx, corbet@xxxxxxx, Nicholas Johnson <nicholas.johnson-opensource@xxxxxxxxxxxxxx> > > >>> Subject: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently > > >>> X-Mailer: git-send-email 2.19.1 > > >>> > > >>> Add kernel parameter pci=hpmemprefsize=nn[KMG] to control > > >>> MMIO_PREF size for PCI hotplug bridges. > > >> > > >> Makes sense. > > >> > > >>> Change behaviour of pci=hpmemsize=nn[KMG] to not set MMIO_PREF > > >>> size if hpmempref has been specified, rather than controlling > > >>> both MMIO and MMIO_PREF sizes unconditionally. > > >> > > >> I don't think I like that fact that hpmemsize behaves differently > > >> if hpmempref size is specfied before it. I'd probably suggest > > >> having three parameters: hpmemsize which sets both as it always > > >> has, a pref one and a regular one which each set one of > > >> parameters. > > > > > > It does not matter if hpmempref is specified before or after > > > hpmemsize. I made sure of that. > > > > > Originally, I proposed to depreciate hpiosize, hpmemsize, and > > > introduce: hp_io_size, hp_mmio_size, hp_mmio_pref_size, each > > > controlling its own window exclusively. > > > > > > The patch had the old parameters work with a warning, and if the > > > new ones were specified, they would override the old ones. Then, > > > after a few kernel releases, the old ones could be removed. > > > > Well I don't like that either. No need to depreciate hpmemsize. > > > > > Bjorn insisted that there be nil changes which break the existing > > > parameters, and the solution he requested was to leave hpmemsize > > > to work exactly the same (controlling both MMIO and MMIO_PREF), > > > unless hpmemprefsize is given, which will take control of > > > MMIO_PREF from hpmemsize. > > > > I agree with Bjorn here too but my suggestion is to leave hpmemsize > > alone and have it set both values as it has always done. And add two > > new parameters to set one or the other. Then there's none of this > > "sets one if the other one wasn't set". Also, if I only want to > > change the non-preftechable version then your method leaves no way > > to do so without setting the preftechable version. > > Adding two new parameters sounds like a good idea to me. Yeah, that is basically what I did originally (except I depreciated the old ones rather than keeping them). I did it this way on your direct advice in keeping with minimal lines of diff, minimal disruption, etc. If I were to do this, the number of lines of diff will increase and then I will be fielding complaints that it is too large to sign off. I am already scrambling to make last minute changes before end of release to the other patches and I am not even convinced that that stuff is going to get accepted based on proximity to deadline and how many change requests are flying around. So I am going to have to respectfully decline to do this for now. I need to know earlier in the release cycle if I am going to go back on stuff I have already been asked to do. Cheers.
