On Thu, Oct 16, 2025 at 10:18:17PM +0200, David Hildenbrand wrote:
On 15.10.25 20:58, Lisa Wang wrote:
The .error_remove_folio a_ops is used by different filesystems to handle folio truncation upon discovery of a memory failure in the memory associated with the given folio. [...snip...]
- /*
* The shmem page, or any page with MF_DELAYED error handling, is kept in* page cache instead of truncating, so is expected to have an extra* refcount after error-handling.*/- extra_pins = shmem_mapping(mapping) || ret == MF_DELAYED;
Hello David,
Thank you for reviewing these patches!
Well, to do it cleanly shouldn't we let shmem_error_remove_folio() also return MF_DELAYED and remove this shmem special case?
I agree shmem_error_remove_folio() should probably also return MF_DELAYED. MF_DELAYED sounds right because shmem does not truncate, and hence it should not call filemap_release_folio() to release fs-specific metadata on a folio.
There's no bug now in memory failure handling for shmem calling filemap_release_folio(), because
shmem does not have folio->private => filemap_release_folio() is a no-op anyway => filemap_release_folio() returns true => truncate_error_folio() returns MF_RECOVERED => truncate_error_folio()'s caller cleans MF_RECOVERED up to eventually return 0.
Or is there a good reason shmem_mapping() wants to return 0 -- and maybe guest_memfd would also wan to do that?
The tradeoff is if I change shmem_error_remove_folio()'s return, mf_stats will be changed. I'd be happy to update shmem_error_remove_folio() to return MF_DELAYED as well, but is it okay that the userspace-visible behavior in the form of statistics will change?
Just reading the code here the inconsistency is unclear.
Another option is to add kvm_gmem_mapping() like shmem_mapping(). I did not do it because KVM is a module, so we'd need extra steps to check of KVM is loaded in memory, and that's a little more complicated. Also, kvm_gmem_error_folio() already returns MF_DELAYED, which seems to be the right thing to return.
-- Cheers
David / dhildenb
Lisa