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@xxxxxxxxxxxxxxxxxx> > Cc: Hugh Dickins <hughd@xxxxxxxxxx> So NAK. 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. Hugh > --- > 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) > mutex_unlock(&shmem_swaplist_mutex); > swapcache_free(swap, NULL); > redirty: > - set_page_dirty(page); > + redirty_page_for_writepage(wbc, page); > if (wbc->for_reclaim) > return AOP_WRITEPAGE_ACTIVATE; /* Return with page locked */ > unlock_page(page); > -- > 1.9.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
