confusing fscache path

confusing fscache path

[Steve French]

Coverity complains about a double lock in fscache code and it may be
right.  Can you explain this path? Or is there a double lock bug?

line 190 of fs/cifs/fscache.c calls fscache_uncache_all_inode_pages
(which locks cifsi->fscache->lock) but then immediately calls
fscache_relinquish_cookie which locks it again.

Is Coverity ((and I) missing something?

The path from fscache_uncache_all_inode_pages is:
   fscache_uncache_all_inode_pages-->__fscache_uncache_all_inode_pages
(line 132 of fs/fscache/page.c) -->__fscache_uncache_page (locks on
line 1120 and then on line 1141 "goto done" without unlocking it)

But it gets locked again in:
    __fscache_relinquish_cookie-->__fscache_disable_cookie
    (on line 749 of fs/fscache/cookie.c)
-- 
Thanks,

Steve

Re: confusing fscache path

[David Howells]

Steve French <smfrench@gmail.com> wrote:

> The path from fscache_uncache_all_inode_pages is:
>    fscache_uncache_all_inode_pages-->__fscache_uncache_all_inode_pages
> (line 132 of fs/fscache/page.c) -->__fscache_uncache_page (locks on
> line 1120 and then on line 1141 "goto done" without unlocking it)

This bit, you mean?

	if (TestClearPageFsCache(page) &&
	    object->cache->ops->uncache_page) {
		/* the cache backend releases the cookie lock */
		fscache_stat(&fscache_n_cop_uncache_page);
		object->cache->ops->uncache_page(object, page);
		fscache_stat_d(&fscache_n_cop_uncache_page);
		goto done;
	}

Note the comment.

Here's the unlock:

	void cachefiles_uncache_page(struct fscache_object *_object, struct page *page)
		__releases(&object->fscache.cookie->lock)
	{
		struct cachefiles_object *object;

		object = container_of(_object, struct cachefiles_object, fscache);

		_enter("%p,{%lu}", object, page->index);

		spin_unlock(&object->fscache.cookie->lock);
	}

David