From: JeffleXu <jefflexu@linux.alibaba.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: dhowells@redhat.com, linux-cachefs@redhat.com, xiang@kernel.org,
chao@kernel.org, linux-erofs@lists.ozlabs.org,
torvalds@linux-foundation.org, gregkh@linuxfoundation.org,
linux-fsdevel@vger.kernel.org, joseph.qi@linux.alibaba.com,
bo.liu@linux.alibaba.com, tao.peng@linux.alibaba.com,
gerry@linux.alibaba.com, eguan@linux.alibaba.com,
linux-kernel@vger.kernel.org, luodaowen.backend@bytedance.com
Subject: Re: [PATCH v5 03/22] cachefiles: introduce on-demand read mode
Date: Mon, 21 Mar 2022 22:08:47 +0800 [thread overview]
Message-ID: <6bc551d2-15fc-5d17-c99b-8db588c6b671@linux.alibaba.com> (raw)
In-Reply-To: <YjiAVezd5B9auhcP@casper.infradead.org>
On 3/21/22 9:40 PM, Matthew Wilcox wrote:
> On Wed, Mar 16, 2022 at 09:17:04PM +0800, Jeffle Xu wrote:
>> +#ifdef CONFIG_CACHEFILES_ONDEMAND
>> + struct xarray reqs; /* xarray of pending on-demand requests */
>> + rwlock_t reqs_lock; /* Lock for reqs xarray */
>
> Why do you have a separate rwlock when the xarray already has its own
> spinlock? This is usually a really bad idea.
Hi,
Thanks for reviewing.
reqs_lock is also used to protect the check of cache->flags. Please
refer to patch 4 [1] of this patchset.
```
+ /*
+ * Enqueue the pending request.
+ *
+ * Stop enqueuing the request when daemon is dying. So we need to
+ * 1) check cache state, and 2) enqueue request if cache is alive.
+ *
+ * The above two ops need to be atomic as a whole. @reqs_lock is used
+ * here to ensure that. Otherwise, request may be enqueued after xarray
+ * has been flushed, in which case the orphan request will never be
+ * completed and thus netfs will hang there forever.
+ */
+ read_lock(&cache->reqs_lock);
+
+ /* recheck dead state under lock */
+ if (test_bit(CACHEFILES_DEAD, &cache->flags)) {
+ read_unlock(&cache->reqs_lock);
+ ret = -EIO;
+ goto out;
+ }
+
+ xa_lock(xa);
+ ret = __xa_alloc(xa, &id, req, xa_limit_32b, GFP_KERNEL);
+ if (!ret)
+ __xa_set_mark(xa, id, CACHEFILES_REQ_NEW);
+ xa_unlock(xa);
+
+ read_unlock(&cache->reqs_lock);
```
It's mainly used to protect against the xarray flush.
Besides, IMHO read-write lock shall be more performance friendly, since
most cases are the read side.
[1] https://lkml.org/lkml/2022/3/16/351
--
Thanks,
Jeffle
next prev parent reply other threads:[~2022-03-21 14:15 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-16 13:17 [PATCH v5 00/22] fscache,erofs: fscache-based on-demand read semantics Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 01/22] fscache: export fscache_end_operation() Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 02/22] cachefiles: extract write routine Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 03/22] cachefiles: introduce on-demand read mode Jeffle Xu
2022-03-21 13:40 ` Matthew Wilcox
2022-03-21 14:08 ` JeffleXu [this message]
2022-03-21 14:26 ` Matthew Wilcox
2022-03-21 15:18 ` JeffleXu
2022-03-21 15:21 ` Matthew Wilcox
2022-03-21 15:30 ` David Howells
2022-03-22 17:04 ` Matthew Wilcox
2022-03-23 5:32 ` JeffleXu
2022-03-21 14:14 ` David Howells
2022-03-21 14:20 ` [Linux-cachefs] " Gao Xiang
2022-03-16 13:17 ` [PATCH v5 04/22] cachefiles: notify user daemon with anon_fd when looking up cookie Jeffle Xu
2022-03-16 19:37 ` kernel test robot
2022-03-16 13:17 ` [PATCH v5 05/22] cachefiles: notify user daemon when withdrawing cookie Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 06/22] cachefiles: implement on-demand read Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 07/22] cachefiles: document on-demand read mode Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 08/22] erofs: use meta buffers for erofs_read_superblock() Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 09/22] erofs: make erofs_map_blocks() generally available Jeffle Xu
2022-03-17 5:35 ` [Linux-cachefs] " Gao Xiang
2022-03-16 13:17 ` [PATCH v5 10/22] erofs: add mode checking helper Jeffle Xu
2022-03-17 5:36 ` [Linux-cachefs] " Gao Xiang
2022-03-18 5:26 ` JeffleXu
2022-03-16 13:17 ` [PATCH v5 11/22] erofs: register global fscache volume Jeffle Xu
2022-03-16 21:52 ` kernel test robot
2022-03-17 1:49 ` kernel test robot
2022-03-16 13:17 ` [PATCH v5 12/22] erofs: add cookie context helper functions Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 13/22] erofs: add anonymous inode managing page cache of blob file Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 14/22] erofs: add erofs_fscache_read_pages() helper Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 15/22] erofs: register cookie context for bootstrap blob Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 16/22] erofs: implement fscache-based metadata read Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 17/22] erofs: implement fscache-based data read for non-inline layout Jeffle Xu
2022-03-17 6:18 ` [Linux-cachefs] " Gao Xiang
2022-03-18 5:29 ` JeffleXu
2022-03-16 13:17 ` [PATCH v5 18/22] erofs: implement fscache-based data read for inline layout Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 19/22] erofs: register cookie context for data blobs Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 20/22] erofs: implement fscache-based data read " Jeffle Xu
2022-03-16 13:17 ` [PATCH v5 21/22] erofs: implement fscache-based data readahead Jeffle Xu
2022-03-17 5:22 ` Gao Xiang
2022-03-18 5:41 ` JeffleXu
2022-03-16 13:17 ` [PATCH v5 22/22] erofs: add 'uuid' mount option Jeffle Xu
2022-03-21 13:34 ` [PATCH v5 03/22] cachefiles: introduce on-demand read mode David Howells
2022-03-21 14:16 ` JeffleXu
2022-03-21 14:01 ` [PATCH v5 04/22] cachefiles: notify user daemon with anon_fd when looking up cookie David Howells
2022-03-21 14:43 ` JeffleXu
2022-03-21 14:05 ` [PATCH v5 06/22] cachefiles: implement on-demand read David Howells
2022-03-21 14:51 ` JeffleXu
2022-03-21 14:20 ` [PATCH v5 05/22] cachefiles: notify user daemon when withdrawing cookie David Howells
2022-03-21 14:31 ` JeffleXu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6bc551d2-15fc-5d17-c99b-8db588c6b671@linux.alibaba.com \
--to=jefflexu@linux.alibaba.com \
--cc=bo.liu@linux.alibaba.com \
--cc=chao@kernel.org \
--cc=dhowells@redhat.com \
--cc=eguan@linux.alibaba.com \
--cc=gerry@linux.alibaba.com \
--cc=gregkh@linuxfoundation.org \
--cc=joseph.qi@linux.alibaba.com \
--cc=linux-cachefs@redhat.com \
--cc=linux-erofs@lists.ozlabs.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luodaowen.backend@bytedance.com \
--cc=tao.peng@linux.alibaba.com \
--cc=torvalds@linux-foundation.org \
--cc=willy@infradead.org \
--cc=xiang@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).