[Intel-gfx] [PATCH 2/5] shmemfs: Use redirty_page_for_writepage()
hughd at google.com
Thu Mar 6 05:06:23 CET 2014
On Wed, 5 Mar 2014, Chris Wilson wrote:
> "When we cannot write a page we should use redirty_page_for_writepage()
> instead of plain set_page_dirty(). That tells writeback code we have
> problems, redirties only the page (redirtying buffers is not needed),
> and updates mm accounting of failed page writes."
I didn't locate the origin of that quotation, but it's talking about
the usual filesystem cap_account_dirty/cap_account_writeback protocol.
shmem doesn't participate it that: it only writes out (to swap) under
memory pressure, not for sync, and follows a much simpler path. Using
redirty_page_for_writepage() would lead it into complications, some of
which we prefer to avoid for efficiency, some of which would actually
be wrong (unless/until there's reason to convert shmem over to the
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Hugh Dickins <hughd at google.com>
But you didn't explain why you want to make this change. I presume
it's for the 5/5 which you didn't Cc to me, but I've looked up on
That's a bigger change and worrying: I've not thought through
the consequences of shmem page writeback from generic_writepages()
called from within a slab shrinker: we're used to doing shmem page
writeback from pageout() in mm/vmscan.c and nowhere else.
One thing that would certainly be wrong (liable to deadlock) would
be to do that when shrink_control's gfp_mask does not have __GFP_IO.
But I'm not comfortable with page writeback from this level at all.
The shmem_truncate_range() (you've had for a long time) should be safe,
and the new invalidate_mapping_pages() too: neither of those gets into
I/O, and invalidate_mapping_pages() looks as if it does all that's
necessary for those pages to be put under writeback at the next
shrink_inactive_list() of the lruvec.
Or perhaps there's a gotcha or two, which we can fix up.
But please try to stick to truncation and invalidation.
> mm/shmem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 142b0bc085e1..18aa88eff8e3 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -872,7 +872,7 @@ static int shmem_writepage(struct page *page, struct writeback_control *wbc)
> swapcache_free(swap, NULL);
> - set_page_dirty(page);
> + redirty_page_for_writepage(wbc, page);
> if (wbc->for_reclaim)
> return AOP_WRITEPAGE_ACTIVATE; /* Return with page locked */
More information about the Intel-gfx