Re: [PATCH v3] pipe: use __pipe_{lock,unlock} instead of spinlock

From: Hongchen Zhang
Date: Sat Jan 28 2023 - 21:29:34 EST


Hi Andrew,

Sorry to reply to you so late, because I took a long holiday.

On 2023/1/17 am 6:16, Andrew Morton wrote:
On Mon, 16 Jan 2023 21:10:37 +0000 Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:

On Mon, Jan 16, 2023 at 04:38:01AM +0000, Matthew Wilcox wrote:
On Mon, Jan 16, 2023 at 11:16:13AM +0800, maobibo wrote:
Hongchen,

I have a glance with this patch, it simply replaces with
spinlock_irqsave with mutex lock. There may be performance
improvement with two processes competing with pipe, however
for N processes, there will be complex context switches
and ipi interruptts.

Can you find some cases with more than 2 processes competing
pipe, rather than only unixbench?

What real applications have pipes with more than 1 writer & 1 reader?
I'm OK with slowing down the weird cases if the common cases go faster.

>From commit 0ddad21d3e99c743a3aa473121dc5561679e26bb:
While this isn't a common occurrence in the traditional "use a pipe as a
data transport" case, where you typically only have a single reader and
a single writer process, there is one common special case: using a pipe
as a source of "locking tokens" rather than for data communication.
In particular, the GNU make jobserver code ends up using a pipe as a way
to limit parallelism, where each job consumes a token by reading a byte
from the jobserver pipe, and releases the token by writing a byte back
to the pipe.

The author has tested this patch with Linus's test code from 0ddad21d3e
and the results were OK
(https://lkml.kernel.org/r/c3cbede6-f19e-3333-ba0f-d3f005e5d599@xxxxxxxxxxx).

I've been stalling on this patch until Linus gets back to his desk,
which now appears to have happened.

Hongchen, when convenient, please capture this discussion (as well as
the testing results with Linus's sample code) in the changelog and send
us a v4, with Linus on cc?

I will send you a v4 and cc to Linus.

Thanks.
Hongchen Zhang