On Wed, Jun 16, 2021 at 05:28:14PM +0100, Matthew Wilcox wrote: > On Wed, Jun 16, 2021 at 08:50:40AM +0200, Greg Kroah-Hartman wrote: > > On Tue, Jun 15, 2021 at 07:13:16PM +0100, Matthew Wilcox wrote: > > > On Tue, Jun 15, 2021 at 07:34:53PM +0200, Christoph Hellwig wrote: > > > > Eventually everything around set_page_dirty should be changed to operate > > > > on folios, and that will be a good time to come up with a sane > > > > naming scheme without introducing extra churn. > > > > > > The way it currently looks in my tree ... > > > > > > set_page_dirty(page) is a thin wrapper that calls folio_mark_dirty(folio). > > > folio_mark_dirty() calls a_ops->dirty_folio(mapping, folio) (which > > > returns bool). > > > __set_page_dirty_nobuffers() becomes filemap_dirty_folio() > > > __set_page_dirty_buffers() becomes block_dirty_folio() > > > __set_page_dirty_no_writeback() becomes dirty_folio_no_writeback() > > > > > > Now I look at it, maybe that last should be nowb_dirty_folio(). > > > > Not to be a pain, but you are mixing "folio" at the front and back of > > the api name? We messed up in the driver core with this for some things > > (get_device() being one), I would recommend just sticking with one > > naming scheme now as you are getting to pick what you want to use. > > That is mostly what I'm doing. eg, > > get_page -> folio_get > lock_page -> folio_lock > PageUptodate -> folio_uptodate > set_page_dirty -> folio_mark_dirty Nice. > What I haven't dealt with yet is the naming of the > address_space_operations. My thinking with those is that they should > be verb_folio, since they _aren't_ the functions that get called. > ie it looks like this: > > folio_mark_dirty() > aops->dirty_folio() > ext4_dirty_folio() > buffer_dirty_folio() > > I actually see the inconsistency here as a good thing -- these are > implementations of the aop, so foo_verb_folio() means you're doing > something weird and internal instead of going through the vfs/mm. > > That implies doing things like renaming ->readpage to ->read_folio, but > if we're changing the API from passing a struct page to a struct folio, > that can all be done at the same time with no additional disruption. Ok, as long as there's a reason for the naming scheme, I'm happy as hopefully it will make sense to others as well. thanks, greg k-h
