+cc Kajtar, who has kindly smoke tested this series on real hardware and confirmed things are working ostensibly the same as before. On this basis I will be un-RFC'ing this and, if Kajtar can reply to confirm, will add a Tested-by tag to patch 3/3. Again to emphasise - there is some urgency here - as struct page fields that defio relies upon prior to this series are being removed non-optionally. Thanks, Lorenzo On Mon, Jan 13, 2025 at 11:15:45PM +0000, Lorenzo Stoakes wrote: > Right now the only means by which we can write-protect a range using the > reverse mapping is via folio_mkclean(). > > However this is not always the appropriate means of doing so, specifically > in the case of the framebuffer deferred I/O logic (fb_defio enabled by > CONFIG_FB_DEFERRED_IO). There, kernel pages are mapped read-only and > write-protect faults used to batch up I/O operations. > > Each time the deferred work is done, folio_mkclean() is used to mark the > framebuffer page as having had I/O performed on it. However doing so > requires the kernel page (perhaps allocated via vmalloc()) to have its > page->mapping, index fields set so the rmap can find everything that maps > it in order to write-protect. > > This is problematic as firstly, these fields should not be set for > kernel-allocated memory, and secondly these are not folios (it's not user > memory) and page->index, mapping fields are now deprecated and soon to be > removed. > > The implementers cannot be blamed for having used this however, as there is > simply no other way of performing this operation correctly. > > This series fixes this - we provide the mapping_wrprotect_page() function > to allow the reverse mapping to be used to look up mappings from the page > cache object (i.e. its address_space pointer) at a specific offset. > > The fb_defio logic already stores this offset, and can simply be expanded > to keep track of the page cache object, so the change then becomes > straight-forward. > > This series should have no functional change. > > *** REVIEWERS NOTES: *** > > I do not have any hardware that uses fb_defio, so I'm asking for help with > testing this series from those who do :) I have tested the mm side of this, > and done a quick compile smoke test of the fb_defio side but this _very > much_ requires testing on actual hardware to ensure everything behaves as > expected. > > This is based on Andrew's tree [0] in the mm-unstable branch - I was > thinking it'd be best to go through the mm tree (with fb_defio maintainer > approval, of course!) as it relies upon the mm changes to work correctly. > > [0]: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/ > > RFC v2: > * Updated Jaya Kumar's email on cc - the MAINTAINERS section is apparently incorrect. > * Corrected rmap_walk_file() comment to refer to folios as per Matthew. > * Reference folio->mapping rather than folio_mapping(folio) in rmap_walk_file() > as per Matthew. > * Reference folio->index rather than folio_pgoff(folio) in rmap_walk_file() as > per Matthew. > * Renamed rmap_wrprotect_file_page() to mapping_wrprotect_page() as per Matthew. > * Fixed kerneldoc and moved to implementation as per Matthew. > * Updated mapping_wrprotect_page() to take a struct page pointer as per David. > * Removed folio lock when invoking mapping_wrprotect_page() in > fb_deferred_io_work() as per Matthew. > * Removed compound_nr() in fb_deferred_io_work() as per Matthew. > > RFC v1: > https://lore.kernel.org/all/1e452b5b65f15a9a5d0c2ed3f5f812fdd1367603.1736352361.git.lorenzo.stoakes@xxxxxxxxxx/ > > Lorenzo Stoakes (3): > mm: refactor rmap_walk_file() to separate out traversal logic > mm: provide mapping_wrprotect_page() function > fb_defio: do not use deprecated page->mapping, index fields > > drivers/video/fbdev/core/fb_defio.c | 38 ++----- > include/linux/fb.h | 1 + > include/linux/rmap.h | 3 + > mm/rmap.c | 152 +++++++++++++++++++++++----- > 4 files changed, 141 insertions(+), 53 deletions(-) > > -- > 2.48.0
