On Mon, Jun 12, 2023 at 02:49:50PM +0530, Ritesh Harjani wrote:
> Christoph Hellwig <hch@xxxxxxxxxxxxx> writes:
>
> > On Sun, Jun 11, 2023 at 11:21:48PM -0700, Christoph Hellwig wrote:
> >> Looks good:
> >>
> >> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
>
> 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
