On Mon 24-08-20 18:36:39, Matthew Wilcox wrote: > On Mon, Aug 24, 2020 at 06:16:20PM +0200, Jan Kara wrote: > > On Wed 19-08-20 16:05:54, Matthew Wilcox (Oracle) wrote: > > > All callers of find_get_entries() use a pvec, so pass it directly > > > instead of manipulating it in the caller. > > > > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx> > > > > Rather than passing pvec to find_get_entries() and then making everybody > > use it, won't it more consistent WRT the naming to make everybody use > > pagevec_lookup_entries() (which is trivial at this point in the series) and > > then rename find_get_entries() to pagevec_lookup_entries()? I.e., I'd prefer > > if the final function was called pagevec_lookup_entries() because that is > > IMO more consistent with how other functions are named in this area... > > It seemed more consistent to me to have everybody using > find_get_entries(). To me the pagevec functions: > > 1. Are in mm/swap.c (not really sure why) Historical :). AFAIK pagevec abstraction was first created to make swapping out and reclaim of pages more effective. It has grown a bit since then... > 2. Take pvec as the first argument, not the last Well, yes, I'd keep the argument order to match original pagevec_lookup_entries(). > 3. Wrap a find_* function > > Whereas the find_* functions: > > 1. Are in mm/filemap.c > 2. Take mapping as the first argument > 3. Manipulate the XArray directly Agreed. > We already have functions in filemap which take a pagevec, eg > page_cache_delete_batch() and delete_from_page_cache_batch(). Right but those are really pretty internal helper functions so I don't think they form or strong precedence. > So if we're going to merge the two functions, it seems more natural to > have it in filemap.c and called find_get_entries(), but I'm definitely > open to persuasion on this! I agree that having non-trivial xarray code in mm/swap.c isn't attractive either. Dunno, I dislike the inconsistency between find_get_pages() and find_get_entries() you create but they aren't completely consistent anyway so I can live with that. Or we can just leave the pagevec_lookup_entries() wrapper and the API will stay consistent... Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
