From: Christoph Hellwig <hch@lst.de> To: Boris Pismenny <borisp@mellanox.com> Cc: dsahern@gmail.com, kuba@kernel.org, davem@davemloft.net, saeedm@nvidia.com, hch@lst.de, sagi@grimberg.me, axboe@fb.com, kbusch@kernel.org, viro@zeniv.linux.org.uk, edumazet@google.com, smalin@marvell.com, boris.pismenny@gmail.com, linux-nvme@lists.infradead.org, netdev@vger.kernel.org, benishay@nvidia.com, ogerlitz@nvidia.com, yorayz@nvidia.com, Ben Ben-Ishay <benishay@mellanox.com>, Or Gerlitz <ogerlitz@mellanox.com>, Yoray Zack <yorayz@mellanox.com> Subject: Re: [PATCH v3 net-next 01/21] iov_iter: Introduce new procedures for copy to iter/pages Date: Mon, 1 Feb 2021 18:35:48 +0100 [thread overview] Message-ID: <20210201173548.GA12960@lst.de> (raw) In-Reply-To: <20210201100509.27351-2-borisp@mellanox.com> On Mon, Feb 01, 2021 at 12:04:49PM +0200, Boris Pismenny wrote: > +static __always_inline __must_check > +size_t ddp_copy_to_iter(const void *addr, size_t bytes, struct iov_iter *i) > +{ > + if (unlikely(!check_copy_size(addr, bytes, true))) > + return 0; > + else > + return _ddp_copy_to_iter(addr, bytes, i); > +} No need for the else after a return, and the normal kernel convention double underscores for magic internal functions. But more importantly: does this belong into the generic header without and comments what the ddp means and when it should be used? > +static void ddp_memcpy_to_page(struct page *page, size_t offset, const char *from, size_t len) Overly long line. But we're also looking into generic helpers for this kind of things, not sure if they made it to linux-next in the meantime, but please check. > +size_t _ddp_copy_to_iter(const void *addr, size_t bytes, struct iov_iter *i) > +{ > + const char *from = addr; > + if (unlikely(iov_iter_is_pipe(i))) > + return copy_pipe_to_iter(addr, bytes, i); > + if (iter_is_iovec(i)) > + might_fault(); > + iterate_and_advance(i, bytes, v, > + copyout(v.iov_base, (from += v.iov_len) - v.iov_len, v.iov_len), > + ddp_memcpy_to_page(v.bv_page, v.bv_offset, > + (from += v.bv_len) - v.bv_len, v.bv_len), > + memcpy(v.iov_base, (from += v.iov_len) - v.iov_len, v.iov_len) > + ) > + > + return bytes; > +} This bloats every kernel build, so please move it into a conditionally built file. And please document the whole thing.
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@lst.de> To: Boris Pismenny <borisp@mellanox.com> Cc: smalin@marvell.com, sagi@grimberg.me, yorayz@nvidia.com, boris.pismenny@gmail.com, Ben Ben-Ishay <benishay@mellanox.com>, Yoray Zack <yorayz@mellanox.com>, linux-nvme@lists.infradead.org, davem@davemloft.net, axboe@fb.com, edumazet@google.com, netdev@vger.kernel.org, viro@zeniv.linux.org.uk, dsahern@gmail.com, kbusch@kernel.org, kuba@kernel.org, Or Gerlitz <ogerlitz@mellanox.com>, benishay@nvidia.com, saeedm@nvidia.com, hch@lst.de, ogerlitz@nvidia.com Subject: Re: [PATCH v3 net-next 01/21] iov_iter: Introduce new procedures for copy to iter/pages Date: Mon, 1 Feb 2021 18:35:48 +0100 [thread overview] Message-ID: <20210201173548.GA12960@lst.de> (raw) In-Reply-To: <20210201100509.27351-2-borisp@mellanox.com> On Mon, Feb 01, 2021 at 12:04:49PM +0200, Boris Pismenny wrote: > +static __always_inline __must_check > +size_t ddp_copy_to_iter(const void *addr, size_t bytes, struct iov_iter *i) > +{ > + if (unlikely(!check_copy_size(addr, bytes, true))) > + return 0; > + else > + return _ddp_copy_to_iter(addr, bytes, i); > +} No need for the else after a return, and the normal kernel convention double underscores for magic internal functions. But more importantly: does this belong into the generic header without and comments what the ddp means and when it should be used? > +static void ddp_memcpy_to_page(struct page *page, size_t offset, const char *from, size_t len) Overly long line. But we're also looking into generic helpers for this kind of things, not sure if they made it to linux-next in the meantime, but please check. > +size_t _ddp_copy_to_iter(const void *addr, size_t bytes, struct iov_iter *i) > +{ > + const char *from = addr; > + if (unlikely(iov_iter_is_pipe(i))) > + return copy_pipe_to_iter(addr, bytes, i); > + if (iter_is_iovec(i)) > + might_fault(); > + iterate_and_advance(i, bytes, v, > + copyout(v.iov_base, (from += v.iov_len) - v.iov_len, v.iov_len), > + ddp_memcpy_to_page(v.bv_page, v.bv_offset, > + (from += v.bv_len) - v.bv_len, v.bv_len), > + memcpy(v.iov_base, (from += v.iov_len) - v.iov_len, v.iov_len) > + ) > + > + return bytes; > +} This bloats every kernel build, so please move it into a conditionally built file. And please document the whole thing. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
next prev parent reply other threads:[~2021-02-01 17:37 UTC|newest] Thread overview: 88+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-02-01 10:04 [PATCH v3 net-next 00/21] nvme-tcp receive offloads Boris Pismenny 2021-02-01 10:04 ` Boris Pismenny 2021-02-01 10:04 ` [PATCH v3 net-next 01/21] iov_iter: Introduce new procedures for copy to iter/pages Boris Pismenny 2021-02-01 10:04 ` Boris Pismenny 2021-02-01 17:35 ` Christoph Hellwig [this message] 2021-02-01 17:35 ` Christoph Hellwig 2021-02-02 18:00 ` Or Gerlitz 2021-02-02 18:00 ` Or Gerlitz 2021-02-03 16:56 ` Christoph Hellwig 2021-02-03 16:56 ` Christoph Hellwig 2021-02-03 19:34 ` Ira Weiny 2021-02-03 19:34 ` Ira Weiny 2021-02-07 14:13 ` Boris Pismenny 2021-02-07 14:13 ` Boris Pismenny 2021-02-07 14:24 ` Boris Pismenny 2021-02-07 14:24 ` Boris Pismenny 2021-02-01 10:04 ` [PATCH v3 net-next 02/21] net: Introduce direct data placement tcp offload Boris Pismenny 2021-02-01 10:04 ` Boris Pismenny 2021-02-02 10:06 ` Tom Parkin 2021-02-02 10:06 ` Tom Parkin 2021-02-01 10:04 ` [PATCH v3 net-next 03/21] net: Introduce crc offload for tcp ddp ulp Boris Pismenny 2021-02-01 10:04 ` Boris Pismenny 2021-02-01 10:04 ` [PATCH v3 net-next 04/21] net: SKB copy(+hash) iterators for DDP offloads Boris Pismenny 2021-02-01 10:04 ` Boris Pismenny 2021-02-01 10:04 ` [PATCH v3 net-next 05/21] net/tls: expose get_netdev_for_sock Boris Pismenny 2021-02-01 10:04 ` Boris Pismenny 2021-02-01 10:04 ` [PATCH v3 net-next 06/21] nvme-tcp: Add DDP offload control path Boris Pismenny 2021-02-01 10:04 ` Boris Pismenny 2021-02-01 17:37 ` Christoph Hellwig 2021-02-01 17:37 ` Christoph Hellwig 2021-02-02 18:09 ` Or Gerlitz 2021-02-02 18:09 ` Or Gerlitz 2021-02-03 9:17 ` Sagi Grimberg 2021-02-03 9:17 ` Sagi Grimberg 2021-02-01 10:04 ` [PATCH v3 net-next 07/21] nvme-tcp: Add DDP data-path Boris Pismenny 2021-02-01 10:04 ` Boris Pismenny 2021-02-01 17:37 ` Christoph Hellwig 2021-02-01 17:37 ` Christoph Hellwig 2021-02-02 18:14 ` Or Gerlitz 2021-02-02 18:14 ` Or Gerlitz 2021-02-03 8:56 ` Sagi Grimberg 2021-02-03 8:56 ` Sagi Grimberg 2021-02-03 10:02 ` Christoph Hellwig 2021-02-03 10:02 ` Christoph Hellwig 2021-02-03 10:21 ` Sagi Grimberg 2021-02-03 10:21 ` Sagi Grimberg 2021-02-03 8:51 ` Sagi Grimberg 2021-02-03 8:51 ` Sagi Grimberg 2021-02-04 19:20 ` Or Gerlitz 2021-02-04 19:20 ` Or Gerlitz 2021-02-01 10:04 ` [PATCH v3 net-next 08/21] nvme-tcp : Recalculate crc in the end of the capsule Boris Pismenny 2021-02-01 10:04 ` Boris Pismenny 2021-02-03 9:06 ` Sagi Grimberg 2021-02-03 9:06 ` Sagi Grimberg 2021-02-04 18:36 ` Or Gerlitz 2021-02-04 18:36 ` Or Gerlitz 2021-02-07 16:40 ` Or Gerlitz 2021-02-07 16:40 ` Or Gerlitz 2021-02-01 10:04 ` [PATCH v3 net-next 09/21] nvme-tcp: Deal with netdevice DOWN events Boris Pismenny 2021-02-01 10:04 ` Boris Pismenny 2021-02-03 9:09 ` Sagi Grimberg 2021-02-03 9:09 ` Sagi Grimberg 2021-02-04 18:29 ` Or Gerlitz 2021-02-04 18:29 ` Or Gerlitz 2021-02-01 10:04 ` [PATCH v3 net-next 10/21] net/mlx5: Header file changes for nvme-tcp offload Boris Pismenny 2021-02-01 10:04 ` Boris Pismenny 2021-02-01 10:04 ` [PATCH v3 net-next 11/21] net/mlx5: Add 128B CQE for NVMEoTCP offload Boris Pismenny 2021-02-01 10:04 ` Boris Pismenny 2021-02-01 10:05 ` [PATCH v3 net-next 12/21] net/mlx5e: TCP flow steering for nvme-tcp Boris Pismenny 2021-02-01 10:05 ` Boris Pismenny 2021-02-01 10:05 ` [PATCH v3 net-next 13/21] net/mlx5e: NVMEoTCP offload initialization Boris Pismenny 2021-02-01 10:05 ` Boris Pismenny 2021-02-01 10:05 ` [PATCH v3 net-next 14/21] net/mlx5e: KLM UMR helper macros Boris Pismenny 2021-02-01 10:05 ` Boris Pismenny 2021-02-01 10:05 ` [PATCH v3 net-next 15/21] net/mlx5e: NVMEoTCP use KLM UMRs Boris Pismenny 2021-02-01 10:05 ` Boris Pismenny 2021-02-01 10:05 ` [PATCH v3 net-next 16/21] net/mlx5e: NVMEoTCP queue init/teardown Boris Pismenny 2021-02-01 10:05 ` Boris Pismenny 2021-02-01 10:05 ` [PATCH v3 net-next 17/21] net/mlx5e: NVMEoTCP async ddp invalidation Boris Pismenny 2021-02-01 10:05 ` Boris Pismenny 2021-02-01 10:05 ` [PATCH v3 net-next 18/21] net/mlx5e: NVMEoTCP ddp setup and resync Boris Pismenny 2021-02-01 10:05 ` Boris Pismenny 2021-02-01 10:05 ` [PATCH v3 net-next 19/21] net/mlx5e: NVMEoTCP, data-path for DDP+CRC offload Boris Pismenny 2021-02-01 10:05 ` Boris Pismenny 2021-02-01 10:05 ` [PATCH v3 net-next 20/21] net/mlx5e: NVMEoTCP statistics Boris Pismenny 2021-02-01 10:05 ` Boris Pismenny 2021-02-01 10:05 ` [PATCH v3 net-next 21/21] Documentation: add TCP DDP offload documentation Boris Pismenny 2021-02-01 10:05 ` Boris Pismenny
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=20210201173548.GA12960@lst.de \ --to=hch@lst.de \ --cc=axboe@fb.com \ --cc=benishay@mellanox.com \ --cc=benishay@nvidia.com \ --cc=boris.pismenny@gmail.com \ --cc=borisp@mellanox.com \ --cc=davem@davemloft.net \ --cc=dsahern@gmail.com \ --cc=edumazet@google.com \ --cc=kbusch@kernel.org \ --cc=kuba@kernel.org \ --cc=linux-nvme@lists.infradead.org \ --cc=netdev@vger.kernel.org \ --cc=ogerlitz@mellanox.com \ --cc=ogerlitz@nvidia.com \ --cc=saeedm@nvidia.com \ --cc=sagi@grimberg.me \ --cc=smalin@marvell.com \ --cc=viro@zeniv.linux.org.uk \ --cc=yorayz@mellanox.com \ --cc=yorayz@nvidia.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.