summaryrefslogtreecommitdiffstats
path: root/fs/fscache/page.c
diff options
context:
space:
mode:
authorDavid Howells <dhowells@redhat.com>2009-11-19 18:11:25 +0000
committerDavid Howells <dhowells@redhat.com>2009-11-19 18:11:25 +0000
commit1bccf513ac49d44604ba1cddcc29f5886e70f1b6 (patch)
tree096ba75a3d02018c5f6e1857aaf1d41471733850 /fs/fscache/page.c
parent6897e3df8fc37bd4a58bbcdef8306da7fc175584 (diff)
downloadop-kernel-dev-1bccf513ac49d44604ba1cddcc29f5886e70f1b6.zip
op-kernel-dev-1bccf513ac49d44604ba1cddcc29f5886e70f1b6.tar.gz
FS-Cache: Fix lock misorder in fscache_write_op()
FS-Cache has two structs internally for keeping track of the internal state of a cached file: the fscache_cookie struct, which represents the netfs's state, and fscache_object struct, which represents the cache's state. Each has a pointer that points to the other (when both are in existence), and each has a spinlock for pointer maintenance. Since netfs operations approach these structures from the cookie side, they get the cookie lock first, then the object lock. Cache operations, on the other hand, approach from the object side, and get the object lock first. It is not then permitted for a cache operation to get the cookie lock whilst it is holding the object lock lest deadlock occur; instead, it must do one of two things: (1) increment the cookie usage counter, drop the object lock and then get both locks in order, or (2) simply hold the object lock as certain parts of the cookie may not be altered whilst the object lock is held. It is also not permitted to follow either pointer without holding the lock at the end you start with. To break the pointers between the cookie and the object, both locks must be held. fscache_write_op(), however, violates the locking rules: It attempts to get the cookie lock without (a) checking that the cookie pointer is a valid pointer, and (b) holding the object lock to protect the cookie pointer whilst it follows it. This is so that it can access the pending page store tree without interference from __fscache_write_page(). This is fixed by splitting the cookie lock, such that the page store tracking tree is protected by its own lock, and checking that the cookie pointer is non-NULL before we attempt to follow it whilst holding the object lock. The new lock is subordinate to both the cookie lock and the object lock, and so should be taken after those. Signed-off-by: David Howells <dhowells@redhat.com>
Diffstat (limited to 'fs/fscache/page.c')
-rw-r--r--fs/fscache/page.c52
1 files changed, 35 insertions, 17 deletions
diff --git a/fs/fscache/page.c b/fs/fscache/page.c
index e6f2e61..3ea8897 100644
--- a/fs/fscache/page.c
+++ b/fs/fscache/page.c
@@ -45,16 +45,26 @@ EXPORT_SYMBOL(__fscache_wait_on_page_write);
/*
* note that a page has finished being written to the cache
*/
-static void fscache_end_page_write(struct fscache_cookie *cookie, struct page *page)
+static void fscache_end_page_write(struct fscache_object *object,
+ struct page *page)
{
- struct page *xpage;
+ struct fscache_cookie *cookie;
+ struct page *xpage = NULL;
- spin_lock(&cookie->lock);
- xpage = radix_tree_delete(&cookie->stores, page->index);
- spin_unlock(&cookie->lock);
- ASSERT(xpage != NULL);
-
- wake_up_bit(&cookie->flags, 0);
+ spin_lock(&object->lock);
+ cookie = object->cookie;
+ if (cookie) {
+ /* delete the page from the tree if it is now no longer
+ * pending */
+ spin_lock(&cookie->stores_lock);
+ fscache_stat(&fscache_n_store_radix_deletes);
+ xpage = radix_tree_delete(&cookie->stores, page->index);
+ spin_unlock(&cookie->stores_lock);
+ wake_up_bit(&cookie->flags, 0);
+ }
+ spin_unlock(&object->lock);
+ if (xpage)
+ page_cache_release(xpage);
}
/*
@@ -591,7 +601,7 @@ static void fscache_write_op(struct fscache_operation *_op)
struct fscache_storage *op =
container_of(_op, struct fscache_storage, op);
struct fscache_object *object = op->op.object;
- struct fscache_cookie *cookie = object->cookie;
+ struct fscache_cookie *cookie;
struct page *page;
unsigned n;
void *results[1];
@@ -601,16 +611,17 @@ static void fscache_write_op(struct fscache_operation *_op)
fscache_set_op_state(&op->op, "GetPage");
- spin_lock(&cookie->lock);
spin_lock(&object->lock);
+ cookie = object->cookie;
- if (!fscache_object_is_active(object)) {
+ if (!fscache_object_is_active(object) || !cookie) {
spin_unlock(&object->lock);
- spin_unlock(&cookie->lock);
_leave("");
return;
}
+ spin_lock(&cookie->stores_lock);
+
fscache_stat(&fscache_n_store_calls);
/* find a page to store */
@@ -621,23 +632,25 @@ static void fscache_write_op(struct fscache_operation *_op)
goto superseded;
page = results[0];
_debug("gang %d [%lx]", n, page->index);
- if (page->index > op->store_limit)
+ if (page->index > op->store_limit) {
+ fscache_stat(&fscache_n_store_pages_over_limit);
goto superseded;
+ }
radix_tree_tag_clear(&cookie->stores, page->index,
FSCACHE_COOKIE_PENDING_TAG);
+ spin_unlock(&cookie->stores_lock);
spin_unlock(&object->lock);
- spin_unlock(&cookie->lock);
if (page) {
fscache_set_op_state(&op->op, "Store");
+ fscache_stat(&fscache_n_store_pages);
fscache_stat(&fscache_n_cop_write_page);
ret = object->cache->ops->write_page(op, page);
fscache_stat_d(&fscache_n_cop_write_page);
fscache_set_op_state(&op->op, "EndWrite");
- fscache_end_page_write(cookie, page);
- page_cache_release(page);
+ fscache_end_page_write(object, page);
if (ret < 0) {
fscache_set_op_state(&op->op, "Abort");
fscache_abort_object(object);
@@ -653,9 +666,9 @@ superseded:
/* this writer is going away and there aren't any more things to
* write */
_debug("cease");
+ spin_unlock(&cookie->stores_lock);
clear_bit(FSCACHE_OBJECT_PENDING_WRITE, &object->flags);
spin_unlock(&object->lock);
- spin_unlock(&cookie->lock);
_leave("");
}
@@ -731,6 +744,7 @@ int __fscache_write_page(struct fscache_cookie *cookie,
/* add the page to the pending-storage radix tree on the backing
* object */
spin_lock(&object->lock);
+ spin_lock(&cookie->stores_lock);
_debug("store limit %llx", (unsigned long long) object->store_limit);
@@ -751,6 +765,7 @@ int __fscache_write_page(struct fscache_cookie *cookie,
if (test_and_set_bit(FSCACHE_OBJECT_PENDING_WRITE, &object->flags))
goto already_pending;
+ spin_unlock(&cookie->stores_lock);
spin_unlock(&object->lock);
op->op.debug_id = atomic_inc_return(&fscache_op_debug_id);
@@ -772,6 +787,7 @@ int __fscache_write_page(struct fscache_cookie *cookie,
already_queued:
fscache_stat(&fscache_n_stores_again);
already_pending:
+ spin_unlock(&cookie->stores_lock);
spin_unlock(&object->lock);
spin_unlock(&cookie->lock);
radix_tree_preload_end();
@@ -781,7 +797,9 @@ already_pending:
return 0;
submit_failed:
+ spin_lock(&cookie->stores_lock);
radix_tree_delete(&cookie->stores, page->index);
+ spin_unlock(&cookie->stores_lock);
page_cache_release(page);
ret = -ENOBUFS;
goto nobufs;
OpenPOWER on IntegriCloud