lkml.org 
[lkml]   [2021]   [Aug]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [RFC][PATCH] netfs, afs, ceph: Use folios
Date
Matthew Wilcox <willy@infradead.org> 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

\
 
 \ /
  Last update: 2021-08-12 22:48    [W:0.043 / U:0.236 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site