linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* confusing fscache path
@ 2021-07-08 18:09 Steve French
  2021-07-12 13:20 ` David Howells
  0 siblings, 1 reply; 2+ messages in thread
From: Steve French @ 2021-07-08 18:09 UTC (permalink / raw)
  To: David Howells; +Cc: CIFS, linux-fsdevel

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: confusing fscache path
  2021-07-08 18:09 confusing fscache path Steve French
@ 2021-07-12 13:20 ` David Howells
  0 siblings, 0 replies; 2+ messages in thread
From: David Howells @ 2021-07-12 13:20 UTC (permalink / raw)
  To: Steve French; +Cc: dhowells, CIFS, linux-fsdevel

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-07-12 13:20 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-08 18:09 confusing fscache path Steve French
2021-07-12 13:20 ` David Howells

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox