Re: [PATCH] fs: clean up usage of noop_dirty_folio

On Mon, Aug 21, 2023 at 01:16:43PM +0200, Jan Kara wrote:

On Sat 19-08-23 20:42:25, Xueshi Hu wrote:

quoted

In folio_mark_dirty(), it will automatically fallback to
noop_dirty_folio() if a_ops->dirty_folio is not registered.

As anon_aops, dev_dax_aops and fb_deferred_io_aops becames empty, remove
them too.

Signed-off-by: Xueshi Hu <redacted>

Yeah, looks sensible to me but for some callbacks we are oscilating between
all users having to provide some callback and providing some default
behavior for NULL callback. I don't have a strong opinion either way so
feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

But I guess let's see what Matthew thinks about this and what plans he has
so that we don't switch back again in the near future. Matthew?

I was hoping Christoph would weigh in ;-)  I don't have a strong
feeling here, but it seems to me that a NULL ->dirty_folio() should mean
"do the noop thing" rather than "do the buffer_head thing" or "do the
filemap thing".  In 0af573780b0b, the buffer_head default was removed.
I think enough time has passed that we're OK to change what a NULL
->dirty_folio means (plus we also changed the name of ->set_page_dirty()
to ->dirty_folio())

So Ack to the concept.  One minor change I'd request:

-bool noop_dirty_folio(struct address_space *mapping, struct folio *folio)
+static bool noop_dirty_folio(struct address_space *mapping, struct folio *folio)
 {
 	if (!folio_test_dirty(folio))
 		return !folio_test_set_dirty(folio);
 	return false;
 }
-EXPORT_SYMBOL(noop_dirty_folio);

Please inline this into folio_mark_dirty() instead of calling it.