> Linus Torvalds <[email protected]> wrote: > > > The PG_fscache bit waiting functions are completely crazy. The comment > > about "this will wake up others" is actively wrong, and the waiting > > function looks insane, because you're mixing the two names for > > "fscache" which makes the code look totally incomprehensible. Why > > would we wait for PF_fscache, when PG_private_2 was set? Yes, I know > > why, but the code looks entirely nonsensical. > > How about the attached change to make it more coherent and fix the doc > comment?
Then I could follow it up with this patch here, moving towards dropping the PG_fscache alias for the new API. David --- commit b415fafb07166732933242e938626fc6cdbdbc5b Author: David Howells <[email protected]> Date: Wed Feb 10 11:22:59 2021 +0000 netfs: Move towards dropping the PG_fscache alias for PG_private_2 Move towards dropping the PG_fscache alias for PG_private_2 with the new fscache I/O API and netfs helper lib (this does not alter the old API). Signed-off-by: David Howells <[email protected]> diff --git a/fs/afs/file.c b/fs/afs/file.c index 8f28d4f4cfd7..bb8c6d501292 100644 --- a/fs/afs/file.c +++ b/fs/afs/file.c @@ -438,7 +438,7 @@ static void afs_invalidatepage(struct page *page, unsigned int offset, if (PagePrivate(page)) afs_invalidate_dirty(page, offset, length); - wait_on_page_fscache(page); + wait_on_page_private_2(page); _leave(""); } @@ -457,10 +457,10 @@ static int afs_releasepage(struct page *page, gfp_t gfp_flags) /* deny if page is being written to the cache and the caller hasn't * elected to wait */ #ifdef CONFIG_AFS_FSCACHE - if (PageFsCache(page)) { + if (PagePrivate2(page)) { if (!(gfp_flags & __GFP_DIRECT_RECLAIM) || !(gfp_flags & __GFP_FS)) return false; - wait_on_page_fscache(page); + wait_on_page_private_2(page); } #endif diff --git a/fs/afs/write.c b/fs/afs/write.c index e672833c99bc..9c554981f53b 100644 --- a/fs/afs/write.c +++ b/fs/afs/write.c @@ -118,7 +118,7 @@ int afs_write_begin(struct file *file, struct address_space *mapping, } #ifdef CONFIG_AFS_FSCACHE - wait_on_page_fscache(page); + wait_on_page_private_2(page); #endif index = page->index; @@ -929,8 +929,8 @@ vm_fault_t afs_page_mkwrite(struct vm_fault *vmf) * be modified. We then assume the entire page will need writing back. */ #ifdef CONFIG_AFS_FSCACHE - if (PageFsCache(page) && - wait_on_page_bit_killable(page, PG_fscache) < 0) + if (PagePrivate2(page) && + wait_on_page_bit_killable(page, PG_private_2) < 0) return VM_FAULT_RETRY; #endif @@ -1026,6 +1026,6 @@ int afs_launder_page(struct page *page) trace_afs_page_dirty(vnode, tracepoint_string("laundered"), page); detach_page_private(page); - wait_on_page_fscache(page); + wait_on_page_private_2(page); return ret; } diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index 0dd64d31eff6..fd2498567b49 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -147,7 +147,7 @@ static void ceph_invalidatepage(struct page *page, unsigned int offset, struct ceph_inode_info *ci; struct ceph_snap_context *snapc = page_snap_context(page); - wait_on_page_fscache(page); + wait_on_page_private_2(page); inode = page->mapping->host; ci = ceph_inode(inode); @@ -176,10 +176,10 @@ static int ceph_releasepage(struct page *page, gfp_t gfp_flags) dout("%p releasepage %p idx %lu (%sdirty)\n", page->mapping->host, page, page->index, PageDirty(page) ? "" : "not "); - if (PageFsCache(page)) { + if (PagePrivate2(page)) { if (!(gfp_flags & __GFP_DIRECT_RECLAIM) || !(gfp_flags & __GFP_FS)) return 0; - wait_on_page_fscache(page); + wait_on_page_private_2(page); } return !PagePrivate(page); } @@ -1258,7 +1258,7 @@ static int ceph_write_begin(struct file *file, struct address_space *mapping, &ceph_netfs_write_begin_ops, NULL); out: if (r == 0) - wait_on_page_fscache(page); + wait_on_page_private_2(page); if (r < 0) { if (page) put_page(page); diff --git a/fs/netfs/read_helper.c b/fs/netfs/read_helper.c index 156941e0de61..9018224693e9 100644 --- a/fs/netfs/read_helper.c +++ b/fs/netfs/read_helper.c @@ -223,7 +223,7 @@ static void netfs_rreq_completed(struct netfs_read_request *rreq) /* * Deal with the completion of writing the data to the cache. We have to clear - * the PG_fscache bits on the pages involved and release the caller's ref. + * the PG_private_2 bits on the pages involved and release the caller's ref. * * May be called in softirq mode and we inherit a ref from the caller. */ @@ -246,7 +246,7 @@ static void netfs_rreq_unmark_after_write(struct netfs_read_request *rreq) if (have_unlocked && page->index <= unlocked) continue; unlocked = page->index; - unlock_page_fscache(page); + unlock_page_private_2(page); have_unlocked = true; } } @@ -357,7 +357,7 @@ static void netfs_rreq_write_to_cache(struct netfs_read_request *rreq) } /* - * Unlock the pages in a read operation. We need to set PG_fscache on any + * Unlock the pages in a read operation. We need to set PG_private_2 on any * pages we're going to write back before we unlock them. */ static void netfs_rreq_unlock(struct netfs_read_request *rreq) @@ -404,7 +404,7 @@ static void netfs_rreq_unlock(struct netfs_read_request *rreq) break; } if (test_bit(NETFS_SREQ_WRITE_TO_CACHE, &subreq->flags)) - SetPageFsCache(page); + SetPagePrivate2(page); pg_failed |= subreq_failed; if (pgend < iopos + subreq->len) break; @@ -1142,7 +1142,7 @@ int netfs_write_begin(struct file *file, struct address_space *mapping, goto error; have_page: - wait_on_page_fscache(page); + wait_on_page_private_2(page); have_page_no_wait: if (netfs_priv) ops->cleanup(netfs_priv, mapping); diff --git a/include/linux/fscache.h b/include/linux/fscache.h index 3f177faa0ac2..ccf533288291 100644 --- a/include/linux/fscache.h +++ b/include/linux/fscache.h @@ -29,6 +29,17 @@ #define fscache_cookie_valid(cookie) (0) #endif +#ifndef FSCACHE_USE_NEW_IO_API +/* + * Overload PG_private_2 to give us PG_fscache - this is used to indicate that + * a page is currently backed by a local disk cache + */ +#define PageFsCache(page) PagePrivate2((page)) +#define SetPageFsCache(page) SetPagePrivate2((page)) +#define ClearPageFsCache(page) ClearPagePrivate2((page)) +#define TestSetPageFsCache(page) TestSetPagePrivate2((page)) +#define TestClearPageFsCache(page) TestClearPagePrivate2((page)) +#endif /* pattern used to fill dead space in an index entry */ #define FSCACHE_INDEX_DEADFILL_PATTERN 0x79 diff --git a/include/linux/netfs.h b/include/linux/netfs.h index d4cb6e6f704c..be03c3b6f0dc 100644 --- a/include/linux/netfs.h +++ b/include/linux/netfs.h @@ -15,18 +15,6 @@ #include <linux/workqueue.h> #include <linux/fs.h> -/* - * Overload PG_private_2 to give us PG_fscache - this is used to indicate that - * a page is currently backed by a local disk cache - */ -#define PageFsCache(page) PagePrivate2((page)) -#define SetPageFsCache(page) SetPagePrivate2((page)) -#define ClearPageFsCache(page) ClearPagePrivate2((page)) -#define TestSetPageFsCache(page) TestSetPagePrivate2((page)) -#define TestClearPageFsCache(page) TestClearPagePrivate2((page)) -#define wait_on_page_fscache(page) wait_on_page_private_2((page)) -#define unlock_page_fscache(page) unlock_page_private_2((page)) - enum netfs_read_source { NETFS_FILL_WITH_ZEROES, NETFS_DOWNLOAD_FROM_SERVER, diff --git a/mm/filemap.c b/mm/filemap.c index 7d321152d579..e7b2a1e2c40b 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -3604,7 +3604,7 @@ EXPORT_SYMBOL(generic_file_write_iter); * The address_space is to try to release any data against the page * (presumably at page->private). * - * This may also be called if PG_fscache is set on a page, indicating that the + * This may also be called if PG_private_2 is set on a page, indicating that the * page is known to the local caching routines. * * The @gfp_mask argument specifies whether I/O may be performed to release diff --git a/mm/readahead.c b/mm/readahead.c index 4446dada0bc2..01209a46e834 100644 --- a/mm/readahead.c +++ b/mm/readahead.c @@ -40,7 +40,7 @@ EXPORT_SYMBOL_GPL(file_ra_state_init); /* * see if a page needs releasing upon read_cache_pages() failure - * - the caller of read_cache_pages() may have set PG_private or PG_fscache + * - the caller of read_cache_pages() may have set PG_private or PG_private_2 * before calling, such as the NFS fs marking pages that are cached locally * on disk, thus we need to give the fs a chance to clean up in the event of * an error -- Linux-cachefs mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-cachefs
