From: Hongchen Zhang <zhanghongchen@loongson.cn>
To: Alexander Viro <viro@zeniv.linux.org.uk>,
Andrew Morton <akpm@linux-foundation.org>,
Kuniyuki Iwashima <kuniyu@amazon.co.jp>,
Hongchen Zhang <zhanghongchen@loongson.cn>,
Luis Chamberlain <mcgrof@kernel.org>,
David Howells <dhowells@redhat.com>,
Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
Randy Dunlap <rdunlap@infradead.org>,
Eric Dumazet <edumazet@google.com>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v3] pipe: use __pipe_{lock,unlock} instead of spinlock
Date: Fri, 6 Jan 2023 17:28:53 +0800 [thread overview]
Message-ID: <20230106092853.26038-1-zhanghongchen@loongson.cn> (raw)
Use spinlock in pipe_read/write cost too much time,IMO
pipe->{head,tail} can be protected by __pipe_{lock,unlock}.
On the other hand, we can use __pipe_{lock,unlock} to protect
the pipe->{head,tail} in pipe_resize_ring and
post_one_notification.
I tested this patch using UnixBench's pipe test case on a x86_64
machine,and get the following data:
1) before this patch
System Benchmarks Partial Index BASELINE RESULT INDEX
Pipe Throughput 12440.0 493023.3 396.3
========
System Benchmarks Index Score (Partial Only) 396.3
2) after this patch
System Benchmarks Partial Index BASELINE RESULT INDEX
Pipe Throughput 12440.0 507551.4 408.0
========
System Benchmarks Index Score (Partial Only) 408.0
so we get ~3% speedup.
Signed-off-by: Hongchen Zhang <zhanghongchen@loongson.cn>
---
fs/pipe.c | 22 +---------------------
include/linux/pipe_fs_i.h | 10 ++++++++++
kernel/watch_queue.c | 8 ++++----
3 files changed, 15 insertions(+), 25 deletions(-)
diff --git a/fs/pipe.c b/fs/pipe.c
index 42c7ff41c2db..4355ee5f754e 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -98,16 +98,6 @@ void pipe_unlock(struct pipe_inode_info *pipe)
}
EXPORT_SYMBOL(pipe_unlock);
-static inline void __pipe_lock(struct pipe_inode_info *pipe)
-{
- mutex_lock_nested(&pipe->mutex, I_MUTEX_PARENT);
-}
-
-static inline void __pipe_unlock(struct pipe_inode_info *pipe)
-{
- mutex_unlock(&pipe->mutex);
-}
-
void pipe_double_lock(struct pipe_inode_info *pipe1,
struct pipe_inode_info *pipe2)
{
@@ -253,8 +243,7 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
*/
was_full = pipe_full(pipe->head, pipe->tail, pipe->max_usage);
for (;;) {
- /* Read ->head with a barrier vs post_one_notification() */
- unsigned int head = smp_load_acquire(&pipe->head);
+ unsigned int head = pipe->head;
unsigned int tail = pipe->tail;
unsigned int mask = pipe->ring_size - 1;
@@ -322,14 +311,12 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
if (!buf->len) {
pipe_buf_release(pipe, buf);
- spin_lock_irq(&pipe->rd_wait.lock);
#ifdef CONFIG_WATCH_QUEUE
if (buf->flags & PIPE_BUF_FLAG_LOSS)
pipe->note_loss = true;
#endif
tail++;
pipe->tail = tail;
- spin_unlock_irq(&pipe->rd_wait.lock);
}
total_len -= chars;
if (!total_len)
@@ -506,16 +493,13 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
* it, either the reader will consume it or it'll still
* be there for the next write.
*/
- spin_lock_irq(&pipe->rd_wait.lock);
head = pipe->head;
if (pipe_full(head, pipe->tail, pipe->max_usage)) {
- spin_unlock_irq(&pipe->rd_wait.lock);
continue;
}
pipe->head = head + 1;
- spin_unlock_irq(&pipe->rd_wait.lock);
/* Insert it into the buffer array */
buf = &pipe->bufs[head & mask];
@@ -1260,14 +1244,12 @@ int pipe_resize_ring(struct pipe_inode_info *pipe, unsigned int nr_slots)
if (unlikely(!bufs))
return -ENOMEM;
- spin_lock_irq(&pipe->rd_wait.lock);
mask = pipe->ring_size - 1;
head = pipe->head;
tail = pipe->tail;
n = pipe_occupancy(head, tail);
if (nr_slots < n) {
- spin_unlock_irq(&pipe->rd_wait.lock);
kfree(bufs);
return -EBUSY;
}
@@ -1303,8 +1285,6 @@ int pipe_resize_ring(struct pipe_inode_info *pipe, unsigned int nr_slots)
pipe->tail = tail;
pipe->head = head;
- spin_unlock_irq(&pipe->rd_wait.lock);
-
/* This might have made more room for writers */
wake_up_interruptible(&pipe->wr_wait);
return 0;
diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
index 6cb65df3e3ba..baae3d062422 100644
--- a/include/linux/pipe_fs_i.h
+++ b/include/linux/pipe_fs_i.h
@@ -223,6 +223,16 @@ static inline void pipe_discard_from(struct pipe_inode_info *pipe,
#define PIPE_SIZE PAGE_SIZE
/* Pipe lock and unlock operations */
+static inline void __pipe_lock(struct pipe_inode_info *pipe)
+{
+ mutex_lock_nested(&pipe->mutex, I_MUTEX_PARENT);
+}
+
+static inline void __pipe_unlock(struct pipe_inode_info *pipe)
+{
+ mutex_unlock(&pipe->mutex);
+}
+
void pipe_lock(struct pipe_inode_info *);
void pipe_unlock(struct pipe_inode_info *);
void pipe_double_lock(struct pipe_inode_info *, struct pipe_inode_info *);
diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c
index a6f9bdd956c3..92e46cfe9419 100644
--- a/kernel/watch_queue.c
+++ b/kernel/watch_queue.c
@@ -108,7 +108,7 @@ static bool post_one_notification(struct watch_queue *wqueue,
if (!pipe)
return false;
- spin_lock_irq(&pipe->rd_wait.lock);
+ __pipe_lock(pipe);
mask = pipe->ring_size - 1;
head = pipe->head;
@@ -135,17 +135,17 @@ static bool post_one_notification(struct watch_queue *wqueue,
buf->offset = offset;
buf->len = len;
buf->flags = PIPE_BUF_FLAG_WHOLE;
- smp_store_release(&pipe->head, head + 1); /* vs pipe_read() */
+ pipe->head = head + 1;
if (!test_and_clear_bit(note, wqueue->notes_bitmap)) {
- spin_unlock_irq(&pipe->rd_wait.lock);
+ __pipe_unlock(pipe);
BUG();
}
wake_up_interruptible_sync_poll_locked(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
done = true;
out:
- spin_unlock_irq(&pipe->rd_wait.lock);
+ __pipe_unlock(pipe);
if (done)
kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
return done;
--
2.34.1
next reply other threads:[~2023-01-06 9:38 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-06 9:28 Hongchen Zhang [this message]
2023-01-06 9:48 [PATCH v3] pipe: use __pipe_{lock,unlock} instead of spinlock Hongchen Zhang
2023-01-06 19:13 ` Luis Chamberlain
2023-01-06 20:33 ` Sedat Dilek
2023-01-07 3:31 ` Hongchen Zhang
2023-01-07 0:58 ` Hongchen Zhang
2023-01-07 1:23 Hongchen Zhang
2023-01-13 3:19 ` Hongchen Zhang
2023-01-13 9:32 ` Sedat Dilek
2023-01-16 1:52 ` Sedat Dilek
2023-01-16 2:16 ` Hongchen Zhang
2023-01-16 2:42 ` Sedat Dilek
2023-01-16 3:16 ` maobibo
2023-01-16 4:38 ` Matthew Wilcox
2023-01-16 21:10 ` Al Viro
2023-01-16 22:16 ` Andrew Morton
2023-01-17 6:54 ` Sedat Dilek
2023-01-29 2:29 ` Hongchen Zhang
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=20230106092853.26038-1-zhanghongchen@loongson.cn \
--to=zhanghongchen@loongson.cn \
--cc=akpm@linux-foundation.org \
--cc=christophe.jaillet@wanadoo.fr \
--cc=dhowells@redhat.com \
--cc=edumazet@google.com \
--cc=kuniyu@amazon.co.jp \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=rdunlap@infradead.org \
--cc=viro@zeniv.linux.org.uk \
/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).