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
