On Wed, Oct 20, 2021 at 05:24:09PM -0700, Yang Shi wrote: > On Wed, Oct 20, 2021 at 4:51 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > > > On Wed, Oct 20, 2021 at 04:38:49PM -0700, Yang Shi wrote: > > > > However, it still doesn't make too much sense to have thp_size passed > > > > to do_invalidatepage(), then have PAGE_SIZE hardcoded in a BUG > > > > assertion IMHO. So it seems this patch is still useful because > > > > block_invalidatepage() is called by a few filesystems as well, for > > > > example, ext4. Or I'm wondering whether we should call > > > > do_invalidatepage() for each subpage of THP in truncate_cleanup_page() > > > > since private is for each subpage IIUC. > > > > > > Seems no interest? > > > > No. I have changes in this area as part of the folio patchset (where > > we end up converting this to invalidate_folio). I'm not really > > interested in doing anything before that, since this shouldn't be > > reachable today. > > Understood. But this is definitely reachable unless Hugh's patch > (skipping non-regular file) is applied. Right. That's the bug that needs to be fixed. Seeing THPs here is a symptom. Getting rid of the error just makes the problem silent. > > > Anyway the more I was staring at the code the more I thought calling > > > do_invalidatepage() for each subpage made more sense. So, something > > > like the below makes sense? > > > > Definitely not. We want to invalidate the entire folio at once. > > I didn't look at the folio patch (on each individual patch level), but > I'm supposed it still needs to invalidate buffer for each subpage, > right? No. Buffers are tracked for the entire folio, not on each subpage. Actually, the filesystem people currently believe that the O(n^2) nature of buffer-head handling mean that it's a bad idea to create multi-page folios for bufferhead based filesystems (which includes block devices), and the correct path forward is to migrate away from buffer-heads. That may change, but it's the current plan.
