"Darrick J. Wong" <djwong@xxxxxxxxxx> 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
