diff mbox series

[v1] coredump: Use vmsplice_to_pipe() for pipes in dump_emit_page()

Message ID 20221029005147.2553-1-yepeilin.cs@gmail.com
State New
Headers
Series [v1] coredump: Use vmsplice_to_pipe() for pipes in dump_emit_page() |

Commit Message

Peilin Ye Oct. 29, 2022, 12:51 a.m. UTC 
  From: Peilin Ye <peilin.ye@bytedance.com>

Currently, there is a copy for each page when dumping VMAs to pipe
handlers using dump_emit_page().  For example:

  fs/binfmt_elf.c:elf_core_dump()
      fs/coredump.c:dump_user_range()
                     :dump_emit_page()
        fs/read_write.c:__kernel_write_iter()
                fs/pipe.c:pipe_write()
             lib/iov_iter.c:copy_page_from_iter()

Use vmsplice_to_pipe() instead of __kernel_write_iter() to avoid this
copy for pipe handlers.

Tested by dumping a 40-GByte core into a simple handler that splice()s
from stdin to disk in a loop, PIPE_DEF_BUFFERS (16) pages at a time.

                              Before           After   Improved by
  Time to Completion   52.04 seconds   46.30 seconds        11.03%
  CPU Usage                   89.43%          84.90%         5.07%

Suggested-by: Cong Wang <cong.wang@bytedance.com>
Signed-off-by: Peilin Ye <peilin.ye@bytedance.com>
---
 fs/coredump.c          | 7 ++++++-
 fs/splice.c            | 4 ++--
 include/linux/splice.h | 2 ++
 3 files changed, 10 insertions(+), 3 deletions(-)

Comments

