From: Matthew Wilcox <willy@infradead.org>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Amir Goldstein <amir73il@gmail.com>,
Dave Chinner <david@fromorbit.com>,
linux-xfs <linux-xfs@vger.kernel.org>,
Christoph Hellwig <hch@lst.de>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [QUESTION] Long read latencies on mixed rw buffered IO
Date: Mon, 25 Mar 2019 09:41:29 -0700 [thread overview]
Message-ID: <20190325164129.GH10344@bombadil.infradead.org> (raw)
In-Reply-To: <20190325154731.GT1183@magnolia>
On Mon, Mar 25, 2019 at 08:47:31AM -0700, Darrick J. Wong wrote:
> Hmmm.... so it looks like the rw_semaphore behavior has shifted over
> time, then?
Yes.
> I thought rwsem was supposed to queue read and write waiters in order,
> at least on x86? Though I suppose that might not matter much since we
> can only run one writer at a time vs. waking up all the readers at once.
> Now I'm wondering if there ever was a time when the readers all got
> batched to the front and starved the writers, but eh I haven't drank
> enough coffee to remember things like that. :P
>
> (I wonder what would happen if rw_semaphore decided to wake up some
> number of the readers in the rwsem wait_list, not just the ones at the
> front...)
rwsems currently allow a limited amount of queue-jumping; if a semaphore
is currently not acquired (it's in transition between two owners), a
running process can acquire it.
I think it is a bug that we only wake readers at the front of the queue;
I think we would get better performance if we wake all readers. ie here:
/*
- * Grant an infinite number of read locks to the readers at the front
- * of the queue. We know that woken will be at least 1 as we accounted
+ * Grant an infinite number of read locks. We know that woken will
+ * be at least 1 as we accounted
* for above. Note we increment the 'active part' of the count by the
* number of readers before waking any processes up.
*/
list_for_each_entry_safe(waiter, tmp, &sem->wait_list, list) {
struct task_struct *tsk;
- if (waiter->type == RWSEM_WAITING_FOR_WRITE)
- break;
Amir, it seems like you have a good test-case for trying this out ...
next prev parent reply other threads:[~2019-03-25 16:41 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAOQ4uxi0pGczXBX7GRAFs88Uw0n1ERJZno3JSeZR71S1dXg+2w@mail.gmail.com>
[not found] ` <20190325001044.GA23020@dastard>
2019-03-25 7:49 ` [QUESTION] Long read latencies on mixed rw buffered IO Amir Goldstein
2019-03-25 15:47 ` Darrick J. Wong
2019-03-25 16:41 ` Matthew Wilcox [this message]
2019-03-25 17:30 ` Amir Goldstein
2019-03-25 18:22 ` Matthew Wilcox
2019-03-25 19:18 ` Amir Goldstein
2019-03-25 19:40 ` Matthew Wilcox
2019-03-25 19:57 ` Amir Goldstein
2019-03-25 23:48 ` Dave Chinner
2019-03-26 3:44 ` Amir Goldstein
2019-03-27 1:29 ` Dave Chinner
2019-03-25 17:56 ` Amir Goldstein
2019-03-25 18:02 ` Christoph Hellwig
2019-03-25 18:44 ` Amir Goldstein
2019-03-25 23:43 ` Dave Chinner
2019-03-26 4:36 ` Amir Goldstein
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=20190325164129.GH10344@bombadil.infradead.org \
--to=willy@infradead.org \
--cc=amir73il@gmail.com \
--cc=darrick.wong@oracle.com \
--cc=david@fromorbit.com \
--cc=hch@lst.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-xfs@vger.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).