• Brian Foster's avatar
    iomap: clean up writeback state logic on writepage error · 50e7d6c7
    Brian Foster authored
    The iomap writepage error handling logic is a mash of old and
    slightly broken XFS writepage logic. When keepwrite writeback state
    tracking was introduced in XFS in commit 0d085a52 ("xfs: ensure
    WB_SYNC_ALL writeback handles partial pages correctly"), XFS had an
    additional cluster writeback context that scanned ahead of
    ->writepage() to process dirty pages over the current ->writepage()
    extent mapping. This context expected a dirty page and required
    retention of the TOWRITE tag on partial page processing so the
    higher level writeback context would revisit the page (in contrast
    to ->writepage(), which passes a page with the dirty bit already
    cleared).
    
    The cluster writeback mechanism was eventually removed and some of
    the error handling logic folded into the primary writeback path in
    commit 150d5be0 ("xfs: remove xfs_cancel_ioend"). This patch
    accidentally conflated the two contexts by using the keepwrite logic
    in ->writepage() without accounting for the fact that the page is
    not dirty. Further, the keepwrite logic has no practical effect on
    the core ->writepage() caller (write_cache_pages()) because it never
    revisits a page in the current function invocation.
    
    Technically, the page should be redirtied for the keepwrite logic to
    have any effect. Otherwise, write_cache_pages() may find the tagged
    page but will skip it since it is clean. Even if the page was
    redirtied, however, there is still no practical effect to keepwrite
    since write_cache_pages() does not wrap around within a single
    invocation of the function. Therefore, the dirty page would simply
    end up retagged on the next writeback sequence over the associated
    range.
    
    All that being said, none of this really matters because redirtying
    a partially processed page introduces a potential infinite redirty
    -> writeback failure loop that deviates from the current design
    principle of clearing the dirty state on writepage failure to avoid
    building up too much dirty, unreclaimable memory on the system.
    Therefore, drop the spurious keepwrite usage and dirty state
    clearing logic from iomap_writepage_map(), treat the partially
    processed page the same as a fully processed page, and let the
    imminent ioend failure clean up the writeback state.
    Signed-off-by: default avatarBrian Foster <bfoster@redhat.com>
    Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
    Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
    50e7d6c7
buffered-io.c 42.2 KB