kernel test robot Oct. 29, 2022, 7:20 a.m. UTC | #1 
Hi Peilin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.1-rc2 next-20221028]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Peilin-Ye/coredump-Use-vmsplice_to_pipe-for-pipes-in-dump_emit_page/20221029-085307
patch link:    https://lore.kernel.org/r/20221029005147.2553-1-yepeilin.cs%40gmail.com
patch subject: [PATCH v1] coredump: Use vmsplice_to_pipe() for pipes in dump_emit_page()
config: x86_64-allyesconfig
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/49da2daf3c9dc6a687c0f64c047bec6199f232af
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Peilin-Ye/coredump-Use-vmsplice_to_pipe-for-pipes-in-dump_emit_page/20221029-085307
        git checkout 49da2daf3c9dc6a687c0f64c047bec6199f232af
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash net/tls/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from net/tls/tls_sw.c:41:
>> include/linux/splice.h:84:56: warning: 'struct iov_iter' declared inside parameter list will not be visible outside of this definition or declaration
      84 | extern long vmsplice_to_pipe(struct file *file, struct iov_iter *iter,
         |                                                        ^~~~~~~~


vim +84 include/linux/splice.h

    64	
    65	typedef int (splice_actor)(struct pipe_inode_info *, struct pipe_buffer *,
    66				   struct splice_desc *);
    67	typedef int (splice_direct_actor)(struct pipe_inode_info *,
    68					  struct splice_desc *);
    69	
    70	extern ssize_t splice_from_pipe(struct pipe_inode_info *, struct file *,
    71					loff_t *, size_t, unsigned int,
    72					splice_actor *);
    73	extern ssize_t __splice_from_pipe(struct pipe_inode_info *,
    74					  struct splice_desc *, splice_actor *);
    75	extern ssize_t splice_to_pipe(struct pipe_inode_info *,
    76				      struct splice_pipe_desc *);
    77	extern ssize_t add_to_pipe(struct pipe_inode_info *,
    78				      struct pipe_buffer *);
    79	extern ssize_t splice_direct_to_actor(struct file *, struct splice_desc *,
    80					      splice_direct_actor *);
    81	extern long do_splice(struct file *in, loff_t *off_in,
    82			      struct file *out, loff_t *off_out,
    83			      size_t len, unsigned int flags);
  > 84	extern long vmsplice_to_pipe(struct file *file, struct iov_iter *iter,
    85				     unsigned int flags);
    86
kernel test robot Oct. 29, 2022, 9:12 a.m. UTC | #2 
Hi Peilin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.1-rc2 next-20221028]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Peilin-Ye/coredump-Use-vmsplice_to_pipe-for-pipes-in-dump_emit_page/20221029-085307
patch link:    https://lore.kernel.org/r/20221029005147.2553-1-yepeilin.cs%40gmail.com
patch subject: [PATCH v1] coredump: Use vmsplice_to_pipe() for pipes in dump_emit_page()
config: hexagon-randconfig-r041-20221029
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/49da2daf3c9dc6a687c0f64c047bec6199f232af
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Peilin-Ye/coredump-Use-vmsplice_to_pipe-for-pipes-in-dump_emit_page/20221029-085307
        git checkout 49da2daf3c9dc6a687c0f64c047bec6199f232af
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash net/tls/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from net/tls/tls_sw.c:41:
>> include/linux/splice.h:84:56: warning: declaration of 'struct iov_iter' will not be visible outside of this function [-Wvisibility]
   extern long vmsplice_to_pipe(struct file *file, struct iov_iter *iter,
                                                          ^
   In file included from net/tls/tls_sw.c:44:
   In file included from include/net/strparser.h:11:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from net/tls/tls_sw.c:44:
   In file included from include/net/strparser.h:11:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from net/tls/tls_sw.c:44:
   In file included from include/net/strparser.h:11:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   7 warnings generated.


vim +84 include/linux/splice.h

    64	
    65	typedef int (splice_actor)(struct pipe_inode_info *, struct pipe_buffer *,
    66				   struct splice_desc *);
    67	typedef int (splice_direct_actor)(struct pipe_inode_info *,
    68					  struct splice_desc *);
    69	
    70	extern ssize_t splice_from_pipe(struct pipe_inode_info *, struct file *,
    71					loff_t *, size_t, unsigned int,
    72					splice_actor *);
    73	extern ssize_t __splice_from_pipe(struct pipe_inode_info *,
    74					  struct splice_desc *, splice_actor *);
    75	extern ssize_t splice_to_pipe(struct pipe_inode_info *,
    76				      struct splice_pipe_desc *);
    77	extern ssize_t add_to_pipe(struct pipe_inode_info *,
    78				      struct pipe_buffer *);
    79	extern ssize_t splice_direct_to_actor(struct file *, struct splice_desc *,
    80					      splice_direct_actor *);
    81	extern long do_splice(struct file *in, loff_t *off_in,
    82			      struct file *out, loff_t *off_out,
    83			      size_t len, unsigned int flags);
  > 84	extern long vmsplice_to_pipe(struct file *file, struct iov_iter *iter,
    85				     unsigned int flags);
    86
diff mbox series

Patch

diff --git a/fs/coredump.c b/fs/coredump.c
index 7bad7785e8e6..a6ef406dee43 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -42,6 +42,7 @@ 
 #include <linux/timekeeping.h>
 #include <linux/sysctl.h>
 #include <linux/elf.h>
+#include <linux/splice.h>
 
 #include <linux/uaccess.h>
 #include <asm/mmu_context.h>
@@ -854,7 +855,11 @@  static int dump_emit_page(struct coredump_params *cprm, struct page *page)
 		return 0;
 	pos = file->f_pos;
 	iov_iter_bvec(&iter, WRITE, &bvec, 1, PAGE_SIZE);
-	n = __kernel_write_iter(cprm->file, &iter, &pos);
+
+	n = vmsplice_to_pipe(file, &iter, 0);
+	if (n == -EBADF)
+		n = __kernel_write_iter(cprm->file, &iter, &pos);
+
 	if (n != PAGE_SIZE)
 		return 0;
 	file->f_pos = pos;
diff --git a/fs/splice.c b/fs/splice.c
index 0878b852b355..2051700cda79 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -1234,8 +1234,8 @@  static long vmsplice_to_user(struct file *file, struct iov_iter *iter,
  * as splice-from-memory, where the regular splice is splice-from-file (or
  * to file). In both cases the output is a pipe, naturally.
  */
-static long vmsplice_to_pipe(struct file *file, struct iov_iter *iter,
-			     unsigned int flags)
+long vmsplice_to_pipe(struct file *file, struct iov_iter *iter,
+		      unsigned int flags)
 {
 	struct pipe_inode_info *pipe;
 	long ret = 0;
diff --git a/include/linux/splice.h b/include/linux/splice.h
index a55179fd60fc..0cd955cf5ee2 100644
--- a/include/linux/splice.h
+++ b/include/linux/splice.h
@@ -81,6 +81,8 @@  extern ssize_t splice_direct_to_actor(struct file *, struct splice_desc *,
 extern long do_splice(struct file *in, loff_t *off_in,
 		      struct file *out, loff_t *off_out,
 		      size_t len, unsigned int flags);
+extern long vmsplice_to_pipe(struct file *file, struct iov_iter *iter,
+			     unsigned int flags);
 
 extern long do_tee(struct file *in, struct file *out, size_t len,
 		   unsigned int flags);