[Intel-gfx] [PATCH 2/5] shmemfs: Use redirty_page_for_writepage()

[Chris Wilson]

On Wed, Mar 05, 2014 at 08:06:23PM -0800, Hugh Dickins wrote:
> 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
> full protocol).
> 
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Hugh Dickins <hughd at google.com>
> 
> So NAK.

No worries, I was only concerned with that it seem to be doing magic
with writeback accounting, which seemed relevant.
 
> 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
> lists.freedesktop.org/archives/intel-gfx.
> 
> 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.

Thank you for your feedback. I am just trying to poke things to
understand how we manage to end up calling oom-killer with an empty
swap. My presumption was that we were failing to trigger writeback. I
shall see how we fare with just invalidating the inode.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre