Re: [PATCH 07/19] mm/filemap: Use head pages in generic_file_buffered_read

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

 



On Thu, Oct 29, 2020 at 07:33:53PM +0000, Matthew Wilcox (Oracle) wrote:
> Add mapping_get_read_heads() which returns the head pages which
> represent a contiguous array of bytes in the file.  It also stops
> when encountering a page marked as Readahead or !Uptodate (but
> does return that page) so it can be handled appropriately by
> gfbr_get_pages().

I don't like the _heads naming - this is like find_get_pages_contig() except it
returns compound pages instead of single pages, and the naming should reflect
that. And, working with compound pages should be the default in the future -
code working with individual pages should be considered a code smell in the
future, i.e. find_get_pages() et. all. should be considered deprecated.

We have the same issue in the block layer with multi page bvecs where the naming
really should be better; the more complicated name should be for the unusual
case, which is not what we have now.

Also - you're implementing new core functionality with a pecularity of the read
path, the two probably be split out. Perhaps find_get_cpages (compound_pages)
that can also be passed a stop condition.

Not all of this is necessarily for this patch, but perhaps a comment indicating
where we want to go in the future would be helpful.

> -			if (writably_mapped)
> -				flush_dcache_page(pages[i]);
> +			if (writably_mapped) {
> +				int j;
> +
> +				for (j = 0; j < thp_nr_pages(page); j++)
> +					flush_dcache_page(page + j);
> +			}

this should be a new helper



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

  Powered by Linux