| Message ID | 20210615162342.1669332-2-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Further set_page_dirty cleanups | expand |
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
On Tue, Jun 15, 2021 at 05:23:37PM +0100, Matthew Wilcox (Oracle) wrote: > Nothing in __set_page_dirty() is specific to buffer_head, so > move it to mm/page-writeback.c. That removes the only caller of > account_page_dirtied() outside of page-writeback.c, so make it static. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > fs/buffer.c | 24 ------------------------ > include/linux/mm.h | 1 - > mm/page-writeback.c | 27 ++++++++++++++++++++++++++- > 3 files changed, 26 insertions(+), 26 deletions(-) > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Tue, Jun 15, 2021 at 05:23:37PM +0100, Matthew Wilcox (Oracle) wrote: > -/* > - * Mark the page dirty, and set it dirty in the page cache, and mark the inode > - * dirty. > - * > - * If warn is true, then emit a warning if the page is not uptodate and has > - * not been truncated. > - * > - * The caller must hold lock_page_memcg(). > - */ Checking against my folio tree, I found a bit of extra documentation that I had added and didn't make it into this submission. Let me know if it's useful and if so I can submit it as a fixup patch: diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 73b937955cc1..2072787d9b44 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -2466,7 +2466,11 @@ void account_page_cleaned(struct page *page, struct addre ss_space *mapping, * If warn is true, then emit a warning if the page is not uptodate and has * not been truncated. * - * The caller must hold lock_page_memcg(). + * The caller must hold lock_page_memcg(). Most callers have the page + * locked. A few have the page blocked from truncation through other + * means (eg zap_page_range() has it mapped and is holding the page table + * lock). This can also be called from mark_buffer_dirty(), which I + * cannot prove is always protected against truncate. */ void __set_page_dirty(struct page *page, struct address_space *mapping, int warn) ... it's a bit "notes to self", so perhaps someone can clean it up. In particular, someone who knows the buffer code better than I do can prove that mark_buffer_dirty() is always protected against truncate.
diff --git a/fs/buffer.c b/fs/buffer.c index 8c02e44f16f2..f5384cff7e0c 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -588,30 +588,6 @@ void mark_buffer_dirty_inode(struct buffer_head *bh, struct inode *inode) } EXPORT_SYMBOL(mark_buffer_dirty_inode); -/* - * Mark the page dirty, and set it dirty in the page cache, and mark the inode - * dirty. - * - * If warn is true, then emit a warning if the page is not uptodate and has - * not been truncated. - * - * The caller must hold lock_page_memcg(). - */ -void __set_page_dirty(struct page *page, struct address_space *mapping, - int warn) -{ - unsigned long flags; - - xa_lock_irqsave(&mapping->i_pages, flags); - if (page->mapping) { /* Race with truncate? */ - WARN_ON_ONCE(warn && !PageUptodate(page)); - account_page_dirtied(page, mapping); - __xa_set_mark(&mapping->i_pages, page_index(page), - PAGECACHE_TAG_DIRTY); - } - xa_unlock_irqrestore(&mapping->i_pages, flags); -} - /* * Add a page to the dirty page list. * diff --git a/include/linux/mm.h b/include/linux/mm.h index d64f8a1284d9..1086b556961a 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1858,7 +1858,6 @@ int __set_page_dirty_nobuffers(struct page *page); int __set_page_dirty_no_writeback(struct page *page); int redirty_page_for_writepage(struct writeback_control *wbc, struct page *page); -void account_page_dirtied(struct page *page, struct address_space *mapping); void account_page_cleaned(struct page *page, struct address_space *mapping, struct bdi_writeback *wb); int set_page_dirty(struct page *page); diff --git a/mm/page-writeback.c b/mm/page-writeback.c index f6ee085767cb..0c2c8355f97f 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -2420,7 +2420,8 @@ int __set_page_dirty_no_writeback(struct page *page) * * NOTE: This relies on being atomic wrt interrupts. */ -void account_page_dirtied(struct page *page, struct address_space *mapping) +static void account_page_dirtied(struct page *page, + struct address_space *mapping) { struct inode *inode = mapping->host; @@ -2461,6 +2462,30 @@ void account_page_cleaned(struct page *page, struct address_space *mapping, } } +/* + * Mark the page dirty, and set it dirty in the page cache, and mark the inode + * dirty. + * + * If warn is true, then emit a warning if the page is not uptodate and has + * not been truncated. + * + * The caller must hold lock_page_memcg(). + */ +void __set_page_dirty(struct page *page, struct address_space *mapping, + int warn) +{ + unsigned long flags; + + xa_lock_irqsave(&mapping->i_pages, flags); + if (page->mapping) { /* Race with truncate? */ + WARN_ON_ONCE(warn && !PageUptodate(page)); + account_page_dirtied(page, mapping); + __xa_set_mark(&mapping->i_pages, page_index(page), + PAGECACHE_TAG_DIRTY); + } + xa_unlock_irqrestore(&mapping->i_pages, flags); +} + /* * For address_spaces which do not use buffers. Just tag the page as dirty in * the xarray.
Nothing in __set_page_dirty() is specific to buffer_head, so move it to mm/page-writeback.c. That removes the only caller of account_page_dirtied() outside of page-writeback.c, so make it static. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/buffer.c | 24 ------------------------ include/linux/mm.h | 1 - mm/page-writeback.c | 27 ++++++++++++++++++++++++++- 3 files changed, 26 insertions(+), 26 deletions(-)