On Mon, Jun 12, 2023 at 11:13:42PM +0530, Ritesh Harjani wrote: > "Darrick J. Wong" <djwong@xxxxxxxxxx> writes: > > 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?) The PG_private flag is being obsoleted in favour of just testing folio->private for being NULL. It's still in widespread use, but I'm removing it as I change code. I think we'll probably get rid of PG_error first, but the more page flags we free up the better. PG_hwpoison is also scheduled for removal, but that requires folios to be dynamically allocated first, so freeing that bit probably comes third.