Don't you think the Cc list is a bit overloaded?
On Wed, Dec 25, 2024 at 05:42:02PM +0800, WangYuli wrote:
> When a user calls the read/write system call and passes a pipe
> descriptor, the pipe_read/pipe_write functions are invoked:
>
> 1. pipe_read():
> 1). Checks if the pipe is valid and if there is any data in the
> pipe buffer.
> 2). Waits for data:
> *If there is no data in the pipe and the write end is still open,
> the current process enters a sleep state (wait_event()) until data
> is written.
> *If the write end is closed, return 0.
> 3). Reads data:
> *Wakes up the process and copies data from the pipe's memory
> buffer to user space.
> *When the buffer is full, the writing process will go to sleep,
> waiting for the pipe state to change to be awakened (using the
> wake_up_interruptible_sync_poll() mechanism). Once data is read
> from the buffer, the writing process can continue writing, and the
> reading process can continue reading new data.
> 4). Returns the number of bytes read upon successful read.
>
> 2. pipe_write():
> 1). Checks if the pipe is valid and if there is any available
> space in the pipe buffer.
> 2). Waits for buffer space:
> *If the pipe buffer is full and the reading process has not
> read any data, pipe_write() may put the current process to sleep
> until there is space in the buffer.
> *If the read end of the pipe is closed (no process is waiting
> to read), an error code -EPIPE is returned, and a SIGPIPE signal may
> be sent to the process.
> 3). Writes data:
> *If there is enough space in the pipe buffer, pipe_write() copies
> data from the user space buffer to the kernel buffer of the pipe
> (using copy_from_user()).
> *If the amount of data the user requests to write is larger than
> the available space in the buffer, multiple writes may be required,
> or the process may wait for new space to be freed.
> 4). Wakes up waiting reading processes:
> *After the data is successfully written, pipe_write() wakes up
> any processes that may be waiting to read data (using the
> wake_up_interruptible_sync_poll() mechanism).
> 5). Returns the number of bytes successfully written.
>
> Check if there are any waiting processes in the process wait queue
> by introducing wq_has_sleeper() when waking up processes for pipe
> read/write operations.
>
> If no processes are waiting, there's no need to execute
> wake_up_interruptible_sync_poll(), thus avoiding unnecessary wake-ups.
>
> Unnecessary wake-ups can lead to context switches, where a process
> is woken up to handle I/O events even when there is no immediate
> need.
>
> Only wake up processes when there are actually waiting processes to
> reduce context switches and system overhead by checking
> with wq_has_sleeper().
>
> Additionally, by reducing unnecessary synchronization and wake-up
> operations, wq_has_sleeper() can decrease system resource waste and
> lock contention, improving overall system performance.
>
> For pipe read/write operations, this eliminates ineffective scheduling
> and enhances concurrency.
>
> It's important to note that enabling this option means invoking
> wq_has_sleeper() to check for sleeping processes in the wait queue
> for every read or write operation.
>
> While this is a lightweight operation, it still incurs some overhead.
>
> In low-load or single-task scenarios, this overhead may not yield
> significant benefits and could even introduce minor performance
> degradation.
>
> UnixBench Pipe benchmark results on Zhaoxin KX-U6780A processor:
>
> With the option disabled: Single-core: 841.8, Multi-core (8): 4621.6
> With the option enabled: Single-core: 877.8, Multi-core (8): 4854.7
>
> Single-core performance improved by 4.1%, multi-core performance
> improved by 4.8%.
...
> +config PIPE_SKIP_SLEEPER
> + bool "Skip sleeping processes during pipe read/write"
> + default n
'n' is the default 'default', no need to have this line.
> + help
> + This option introduces a check whether the sleep queue will
> + be awakened during pipe read/write.
> +
> + It often leads to a performance improvement. However, in
> + low-load or single-task scenarios, it may introduce minor
> + performance overhead.
> + If unsure, say N.
Illogical, it's already N as you stated by putting a redundant line, but after
removing that line it will make sense.
...
> +static inline bool
Have you build this with Clang and `make W=1 ...`?
> +pipe_check_wq_has_sleeper(struct wait_queue_head *wq_head)
> +{
> + if (IS_ENABLED(CONFIG_PIPE_SKIP_SLEEPER))
> + return wq_has_sleeper(wq_head);
> + else
Redundant.
> + return true;
if (!foo)
return true;
return bar(...);
> +}
--
With Best Regards,
Andy Shevchenko
