it's too early to set the radix tree tag here. We should set page's snapshot context and increase the i_wrbuffer_ref first. This is because once the tag is set, writeback thread can find and start flushing the page.
> dout("%p set_page_dirty %p idx %lu -- already dirty\n",
> mapping->host, page, page->index);
> return 0;
> @@ -107,14 +107,7 @@ static int ceph_set_page_dirty(struct page *page)
> snapc, snapc->seq, snapc->num_snaps);
> spin_unlock(&ci->i_ceph_lock);
>
> - /* now adjust page */
> - spin_lock_irq(&mapping->tree_lock);
> if (page->mapping) { /* Race with truncate? */
> - WARN_ON_ONCE(!PageUptodate(page));
> - account_page_dirtied(page, page->mapping);
> - radix_tree_tag_set(&mapping->page_tree,
> - page_index(page), PAGECACHE_TAG_DIRTY);
> -
this code was coped from __set_page_dirty_nobuffers(). I think the reason Sage did this is to handle the race described in __set_page_dirty_nobuffers()'s comment. But I'm wonder if "page->mapping == NULL" can still happen here. Because truncate_inode_page() unmap page from processes's address spaces first, then delete page from page cache.
Regards
Yan, Zheng
> /*
> * Reference snap context in page->private. Also set
> * PagePrivate so that we get invalidatepage callback.
> @@ -126,14 +119,10 @@ static int ceph_set_page_dirty(struct page *page)
> undo = 1;
> }
>
> - spin_unlock_irq(&mapping->tree_lock);
> -
> if (undo)
> /* whoops, we failed to dirty the page */
> ceph_put_wrbuffer_cap_refs(ci, 1, snapc);