Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> > (*) Can page_endio() be split into two separate functions, one for read
> > and one for write? If seems a waste of time to conditionally switch
> > between two different branches.
>
> At this point I'm thinking ...
>
> static inline void folio_end_read(struct folio *folio, int err)
> {
> if (!err)
> folio_set_uptodate(folio);
> folio_unlock(folio);
> }
>
> Clearly the page isn't uptodate at this point, or ->readpage wouldn't've
> been called. So there's no need to clear it. And PageError is
> completely useless.
Seems reasonable.
> > - *_page = page;
> > + *_page = &folio->page;
>
> Can't do anything about this one; the write_begin API needs to be fixed.
That's fine. I expected things like this at this stage.
> > @@ -174,40 +175,32 @@ static void afs_kill_pages(struct address_space *mapping,
> [...]
> > + folio_clear_uptodate(folio);
> > + folio_end_writeback(folio);
> > + folio_lock(folio);
> > + generic_error_remove_page(mapping, &folio->page);
> > + folio_unlock(folio);
> > + folio_put(folio);
>
> This one I'm entirely missing. It's awkward. I'll work on it.
afs_kill_pages() is just a utility to end writeback, clear uptodate and do
generic_error_remove_page() over a range of pages and afs_redirty_pages() is a
utility that to end writeback and redirty a range of pages - hence why I was
thinking it might make sense to put them into common code.
> > - index += thp_nr_pages(page);
> > - if (!pagevec_add(&pvec, page))
> > + index += folio_nr_pages(folio);
> > + if (!pagevec_add(&pvec, &folio->page))
>
> Pagevecs are also awkward. I haven't quite figured out how to
> transition them to folios.
Maybe provide pagevec_add_folio(struct pagevec *, struct folio *)?
> > zero_out:
> > - zero_user_segments(page, 0, offset, offset + len, thp_size(page));
> > + zero_user_segments(&folio->page, 0, offset, offset + len, folio_size(folio));
>
> Yeah, that's ugly.
Maybe:
folio_clear_around(folio, keep_from, keep_to);
clearing the bits of the folio outside the specified section?
David
