[LU-6854] Setting page_writeback on a non-dirty page

• Type: Bug
• Resolution: Fixed
• Priority: Critical
• Fix Version/s: Lustre 2.10.0
• Affects Version/s: Lustre 2.5.3, Lustre 2.8.0
• Severity: 3
• Assignee: Zhenyu Xu
• Reporter: Oleg Drokin

Description

A recent change in the upstream kernel uncovered what I think is a bug in our handling of writeback bit on pages.

If we go by the "sync io" path: vvp_io_commit_write->vvp_page_sync_io->…..->vvp_page_prep_write

then the page is never dirty, vvp_page_prep_write then asserts the page is not dirty and then sets it as writeback:

static int vvp_page_prep_write(const struct lu_env *env,
                               const struct cl_page_slice *slice,
                               struct cl_io *unused)
{
        struct page *vmpage = cl2vm_page(slice);

        LASSERT(PageLocked(vmpage));
        LASSERT(!PageDirty(vmpage));

        set_page_writeback(vmpage);
        vvp_write_pending(cl2ccc(slice->cpl_obj), cl2ccc_page(slice));

        return 0;
}

Now, the problem is, from kernel perspective page writeback means this is a cached page that is just in flight being written to the device, so it must start as dirty in the first place and we violate that assumption.

So now in 4.2.0 there's new cgroup dirty page accounting logic and as part of that set_page_writeback updates a writeback structure hanging off inode (i_wb), but this structure is only initialized when either an inode is dirtied or a page is dirtied and we crash otherwise (that's how I uncovered this).

Since this path I am talking about is a sync write, there are two trains of thoughts possible here, I imagine.
1: This is a sync write, so that's why we do not set dirty bit and we do a sync writeout => we probably don't need to set page_writeback either then.
2: The page is in cache already anyway (that's how we got to it in the first place), so even though we cannot add it to OUR cache, we still need to set it dirty and then it'll become clean once the write completes anyway and we can throw it away out of cache in the same breath too if we want to.

Issue Links

• is duplicated by: LU-8971 sanity test_99a: test failed to respond and timed out