From 8fb883f3e30065529e4f35d4b4f355193dcdb7a2 Mon Sep 17 00:00:00 2001 From: David Howells Date: Sat, 21 Sep 2013 00:09:31 +0100 Subject: FS-Cache: Add use/unuse/wake cookie wrappers Add wrapper functions for dealing with cookie->n_active: (*) __fscache_use_cookie() to increment it. (*) __fscache_unuse_cookie() to decrement and test against zero. (*) __fscache_wake_unused_cookie() to wake up anyone waiting for it to reach zero. The second and third are split so that the third can be done after cookie->lock has been released in case the waiter wakes up whilst we're still holding it and tries to get it. We will need to wake-on-zero once the cookie disablement patch is applied because it will then be possible to see n_active become zero without the cookie being relinquished. Also move the cookie usement out of fscache_attr_changed_op() and into fscache_attr_changed() and the operation struct so that cookie disablement will be able to track it. Whilst we're at it, only increment n_active if we're about to do fscache_submit_op() so that we don't have to deal with undoing it if anything earlier fails. Possibly this should be moved into fscache_submit_op() which could look at FSCACHE_OP_UNUSE_COOKIE. Signed-off-by: David Howells --- include/linux/fscache-cache.h | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) (limited to 'include/linux') diff --git a/include/linux/fscache-cache.h b/include/linux/fscache-cache.h index 7823e9ef995e..96a2b66f5968 100644 --- a/include/linux/fscache-cache.h +++ b/include/linux/fscache-cache.h @@ -511,6 +511,11 @@ static inline void fscache_end_io(struct fscache_retrieval *op, op->end_io_func(page, op->context, error); } +static inline void __fscache_use_cookie(struct fscache_cookie *cookie) +{ + atomic_inc(&cookie->n_active); +} + /** * fscache_use_cookie - Request usage of cookie attached to an object * @object: Object description @@ -524,6 +529,16 @@ static inline bool fscache_use_cookie(struct fscache_object *object) return atomic_inc_not_zero(&cookie->n_active) != 0; } +static inline bool __fscache_unuse_cookie(struct fscache_cookie *cookie) +{ + return atomic_dec_and_test(&cookie->n_active); +} + +static inline void __fscache_wake_unused_cookie(struct fscache_cookie *cookie) +{ + wake_up_atomic_t(&cookie->n_active); +} + /** * fscache_unuse_cookie - Cease usage of cookie attached to an object * @object: Object description @@ -534,8 +549,8 @@ static inline bool fscache_use_cookie(struct fscache_object *object) static inline void fscache_unuse_cookie(struct fscache_object *object) { struct fscache_cookie *cookie = object->cookie; - if (atomic_dec_and_test(&cookie->n_active)) - wake_up_atomic_t(&cookie->n_active); + if (__fscache_unuse_cookie(cookie)) + __fscache_wake_unused_cookie(cookie); } /* -- cgit v1.2.3 From 94d30ae90a00cafe686c1057be57f4885f963abf Mon Sep 17 00:00:00 2001 From: David Howells Date: Sat, 21 Sep 2013 00:09:31 +0100 Subject: FS-Cache: Provide the ability to enable/disable cookies Provide the ability to enable and disable fscache cookies. A disabled cookie will reject or ignore further requests to: Acquire a child cookie Invalidate and update backing objects Check the consistency of a backing object Allocate storage for backing page Read backing pages Write to backing pages but still allows: Checks/waits on the completion of already in-progress objects Uncaching of pages Relinquishment of cookies Two new operations are provided: (1) Disable a cookie: void fscache_disable_cookie(struct fscache_cookie *cookie, bool invalidate); If the cookie is not already disabled, this locks the cookie against other dis/enablement ops, marks the cookie as being disabled, discards or invalidates any backing objects and waits for cessation of activity on any associated object. This is a wrapper around a chunk split out of fscache_relinquish_cookie(), but it reinitialises the cookie such that it can be reenabled. All possible failures are handled internally. The caller should consider calling fscache_uncache_all_inode_pages() afterwards to make sure all page markings are cleared up. (2) Enable a cookie: void fscache_enable_cookie(struct fscache_cookie *cookie, bool (*can_enable)(void *data), void *data) If the cookie is not already enabled, this locks the cookie against other dis/enablement ops, invokes can_enable() and, if the cookie is not an index cookie, will begin the procedure of acquiring backing objects. The optional can_enable() function is passed the data argument and returns a ruling as to whether or not enablement should actually be permitted to begin. All possible failures are handled internally. The cookie will only be marked as enabled if provisional backing objects are allocated. A later patch will introduce these to NFS. Cookie enablement during nfs_open() is then contingent on i_writecount <= 0. can_enable() checks for a race between open(O_RDONLY) and open(O_WRONLY/O_RDWR). This simplifies NFS's cookie handling and allows us to get rid of open(O_RDONLY) accidentally introducing caching to an inode that's open for writing already. One operation has its API modified: (3) Acquire a cookie. struct fscache_cookie *fscache_acquire_cookie( struct fscache_cookie *parent, const struct fscache_cookie_def *def, void *netfs_data, bool enable); This now has an additional argument that indicates whether the requested cookie should be enabled by default. It doesn't need the can_enable() function because the caller must prevent multiple calls for the same netfs object and it doesn't need to take the enablement lock because no one else can get at the cookie before this returns. Signed-off-by: David Howells object_list */ struct hlist_node cookie_link; /* link in cookie->backing_objects */ diff --git a/include/linux/fscache.h b/include/linux/fscache.h index 19b46458e4e8..115bb81912cc 100644 --- a/include/linux/fscache.h +++ b/include/linux/fscache.h @@ -166,6 +166,42 @@ struct fscache_netfs { struct list_head link; /* internal link */ }; +/* + * data file or index object cookie + * - a file will only appear in one cache + * - a request to cache a file may or may not be honoured, subject to + * constraints such as disk space + * - indices are created on disk just-in-time + */ +struct fscache_cookie { + atomic_t usage; /* number of users of this cookie */ + atomic_t n_children; /* number of children of this cookie */ + atomic_t n_active; /* number of active users of netfs ptrs */ + spinlock_t lock; + spinlock_t stores_lock; /* lock on page store tree */ + struct hlist_head backing_objects; /* object(s) backing this file/index */ + const struct fscache_cookie_def *def; /* definition */ + struct fscache_cookie *parent; /* parent of this entry */ + void *netfs_data; /* back pointer to netfs */ + struct radix_tree_root stores; /* pages to be stored on this cookie */ +#define FSCACHE_COOKIE_PENDING_TAG 0 /* pages tag: pending write to cache */ +#define FSCACHE_COOKIE_STORING_TAG 1 /* pages tag: writing to cache */ + + unsigned long flags; +#define FSCACHE_COOKIE_LOOKING_UP 0 /* T if non-index cookie being looked up still */ +#define FSCACHE_COOKIE_NO_DATA_YET 1 /* T if new object with no cached data yet */ +#define FSCACHE_COOKIE_UNAVAILABLE 2 /* T if cookie is unavailable (error, etc) */ +#define FSCACHE_COOKIE_INVALIDATING 3 /* T if cookie is being invalidated */ +#define FSCACHE_COOKIE_RELINQUISHED 4 /* T if cookie has been relinquished */ +#define FSCACHE_COOKIE_ENABLED 5 /* T if cookie is enabled */ +#define FSCACHE_COOKIE_ENABLEMENT_LOCK 6 /* T if cookie is being en/disabled */ +}; + +static inline bool fscache_cookie_enabled(struct fscache_cookie *cookie) +{ + return test_bit(FSCACHE_COOKIE_ENABLED, &cookie->flags); +} + /* * slow-path functions for when there is actually caching available, and the * netfs does actually have a valid token @@ -181,8 +217,8 @@ extern void __fscache_release_cache_tag(struct fscache_cache_tag *); extern struct fscache_cookie *__fscache_acquire_cookie( struct fscache_cookie *, const struct fscache_cookie_def *, - void *); -extern void __fscache_relinquish_cookie(struct fscache_cookie *, int); + void *, bool); +extern void __fscache_relinquish_cookie(struct fscache_cookie *, bool); extern int __fscache_check_consistency(struct fscache_cookie *); extern void __fscache_update_cookie(struct fscache_cookie *); extern int __fscache_attr_changed(struct fscache_cookie *); @@ -211,6 +247,9 @@ extern void __fscache_uncache_all_inode_pages(struct fscache_cookie *, struct inode *); extern void __fscache_readpages_cancel(struct fscache_cookie *cookie, struct list_head *pages); +extern void __fscache_disable_cookie(struct fscache_cookie *, bool); +extern void __fscache_enable_cookie(struct fscache_cookie *, + bool (*)(void *), void *); /** * fscache_register_netfs - Register a filesystem as desiring caching services @@ -289,6 +328,7 @@ void fscache_release_cache_tag(struct fscache_cache_tag *tag) * @def: A description of the cache object, including callback operations * @netfs_data: An arbitrary piece of data to be kept in the cookie to * represent the cache object to the netfs + * @enable: Whether or not to enable a data cookie immediately * * This function is used to inform FS-Cache about part of an index hierarchy * that can be used to locate files. This is done by requesting a cookie for @@ -301,10 +341,12 @@ static inline struct fscache_cookie *fscache_acquire_cookie( struct fscache_cookie *parent, const struct fscache_cookie_def *def, - void *netfs_data) + void *netfs_data, + bool enable) { - if (fscache_cookie_valid(parent)) - return __fscache_acquire_cookie(parent, def, netfs_data); + if (fscache_cookie_valid(parent) && fscache_cookie_enabled(parent)) + return __fscache_acquire_cookie(parent, def, netfs_data, + enable); else return NULL; } @@ -322,7 +364,7 @@ struct fscache_cookie *fscache_acquire_cookie( * description. */ static inline -void fscache_relinquish_cookie(struct fscache_cookie *cookie, int retire) +void fscache_relinquish_cookie(struct fscache_cookie *cookie, bool retire) { if (fscache_cookie_valid(cookie)) __fscache_relinquish_cookie(cookie, retire); @@ -341,7 +383,7 @@ void fscache_relinquish_cookie(struct fscache_cookie *cookie, int retire) static inline int fscache_check_consistency(struct fscache_cookie *cookie) { - if (fscache_cookie_valid(cookie)) + if (fscache_cookie_valid(cookie) && fscache_cookie_enabled(cookie)) return __fscache_check_consistency(cookie); else return 0; @@ -360,7 +402,7 @@ int fscache_check_consistency(struct fscache_cookie *cookie) static inline void fscache_update_cookie(struct fscache_cookie *cookie) { - if (fscache_cookie_valid(cookie)) + if (fscache_cookie_valid(cookie) && fscache_cookie_enabled(cookie)) __fscache_update_cookie(cookie); } @@ -407,7 +449,7 @@ void fscache_unpin_cookie(struct fscache_cookie *cookie) static inline int fscache_attr_changed(struct fscache_cookie *cookie) { - if (fscache_cookie_valid(cookie)) + if (fscache_cookie_valid(cookie) && fscache_cookie_enabled(cookie)) return __fscache_attr_changed(cookie); else return -ENOBUFS; @@ -429,7 +471,7 @@ int fscache_attr_changed(struct fscache_cookie *cookie) static inline void fscache_invalidate(struct fscache_cookie *cookie) { - if (fscache_cookie_valid(cookie)) + if (fscache_cookie_valid(cookie) && fscache_cookie_enabled(cookie)) __fscache_invalidate(cookie); } @@ -503,7 +545,7 @@ int fscache_read_or_alloc_page(struct fscache_cookie *cookie, void *context, gfp_t gfp) { - if (fscache_cookie_valid(cookie)) + if (fscache_cookie_valid(cookie) && fscache_cookie_enabled(cookie)) return __fscache_read_or_alloc_page(cookie, page, end_io_func, context, gfp); else @@ -554,7 +596,7 @@ int fscache_read_or_alloc_pages(struct fscache_cookie *cookie, void *context, gfp_t gfp) { - if (fscache_cookie_valid(cookie)) + if (fscache_cookie_valid(cookie) && fscache_cookie_enabled(cookie)) return __fscache_read_or_alloc_pages(cookie, mapping, pages, nr_pages, end_io_func, context, gfp); @@ -585,7 +627,7 @@ int fscache_alloc_page(struct fscache_cookie *cookie, struct page *page, gfp_t gfp) { - if (fscache_cookie_valid(cookie)) + if (fscache_cookie_valid(cookie) && fscache_cookie_enabled(cookie)) return __fscache_alloc_page(cookie, page, gfp); else return -ENOBUFS; @@ -634,7 +676,7 @@ int fscache_write_page(struct fscache_cookie *cookie, struct page *page, gfp_t gfp) { - if (fscache_cookie_valid(cookie)) + if (fscache_cookie_valid(cookie) && fscache_cookie_enabled(cookie)) return __fscache_write_page(cookie, page, gfp); else return -ENOBUFS; @@ -744,4 +786,47 @@ void fscache_uncache_all_inode_pages(struct fscache_cookie *cookie, __fscache_uncache_all_inode_pages(cookie, inode); } +/** + * fscache_disable_cookie - Disable a cookie + * @cookie: The cookie representing the cache object + * @invalidate: Invalidate the backing object + * + * Disable a cookie from accepting further alloc, read, write, invalidate, + * update or acquire operations. Outstanding operations can still be waited + * upon and pages can still be uncached and the cookie relinquished. + * + * This will not return until all outstanding operations have completed. + * + * If @invalidate is set, then the backing object will be invalidated and + * detached, otherwise it will just be detached. + */ +static inline +void fscache_disable_cookie(struct fscache_cookie *cookie, bool invalidate) +{ + if (fscache_cookie_valid(cookie) && fscache_cookie_enabled(cookie)) + __fscache_disable_cookie(cookie, invalidate); +} + +/** + * fscache_enable_cookie - Reenable a cookie + * @cookie: The cookie representing the cache object + * @can_enable: A function to permit enablement once lock is held + * @data: Data for can_enable() + * + * Reenable a previously disabled cookie, allowing it to accept further alloc, + * read, write, invalidate, update or acquire operations. An attempt will be + * made to immediately reattach the cookie to a backing object. + * + * The can_enable() function is called (if not NULL) once the enablement lock + * is held to rule on whether enablement is still permitted to go ahead. + */ +static inline +void fscache_enable_cookie(struct fscache_cookie *cookie, + bool (*can_enable)(void *data), + void *data) +{ + if (fscache_cookie_valid(cookie) && !fscache_cookie_enabled(cookie)) + __fscache_enable_cookie(cookie, can_enable, data); +} + #endif /* _LINUX_FSCACHE_H */ -- cgit v1.2.3 From f1fe29b4a02d0805aa7d0ff6b73410a9f9316d69 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 27 Sep 2013 11:20:03 +0100 Subject: NFS: Use i_writecount to control whether to get an fscache cookie in nfs_open() Use i_writecount to control whether to get an fscache cookie in nfs_open() as NFS does not do write caching yet. I *think* this is the cause of a problem encountered by Mark Moseley whereby __fscache_uncache_page() gets a NULL pointer dereference because cookie->def is NULL: BUG: unable to handle kernel NULL pointer dereference at 0000000000000010 IP: [] __fscache_uncache_page+0x23/0x160 PGD 0 Thread overran stack, or stack corrupted Oops: 0000 [#1] SMP Modules linked in: ... CPU: 7 PID: 18993 Comm: php Not tainted 3.11.1 #1 Hardware name: Dell Inc. PowerEdge R420/072XWF, BIOS 1.3.5 08/21/2012 task: ffff8804203460c0 ti: ffff880420346640 RIP: 0010:[] __fscache_uncache_page+0x23/0x160 RSP: 0018:ffff8801053af878 EFLAGS: 00210286 RAX: 0000000000000000 RBX: ffff8800be2f8780 RCX: ffff88022ffae5e8 RDX: 0000000000004c66 RSI: ffffea00055ff440 RDI: ffff8800be2f8780 RBP: ffff8801053af898 R08: 0000000000000001 R09: 0000000000000003 R10: 0000000000000000 R11: 0000000000000000 R12: ffffea00055ff440 R13: 0000000000001000 R14: ffff8800c50be538 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffff88042fc60000(0063) knlGS:00000000e439c700 CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033 CR2: 0000000000000010 CR3: 0000000001d8f000 CR4: 00000000000607f0 Stack: ... Call Trace: [] __nfs_fscache_invalidate_page+0x42/0x70 [] nfs_invalidate_page+0x75/0x90 [] truncate_inode_page+0x8e/0x90 [] truncate_inode_pages_range.part.12+0x14d/0x620 [] ? __mutex_lock_slowpath+0x1fd/0x2e0 [] truncate_inode_pages_range+0x53/0x70 [] truncate_inode_pages+0x2d/0x40 [] truncate_pagecache+0x4f/0x70 [] nfs_setattr_update_inode+0xa0/0x120 [] nfs3_proc_setattr+0xc4/0xe0 [] nfs_setattr+0xc8/0x150 [] notify_change+0x1cb/0x390 [] do_truncate+0x7b/0xc0 [] do_last+0xa4c/0xfd0 [] path_openat+0xcc/0x670 [] do_filp_open+0x4e/0xb0 [] do_sys_open+0x13f/0x2b0 [] compat_SyS_open+0x36/0x50 [] sysenter_dispatch+0x7/0x24 The code at the instruction pointer was disassembled: > (gdb) disas __fscache_uncache_page > Dump of assembler code for function __fscache_uncache_page: > ... > 0xffffffff812a18ff <+31>: mov 0x48(%rbx),%rax > 0xffffffff812a1903 <+35>: cmpb $0x0,0x10(%rax) > 0xffffffff812a1907 <+39>: je 0xffffffff812a19cd <__fscache_uncache_page+237> These instructions make up: ASSERTCMP(cookie->def->type, !=, FSCACHE_COOKIE_TYPE_INDEX); That cmpb is the faulting instruction (%rax is 0). So cookie->def is NULL - which presumably means that the cookie has already been at least partway through __fscache_relinquish_cookie(). What I think may be happening is something like a three-way race on the same file: PROCESS 1 PROCESS 2 PROCESS 3 =============== =============== =============== open(O_TRUNC|O_WRONLY) open(O_RDONLY) open(O_WRONLY) -->nfs_open() -->nfs_fscache_set_inode_cookie() nfs_fscache_inode_lock() nfs_fscache_disable_inode_cookie() __fscache_relinquish_cookie() nfs_inode->fscache = NULL <--nfs_fscache_set_inode_cookie() -->nfs_open() -->nfs_fscache_set_inode_cookie() nfs_fscache_inode_lock() nfs_fscache_enable_inode_cookie() __fscache_acquire_cookie() nfs_inode->fscache = cookie <--nfs_fscache_set_inode_cookie() <--nfs_open() -->nfs_setattr() ... ... -->nfs_invalidate_page() -->__nfs_fscache_invalidate_page() cookie = nfsi->fscache -->nfs_open() -->nfs_fscache_set_inode_cookie() nfs_fscache_inode_lock() nfs_fscache_disable_inode_cookie() -->__fscache_relinquish_cookie() -->__fscache_uncache_page(cookie) <--__fscache_relinquish_cookie() nfs_inode->fscache = NULL <--nfs_fscache_set_inode_cookie() What is needed is something to prevent process #2 from reacquiring the cookie - and I think checking i_writecount should do the trick. It's also possible to have a two-way race on this if the file is opened O_TRUNC|O_RDONLY instead. Reported-by: Mark Moseley Signed-off-by: David Howells --- include/linux/fs.h | 5 +++++ include/linux/nfs_fs.h | 8 ++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) (limited to 'include/linux') diff --git a/include/linux/fs.h b/include/linux/fs.h index 3f40547ba191..955dff5da56a 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2292,6 +2292,11 @@ static inline void allow_write_access(struct file *file) if (file) atomic_inc(&file_inode(file)->i_writecount); } +static inline bool inode_is_open_for_write(const struct inode *inode) +{ + return atomic_read(&inode->i_writecount) > 0; +} + #ifdef CONFIG_IMA static inline void i_readcount_dec(struct inode *inode) { diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index 3ea4cde8701c..14a48207a304 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -269,9 +269,13 @@ static inline int NFS_STALE(const struct inode *inode) return test_bit(NFS_INO_STALE, &NFS_I(inode)->flags); } -static inline int NFS_FSCACHE(const struct inode *inode) +static inline struct fscache_cookie *nfs_i_fscache(struct inode *inode) { - return test_bit(NFS_INO_FSCACHE, &NFS_I(inode)->flags); +#ifdef CONFIG_NFS_FSCACHE + return NFS_I(inode)->fscache; +#else + return NULL; +#endif } static inline __u64 NFS_FILEID(const struct inode *inode) -- cgit v1.2.3