Re: disable large folios for shmem file used by xfs xfile
On Wed, Jan 10, 2024 at 4:35 PM Matthew Wilcox wrote: > > On Wed, Jan 10, 2024 at 05:28:22PM +0200, Joonas Lahtinen wrote: > > Quoting Joonas Lahtinen (2024-01-10 17:20:24) > > > However we specifically pass "huge=within_size" to vfs_kern_mount when > > > creating a private mount of tmpfs for the purpose of i915 created > > > allocations. > > > > > > Older hardware also had some address hashing bugs where 2M aligned > > > memory caused a lot of collisions in TLB so we don't enable it always. > > > > > > You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function > > > i915_gemfs_init for details and references. > > > > > > So in short, functionality wise we should be fine either default > > > for using 2M pages or not. If they become the default, we'd probably > > > want an option that would still be able to prevent them for performance > > > regression reasons on older hardware. > > > > To maybe write out my concern better: > > > > Is there plan to enable huge pages by default in shmem? > > Not in the next kernel release, but eventually the plan is to allow > arbitrary order folios to be used in shmem. So you could ask it to create > a 256kB folio for you, if that's the right size to manage memory in. > > How shmem and its various users go about choosing the right size is not > quite clear to me yet. Perhaps somebody else will do it before I get > to it; I have a lot of different sub-projects to work on at the moment, > and shmem isn't blocking any of them. And I have a sneaking suspicion > that more work is needed in the swap code to deal with arbitrary order > folios, so that's another reason for me to delay looking at this ;-) I have sent large folios support for shmem for the write and fallocate path some releases ago. The main problem I was facing was a current upstream problem with huge pages when seeking holes/data (fstests generic/285 and generic/436). The strategy suggested was to use large folios in an opportunistic way based on the file size. This hit the same problem we currently have with huge pages and I considered that a regression. We have made some progress to fix seeking in huge pages upstream but is not yet finished. I can send the patches tomorrow for further discussion. >
Re: disable large folios for shmem file used by xfs xfile
On Wed, Jan 10, 2024 at 12:37:18PM +, Matthew Wilcox wrote: > On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote: > > Hi all, > > > > Darrick reported that the fairly new XFS xfile code blows up when force > > enabling large folio for shmem. This series fixes this quickly by disabling > > large folios for this particular shmem file for now until it can be fixed > > properly, which will be a lot more invasive. > > > > I've added most of you to the CC list as I suspect most other users of > > shmem_file_setup and friends will have similar issues. > > The graphics users _want_ to use large folios. I'm pretty sure they've > been tested with this. It's just XFS that didn't know about this > feature of shmem. At least sgx has all kinds of PAGE_SIZE assumptions. I haven't audited (and am probably not qualified to) audit that code, so I wanted to send a headsup to everyone.
Re: disable large folios for shmem file used by xfs xfile
On Wed, Jan 10, 2024 at 07:38:43AM -0800, Andrew Morton wrote: > I assume that kernels which contain 137db333b29186 ("xfs: teach xfile > to pass back direct-map pages to caller") want this, so a Fixes: that > and a cc:stable are appropriate? I think it needs to back all the way back to 3934e8ebb7c ("xfs: create a big array data structure") as that only clears the page sized chunk of a new allocation and could lead to corruption / or information leaks.
Re: disable large folios for shmem file used by xfs xfile
On Wed, 10 Jan 2024 10:21:07 +0100 Christoph Hellwig wrote: > Hi all, > > Darrick reported that the fairly new XFS xfile code blows up when force > enabling large folio for shmem. This series fixes this quickly by disabling > large folios for this particular shmem file for now until it can be fixed > properly, which will be a lot more invasive. > Patches seems reasonable as a short-term only-affects-xfs thing. I assume that kernels which contain 137db333b29186 ("xfs: teach xfile to pass back direct-map pages to caller") want this, so a Fixes: that and a cc:stable are appropriate?
Re: disable large folios for shmem file used by xfs xfile
On Wed, Jan 10, 2024 at 05:28:22PM +0200, Joonas Lahtinen wrote: > Quoting Joonas Lahtinen (2024-01-10 17:20:24) > > However we specifically pass "huge=within_size" to vfs_kern_mount when > > creating a private mount of tmpfs for the purpose of i915 created > > allocations. > > > > Older hardware also had some address hashing bugs where 2M aligned > > memory caused a lot of collisions in TLB so we don't enable it always. > > > > You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function > > i915_gemfs_init for details and references. > > > > So in short, functionality wise we should be fine either default > > for using 2M pages or not. If they become the default, we'd probably > > want an option that would still be able to prevent them for performance > > regression reasons on older hardware. > > To maybe write out my concern better: > > Is there plan to enable huge pages by default in shmem? Not in the next kernel release, but eventually the plan is to allow arbitrary order folios to be used in shmem. So you could ask it to create a 256kB folio for you, if that's the right size to manage memory in. How shmem and its various users go about choosing the right size is not quite clear to me yet. Perhaps somebody else will do it before I get to it; I have a lot of different sub-projects to work on at the moment, and shmem isn't blocking any of them. And I have a sneaking suspicion that more work is needed in the swap code to deal with arbitrary order folios, so that's another reason for me to delay looking at this ;-)
Re: disable large folios for shmem file used by xfs xfile
Quoting Joonas Lahtinen (2024-01-10 17:20:24) > Quoting Matthew Wilcox (2024-01-10 14:37:18) > > On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote: > > > Hi all, > > > > > > Darrick reported that the fairly new XFS xfile code blows up when force > > > enabling large folio for shmem. This series fixes this quickly by > > > disabling > > > large folios for this particular shmem file for now until it can be fixed > > > properly, which will be a lot more invasive. > > > > > > I've added most of you to the CC list as I suspect most other users of > > > shmem_file_setup and friends will have similar issues. > > > > The graphics users _want_ to use large folios. I'm pretty sure they've > > been tested with this. > > Correct. We've done quite a bit of optimization in userspace and > enabling in kernel to take advantage of page sizes of 2M and beyond. > > However we specifically pass "huge=within_size" to vfs_kern_mount when > creating a private mount of tmpfs for the purpose of i915 created > allocations. > > Older hardware also had some address hashing bugs where 2M aligned > memory caused a lot of collisions in TLB so we don't enable it always. > > You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function > i915_gemfs_init for details and references. > > So in short, functionality wise we should be fine either default > for using 2M pages or not. If they become the default, we'd probably > want an option that would still be able to prevent them for performance > regression reasons on older hardware. To maybe write out my concern better: Is there plan to enable huge pages by default in shmem? If not I guess we should be pretty good with the way current code is, force enabling them just might bring out some performance, so we might want to add a warning for that. If there is, then we'll probably want to in sync with those default changes apply a similar call to block them on older HW. Regards, Joonas > > Regards, Joonas > > > It's just XFS that didn't know about this > > feature of shmem.
Re: disable large folios for shmem file used by xfs xfile
Quoting Matthew Wilcox (2024-01-10 14:37:18) > On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote: > > Hi all, > > > > Darrick reported that the fairly new XFS xfile code blows up when force > > enabling large folio for shmem. This series fixes this quickly by disabling > > large folios for this particular shmem file for now until it can be fixed > > properly, which will be a lot more invasive. > > > > I've added most of you to the CC list as I suspect most other users of > > shmem_file_setup and friends will have similar issues. > > The graphics users _want_ to use large folios. I'm pretty sure they've > been tested with this. Correct. We've done quite a bit of optimization in userspace and enabling in kernel to take advantage of page sizes of 2M and beyond. However we specifically pass "huge=within_size" to vfs_kern_mount when creating a private mount of tmpfs for the purpose of i915 created allocations. Older hardware also had some address hashing bugs where 2M aligned memory caused a lot of collisions in TLB so we don't enable it always. You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function i915_gemfs_init for details and references. So in short, functionality wise we should be fine either default for using 2M pages or not. If they become the default, we'd probably want an option that would still be able to prevent them for performance regression reasons on older hardware. Regards, Joonas > It's just XFS that didn't know about this > feature of shmem.
Re: disable large folios for shmem file used by xfs xfile
On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote: > Hi all, > > Darrick reported that the fairly new XFS xfile code blows up when force > enabling large folio for shmem. This series fixes this quickly by disabling > large folios for this particular shmem file for now until it can be fixed > properly, which will be a lot more invasive. > > I've added most of you to the CC list as I suspect most other users of > shmem_file_setup and friends will have similar issues. The graphics users _want_ to use large folios. I'm pretty sure they've been tested with this. It's just XFS that didn't know about this feature of shmem.
disable large folios for shmem file used by xfs xfile
Hi all, Darrick reported that the fairly new XFS xfile code blows up when force enabling large folio for shmem. This series fixes this quickly by disabling large folios for this particular shmem file for now until it can be fixed properly, which will be a lot more invasive. I've added most of you to the CC list as I suspect most other users of shmem_file_setup and friends will have similar issues.