Re: [PATCHv9 1/6] iomap: Rename iomap_page to iomap_folio_state and others

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Jun 12, 2023 at 02:49:50PM +0530, Ritesh Harjani wrote:
> Christoph Hellwig <hch@infradead.org> writes:
> 
> > On Sun, Jun 11, 2023 at 11:21:48PM -0700, Christoph Hellwig wrote:
> >> Looks good:
> >>
> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Thanks!
> 
> >
> > Actually, coming back to this:
> >
> > -to_iomap_page(struct folio *folio)
> > +static inline struct iomap_folio_state *iomap_get_ifs
> >
> > I find the to_* naming much more descriptive for derving the
> > private data.  get tends to imply grabbing a refcount.
> 
> Not always. That would be iomap_ifs_get(ifs)/iomap_ifs_put(ifs).
> 
> See this -
> 
> static inline void *folio_get_private(struct folio *folio)
> {
> 	return folio->private;
> }
> 
> So, is it ok if we hear from others too on iomap_get_ifs() function
> name?

It's a static inline, no need for namespacing in the name.  And hch is
right, _get/_put often imply receiving and returning some active
refcount.  That (IMO) makes folio_get_private the odd one out, since
pages don't refcount the private pointer.

I think of this more as a C(rap)-style type conversion function for a
generic object that can get touched by many subsystems (and hence we
cannot do the embed-parent-in-child-object thing).

So.

static inline struct iomap_folio_state *
to_folio_state(struct folio *folio)
{
	return folio->private;
}

is fine with me.  Can we go with this, and not make Ritesh run around
renaming and rebasing beyond v10?

[08:02] <willy> honestly, I think even having the abstraction was a
mistake.  just use folio->private

--D

> -ritesh