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 23:18:05 +0800 [thread overview]
Message-ID: <adb957da-8909-06d8-1b2c-b8a293b37930@linux.alibaba.com> (raw)
In-Reply-To: <YjiLACenpRV4XTcs@casper.infradead.org>
On 3/21/22 10:26 PM, Matthew Wilcox wrote:
> On Mon, Mar 21, 2022 at 10:08:47PM +0800, JeffleXu wrote:
>> reqs_lock is also used to protect the check of cache->flags. Please
>> refer to patch 4 [1] of this patchset.
>
> Yes, that's exactly what I meant by "bad idea".
>
>> ```
>> + /*
>> + * 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;
>> + }
>
> So this is an error path. We're almost always going to take the xa_lock
> immediately after taking the read_lock. In other words, you've done two
> atomic operations instead of one.
Right.
>
>> + 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.
>
> That's almost never true. rwlocks are usually a bad idea because you
> still have to bounce the cacheline, so you replace lock contention
> (which you can see) with cacheline contention (which is harder to
> measure). And then you have questions about reader/writer fairness
> (should new readers queue behind a writer if there's one waiting, or
> should a steady stream of readers be able to hold a writer off
> indefinitely?)
Interesting, I didn't notice it before. Thanks for explaining it.
BTW what I want is just
```
PROCESS 1 PROCESS 2
========= =========
#lock #lock
set DEAD state if (not DEAD)
flush xarray enqueue into xarray
#unlock #unlock
```
I think it is a generic paradigm. So it seems that the spinlock inside
xarray is already adequate for this job?
--
Thanks,
Jeffle
next prev parent reply other threads:[~2022-03-21 15:19 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
2022-03-21 14:26 ` Matthew Wilcox
2022-03-21 15:18 ` JeffleXu [this message]
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=adb957da-8909-06d8-1b2c-b8a293b37930@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).