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

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

"Darrick J. Wong" <djwong@kernel.org> writes:

> On Mon, Jun 12, 2023 at 04:08:51PM +0100, Matthew Wilcox wrote:
>> On Mon, Jun 12, 2023 at 08:05:20AM -0700, Darrick J. Wong wrote:
>> > static inline struct iomap_folio_state *
>> > to_folio_state(struct folio *folio)
>> > {
>> > 	return folio->private;
>> > }
>> 
>> I'd reformat it to not-XFS-style:
>> 
>> static inline struct iomap_folio_state *to_folio_state(struct folio *folio)
>> {
>> 	return folio->private;
>> }
>> 
>> But other than that, I approve.  Unless you just want to do without it
>> entirely and do
>> 
>> 	struct iomap_folio_state *state = folio->private;
>> 
>> instead of
>> 
>> 	struct iomap_folio_state *state = to_folio_state(folio);
>> 
>> I'm having a hard time caring between the two.
>
> Either's fine with me too.  I'm getting tired of reading this series
> over and over again.
>
> Ritesh: Please pick whichever variant you like, and that's it, no more
> discussion.

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

Sure this looks fine to me. So, I am hoping that there is no need to check
folio_test_private(folio) PG_private flag here before returning
folio->private (which was the case in original code to_iomap_page())

I did take a cursory look and didn't find any reason to test for doing
folio_test_private(folio) here. It should always remain set between
iomap_ifs_alloc() and iomap_ifs_free().

- IIUC, it is mostly for MM subsystem to see whether there is a
private FS data attached to a folio for which they think we might have
to call FS callback. for e.g. .is_dirty_writeback callback.
- Or like FS can use it within it's own subsystem to say whether a
folio is being associated with an in-progress read or write request. (e.g. NFS?)


-ritesh