Re: [v3 PATCH 2/5] mm: filemap: check if THP has hwpoisoned subpage for PMD page fault

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 12, 2021 at 08:27:06PM -0700, Yang Shi wrote:
> > But this also reminded me that shouldn't we be with the page lock already
> > during the process of "setting hwpoison-subpage bit, split thp, clear
> > hwpoison-subpage bit"?  If it's only the small window that needs protection,
> > while when looking up the shmem pagecache we always need to take the page lock
> > too, then it seems already safe even without the extra bit?  Hmm?
> 
> I don't quite get your point. Do you mean memory_failure()? If so the
> answer is no, outside the page lock. And the window may be indefinite
> since file THP doesn't get split before this series and the split may
> fail even after this series.

What I meant is that we could extend the page_lock in try_to_split_thp_page()
to cover setting hwpoison-subpage too (and it of course covers the clearing if
thp split succeeded, as that's part of the split process).  But yeah it's a
good point that the split may fail, so the extra bit seems still necessary.

Maybe that'll be something worth mentioning in the commit message too?  The
commit message described very well on the overhead of looping over 512 pages,
however the reader can easily overlook the real reason for needing this bit -
IMHO it's really for the thp split failure case, as we could also mention that
if thp split won't fail, page lock should be suffice (imho).  We could also
mention about why soft offline does not need that extra bit, which seems not
obvious as well, so imho good material for commit messages.

Sorry to have asked for a lot of commit message changes; I hope they make sense.

Thanks,

-- 
Peter Xu




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux