Skip to content

Commit b4fa966

Browse files
dhowellsakpm00
authored andcommitted
mm, netfs, fscache: stop read optimisation when folio removed from pagecache
Fscache has an optimisation by which reads from the cache are skipped until we know that (a) there's data there to be read and (b) that data isn't entirely covered by pages resident in the netfs pagecache. This is done with two flags manipulated by fscache_note_page_release(): if (... test_bit(FSCACHE_COOKIE_HAVE_DATA, &cookie->flags) && test_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags)) clear_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags); where the NO_DATA_TO_READ flag causes cachefiles_prepare_read() to indicate that netfslib should download from the server or clear the page instead. The fscache_note_page_release() function is intended to be called from ->releasepage() - but that only gets called if PG_private or PG_private_2 is set - and currently the former is at the discretion of the network filesystem and the latter is only set whilst a page is being written to the cache, so sometimes we miss clearing the optimisation. Fix this by following Willy's suggestion[1] and adding an address_space flag, AS_RELEASE_ALWAYS, that causes filemap_release_folio() to always call ->release_folio() if it's set, even if PG_private or PG_private_2 aren't set. Note that this would require folio_test_private() and page_has_private() to become more complicated. To avoid that, in the places[*] where these are used to conditionalise calls to filemap_release_folio() and try_to_release_page(), the tests are removed the those functions just jumped to unconditionally and the test is performed there. [*] There are some exceptions in vmscan.c where the check guards more than just a call to the releaser. I've added a function, folio_needs_release() to wrap all the checks for that. AS_RELEASE_ALWAYS should be set if a non-NULL cookie is obtained from fscache and cleared in ->evict_inode() before truncate_inode_pages_final() is called. Additionally, the FSCACHE_COOKIE_NO_DATA_TO_READ flag needs to be cleared and the optimisation cancelled if a cachefiles object already contains data when we open it. [[email protected]: call folio_mapping() inside folio_needs_release()] Link: DaveWysochanskiRH/kernel@902c990 Link: https://lkml.kernel.org/r/[email protected] Fixes: 1f67e6d ("fscache: Provide a function to note the release of a page") Fixes: 047487c ("cachefiles: Implement the I/O routines") Signed-off-by: David Howells <[email protected]> Signed-off-by: Dave Wysochanski <[email protected]> Reported-by: Rohith Surabattula <[email protected]> Suggested-by: Matthew Wilcox <[email protected]> Tested-by: SeongJae Park <[email protected]> Cc: Daire Byrne <[email protected]> Cc: Matthew Wilcox <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: Steve French <[email protected]> Cc: Shyam Prasad N <[email protected]> Cc: Rohith Surabattula <[email protected]> Cc: Dave Wysochanski <[email protected]> Cc: Dominique Martinet <[email protected]> Cc: Ilya Dryomov <[email protected]> Cc: Andreas Dilger <[email protected]> Cc: Jingbo Xu <[email protected]> Cc: "Theodore Ts'o" <[email protected]> Cc: Xiubo Li <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
1 parent 0201ebf commit b4fa966

File tree

8 files changed

+33
-1
lines changed

8 files changed

+33
-1
lines changed

fs/9p/cache.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ void v9fs_cache_inode_get_cookie(struct inode *inode)
6868
&path, sizeof(path),
6969
&version, sizeof(version),
7070
i_size_read(&v9inode->netfs.inode));
71+
if (v9inode->netfs.cache)
72+
mapping_set_release_always(inode->i_mapping);
7173

7274
p9_debug(P9_DEBUG_FSC, "inode %p get cookie %p\n",
7375
inode, v9fs_inode_cookie(v9inode));

fs/afs/internal.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,8 @@ static inline void afs_vnode_set_cache(struct afs_vnode *vnode,
681681
{
682682
#ifdef CONFIG_AFS_FSCACHE
683683
vnode->netfs.cache = cookie;
684+
if (cookie)
685+
mapping_set_release_always(vnode->netfs.inode.i_mapping);
684686
#endif
685687
}
686688

fs/cachefiles/namei.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,8 @@ static bool cachefiles_open_file(struct cachefiles_object *object,
585585
if (ret < 0)
586586
goto check_failed;
587587

588+
clear_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &object->cookie->flags);
589+
588590
object->file = file;
589591

590592
/* Always update the atime on an object we've just looked up (this is

fs/ceph/cache.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ void ceph_fscache_register_inode_cookie(struct inode *inode)
3636
&ci->i_vino, sizeof(ci->i_vino),
3737
&ci->i_version, sizeof(ci->i_version),
3838
i_size_read(inode));
39+
if (ci->netfs.cache)
40+
mapping_set_release_always(inode->i_mapping);
3941
}
4042

4143
void ceph_fscache_unregister_inode_cookie(struct ceph_inode_info *ci)

fs/nfs/fscache.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,9 @@ void nfs_fscache_init_inode(struct inode *inode)
180180
&auxdata, /* aux_data */
181181
sizeof(auxdata),
182182
i_size_read(inode));
183+
184+
if (netfs_inode(inode)->cache)
185+
mapping_set_release_always(inode->i_mapping);
183186
}
184187

185188
/*

fs/smb/client/fscache.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ void cifs_fscache_get_inode_cookie(struct inode *inode)
108108
&cifsi->uniqueid, sizeof(cifsi->uniqueid),
109109
&cd, sizeof(cd),
110110
i_size_read(&cifsi->netfs.inode));
111+
if (cifsi->netfs.cache)
112+
mapping_set_release_always(inode->i_mapping);
111113
}
112114

113115
void cifs_fscache_unuse_inode_cookie(struct inode *inode, bool update)

include/linux/pagemap.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ enum mapping_flags {
203203
/* writeback related tags are not used */
204204
AS_NO_WRITEBACK_TAGS = 5,
205205
AS_LARGE_FOLIO_SUPPORT = 6,
206+
AS_RELEASE_ALWAYS, /* Call ->release_folio(), even if no private data */
206207
};
207208

208209
/**
@@ -273,6 +274,21 @@ static inline int mapping_use_writeback_tags(struct address_space *mapping)
273274
return !test_bit(AS_NO_WRITEBACK_TAGS, &mapping->flags);
274275
}
275276

277+
static inline bool mapping_release_always(const struct address_space *mapping)
278+
{
279+
return test_bit(AS_RELEASE_ALWAYS, &mapping->flags);
280+
}
281+
282+
static inline void mapping_set_release_always(struct address_space *mapping)
283+
{
284+
set_bit(AS_RELEASE_ALWAYS, &mapping->flags);
285+
}
286+
287+
static inline void mapping_clear_release_always(struct address_space *mapping)
288+
{
289+
clear_bit(AS_RELEASE_ALWAYS, &mapping->flags);
290+
}
291+
276292
static inline gfp_t mapping_gfp_mask(struct address_space * mapping)
277293
{
278294
return mapping->gfp_mask;

mm/internal.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,10 @@ static inline void set_page_refcounted(struct page *page)
181181
*/
182182
static inline bool folio_needs_release(struct folio *folio)
183183
{
184-
return folio_has_private(folio);
184+
struct address_space *mapping = folio_mapping(folio);
185+
186+
return folio_has_private(folio) ||
187+
(mapping && mapping_release_always(mapping));
185188
}
186189

187190
extern unsigned long highest_memmap_pfn;

0 commit comments

Comments
 (0)