| Message ID | 159010426294.1062454.8853083370975871627.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | libnvdimm: Cross-arch compatible namespace alignment | expand |
On Thu, May 21, 2020 at 04:37:43PM -0700, Dan Williams wrote: > Hello stable team, > > These patches have been shipping in mainline since v5.7-rc1 with no > reported issues. They address long standing problems in libnvdimm's > handling of namespace provisioning relative to alignment constraints > including crashes trying to even load the driver on some PowerPC > configurations. > > I did fold one build fix [1] into "libnvdimm/region: Introduce an 'align' > attribute" so as to not convey the bisection breakage to -stable. > > Please consider them for v5.4-stable. They do pass the latest > version of the ndctl unit tests. What about 5.6.y? Any user upgrading from 5.4-stable to 5.6-stable would hit a regression, right? So can we get a series backported to 5.6.y as well? I need that before I can take this series. thanks, greg k-h
On Fri, May 22, 2020 at 01:58:00PM +0200, Greg KH wrote: > On Thu, May 21, 2020 at 04:37:43PM -0700, Dan Williams wrote: > > Hello stable team, > > > > These patches have been shipping in mainline since v5.7-rc1 with no > > reported issues. They address long standing problems in libnvdimm's > > handling of namespace provisioning relative to alignment constraints > > including crashes trying to even load the driver on some PowerPC > > configurations. > > > > I did fold one build fix [1] into "libnvdimm/region: Introduce an 'align' > > attribute" so as to not convey the bisection breakage to -stable. > > > > Please consider them for v5.4-stable. They do pass the latest > > version of the ndctl unit tests. > > What about 5.6.y? Any user upgrading from 5.4-stable to 5.6-stable > would hit a regression, right? > > So can we get a series backported to 5.6.y as well? I need that before > I can take this series. Also, I really don't see the "bug" that this is fixing here. If this didn't work on PowerPC before, it can continue to just "not work" until 5.7, right? What problems with 5.4.y and 5.6.y is this series fixing that used to work before? thanks, greg k-h
On Fri, May 22, 2020 at 5:00 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Fri, May 22, 2020 at 01:58:00PM +0200, Greg KH wrote: > > On Thu, May 21, 2020 at 04:37:43PM -0700, Dan Williams wrote: > > > Hello stable team, > > > > > > These patches have been shipping in mainline since v5.7-rc1 with no > > > reported issues. They address long standing problems in libnvdimm's > > > handling of namespace provisioning relative to alignment constraints > > > including crashes trying to even load the driver on some PowerPC > > > configurations. > > > > > > I did fold one build fix [1] into "libnvdimm/region: Introduce an 'align' > > > attribute" so as to not convey the bisection breakage to -stable. > > > > > > Please consider them for v5.4-stable. They do pass the latest > > > version of the ndctl unit tests. > > > > What about 5.6.y? Any user upgrading from 5.4-stable to 5.6-stable > > would hit a regression, right? > > > > So can we get a series backported to 5.6.y as well? I need that before > > I can take this series. Yes, should be the exact same set, but I will run the regression suite to be sure. > Also, I really don't see the "bug" that this is fixing here. If this > didn't work on PowerPC before, it can continue to just "not work" until > 5.7, right? There's a mix of "never worked" and "used to work" in this set. The PowerPC case is indeed a "never worked", but I highlighted it as it was the simplest to understand. > What problems with 5.4.y and 5.6.y is this series fixing > that used to work before? The "used to work" bug fixed by this set is the fact that the kernel used to force a 128MB (memory hotplug section size) alignment padding on all persistent memory namespaces to enable DAX operation. The support for sub-sections (2MB) dropped forced alignment padding, but unfortunately introduced a regression for the case of trying to create multiple unaligned namespaces. When that bug triggers namespace creation for the region is disabled, iirc, previously that lockout scenario was prevented. Jeff, can you corroborate this? I otherwise agree, if the above never worked then this can all wait for v5.7 upgrades.
Dan Williams <dan.j.williams@intel.com> writes: >> What problems with 5.4.y and 5.6.y is this series fixing >> that used to work before? > > The "used to work" bug fixed by this set is the fact that the kernel > used to force a 128MB (memory hotplug section size) alignment padding > on all persistent memory namespaces to enable DAX operation. The > support for sub-sections (2MB) dropped forced alignment padding, but > unfortunately introduced a regression for the case of trying to create > multiple unaligned namespaces. When that bug triggers namespace > creation for the region is disabled, iirc, previously that lockout > scenario was prevented. > > Jeff, can you corroborate this? So, I don't pretend to remember the exact state of brokenness for each iteration. :) As far as I can recall, though, the issue you describe with a misaligned namespace preventing further namespace creation was present in all kernels up until it was finally fixed. > I otherwise agree, if the above never worked then this can all wait > for v5.7 upgrades. I can test specific kernel versions if that would help out. Cheers, Jeff
On Tue, May 26, 2020 at 1:49 PM Jeff Moyer <jmoyer@redhat.com> wrote: > > Dan Williams <dan.j.williams@intel.com> writes: > > >> What problems with 5.4.y and 5.6.y is this series fixing > >> that used to work before? > > > > The "used to work" bug fixed by this set is the fact that the kernel > > used to force a 128MB (memory hotplug section size) alignment padding > > on all persistent memory namespaces to enable DAX operation. The > > support for sub-sections (2MB) dropped forced alignment padding, but > > unfortunately introduced a regression for the case of trying to create > > multiple unaligned namespaces. When that bug triggers namespace > > creation for the region is disabled, iirc, previously that lockout > > scenario was prevented. > > > > Jeff, can you corroborate this? > > So, I don't pretend to remember the exact state of brokenness for each > iteration. :) As far as I can recall, though, the issue you describe > with a misaligned namespace preventing further namespace creation was > present in all kernels up until it was finally fixed. Well, if it was always there, then there is nothing to fix, and I misremembered that we went backwards. > > I otherwise agree, if the above never worked then this can all wait > > for v5.7 upgrades. > > I can test specific kernel versions if that would help out. Thanks for that offer, but outside of a clear regression I don't think this meets -stable criteria.
On Tue, May 26, 2020 at 01:52:21PM -0700, Dan Williams wrote: > On Tue, May 26, 2020 at 1:49 PM Jeff Moyer <jmoyer@redhat.com> wrote: > > > > Dan Williams <dan.j.williams@intel.com> writes: > > > > >> What problems with 5.4.y and 5.6.y is this series fixing > > >> that used to work before? > > > > > > The "used to work" bug fixed by this set is the fact that the kernel > > > used to force a 128MB (memory hotplug section size) alignment padding > > > on all persistent memory namespaces to enable DAX operation. The > > > support for sub-sections (2MB) dropped forced alignment padding, but > > > unfortunately introduced a regression for the case of trying to create > > > multiple unaligned namespaces. When that bug triggers namespace > > > creation for the region is disabled, iirc, previously that lockout > > > scenario was prevented. > > > > > > Jeff, can you corroborate this? > > > > So, I don't pretend to remember the exact state of brokenness for each > > iteration. :) As far as I can recall, though, the issue you describe > > with a misaligned namespace preventing further namespace creation was > > present in all kernels up until it was finally fixed. > > Well, if it was always there, then there is nothing to fix, and I > misremembered that we went backwards. > > > > I otherwise agree, if the above never worked then this can all wait > > > for v5.7 upgrades. > > > > I can test specific kernel versions if that would help out. > > Thanks for that offer, but outside of a clear regression I don't think > this meets -stable criteria. I agree, I'll drop this series from my pending-queue. thanks, greg k-h
Hello stable team, These patches have been shipping in mainline since v5.7-rc1 with no reported issues. They address long standing problems in libnvdimm's handling of namespace provisioning relative to alignment constraints including crashes trying to even load the driver on some PowerPC configurations. I did fold one build fix [1] into "libnvdimm/region: Introduce an 'align' attribute" so as to not convey the bisection breakage to -stable. Please consider them for v5.4-stable. They do pass the latest version of the ndctl unit tests. [1]: 04ff4863e126 libnvdimm/region: Fix build error --- Original cover letter for the series: Aneesh reports that PowerPC requires 16MiB alignment for the address range passed to devm_memremap_pages(), and Jeff reports that it is possible to create a misaligned namespace which blocks future namespace creation in that region. Both of these issues require namespace alignment to be managed at the region level rather than padding at the namespace level which has been a broken approach to date. Introduce memremap_compat_align() to indicate the hard requirements of an arch's memremap_pages() implementation. Use the maximum known memremap_compat_align() to set the default namespace alignment for libnvdimm. Consult that alignment when allocating free space. Finally, allow the default region alignment to be overridden to maintain the same namespace creation capability as previous kernels (modulo dax operation not being supported with a non-zero start_pad). --- Aneesh Kumar K.V (1): libnvdimm/pfn_dev: Don't clear device memmap area during generic namespace probe Dan Williams (6): mm/memremap_pages: Kill unused __devm_memremap_pages() mm/memremap_pages: Introduce memremap_compat_align() libnvdimm/pfn: Prevent raw mode fallback if pfn-infoblock valid libnvdimm/namespace: Enforce memremap_compat_align() libnvdimm/region: Introduce NDD_LABELING libnvdimm/region: Introduce an 'align' attribute arch/powerpc/Kconfig | 1 arch/powerpc/mm/ioremap.c | 21 +++++ arch/powerpc/platforms/pseries/papr_scm.c | 2 drivers/acpi/nfit/core.c | 4 + drivers/nvdimm/dimm.c | 2 drivers/nvdimm/dimm_devs.c | 95 +++++++++++++++++---- drivers/nvdimm/namespace_devs.c | 28 ++++++ drivers/nvdimm/nd.h | 3 - drivers/nvdimm/pfn.h | 12 +++ drivers/nvdimm/pfn_devs.c | 48 +++++++++-- drivers/nvdimm/region_devs.c | 130 ++++++++++++++++++++++++++--- include/linux/io.h | 2 include/linux/libnvdimm.h | 2 include/linux/memremap.h | 8 ++ include/linux/mmzone.h | 1 lib/Kconfig | 3 + mm/memremap.c | 23 +++++ 17 files changed, 335 insertions(+), 50 deletions(-) base-commit: 1cdaf895c99d319c0007d0b62818cf85fc4b087f