mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sagi Grimberg <sagi@grimberg.me>
To: David Howells <dhowells@redhat.com>,
	Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: netdev@vger.kernel.org,
	Alexander Duyck <alexander.duyck@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	David Ahern <dsahern@kernel.org>,
	Matthew Wilcox <willy@infradead.org>,
	Jens Axboe <axboe@kernel.dk>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Keith Busch <kbusch@kernel.org>, Jens Axboe <axboe@fb.com>,
	Christoph Hellwig <hch@lst.de>,
	Chaitanya Kulkarni <kch@nvidia.com>,
	linux-nvme@lists.infradead.org
Subject: Re: [PATCH net-next v2 10/17] nvme: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage
Date: Tue, 20 Jun 2023 16:00:31 +0300	[thread overview]
Message-ID: <028fd6f1-6fc2-6e8d-3de0-71c2b8f6b754@grimberg.me> (raw)
In-Reply-To: <55e7058b-07d0-3619-3481-2d70e95875ea@grimberg.me>


>>>      struct bio_vec bvec;
>>>      struct msghdr msg = { .msg_flags = MSG_SPLICE_PAGES | ... };
>>>
>>>      ..
>>>
>>>      bvec_set_virt
>>>      iov_iter_bvec
>>>      sock_sendmsg
>>>
>>> is a frequent pattern. Does it make sense to define a wrapper? Same 
>>> for bvec_set_page.
>>
>> I dunno.  I'm trying to move towards aggregating multiple pages in a bvec
>> before calling sendmsg if possible rather than doing it one page at a 
>> time,
>> but it's easier and more obvious in some places than others.
> 
> That would be great to do, but nvme needs to calculate a data digest
> and doing that in a separate scan of the payload is not very cache
> friendly...
> 
> There is also the fact that the payload may be sent in portions 
> asynchronously driven by how the controller wants to accept them,
> so there is some complexity there.
> 
> But worth looking at for sure.
> 
> The patch looks good to me, taking it to run some tests
> (from sendpage-3-frag branch in your kernel.org tree correct?)
> 
> For now, you can add:
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Patches seem to hold up.
Tested-by: Sagi Grimberg <sagi@grimberg.me>

However if possible, can you please split nvme/host and nvme/target
changes? We try to separate host side and target side changes in the
same patch.

  reply	other threads:[~2023-06-20 13:01 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-17 12:11 [PATCH net-next v2 00/17] splice, net: Switch over users of sendpage() and remove it David Howells
2023-06-17 12:11 ` [PATCH net-next v2 01/17] net: Copy slab data for sendmsg(MSG_SPLICE_PAGES) David Howells
2023-06-18 16:43   ` Willem de Bruijn
2023-06-17 12:11 ` [PATCH net-next v2 02/17] net: Display info about MSG_SPLICE_PAGES memory handling in proc David Howells
2023-06-17 12:11 ` [PATCH net-next v2 03/17] tcp_bpf, smc, tls, espintcp: Reduce MSG_SENDPAGE_NOTLAST usage David Howells
2023-06-17 12:11 ` [PATCH net-next v2 04/17] siw: Use sendmsg(MSG_SPLICE_PAGES) rather than sendpage to transmit David Howells
2023-06-17 12:11 ` [PATCH net-next v2 05/17] ceph: Use sendmsg(MSG_SPLICE_PAGES) rather than sendpage David Howells
2023-06-17 12:11 ` [PATCH net-next v2 06/17] net: Use sendmsg(MSG_SPLICE_PAGES) not sendpage in skb_send_sock() David Howells
2023-06-17 12:11 ` [PATCH net-next v2 07/17] ceph: Use sendmsg(MSG_SPLICE_PAGES) rather than sendpage() David Howells
2023-06-17 12:11 ` [PATCH net-next v2 08/17] rds: Use sendmsg(MSG_SPLICE_PAGES) rather than sendpage David Howells
2023-06-17 12:11 ` [PATCH net-next v2 09/17] dlm: " David Howells
2023-06-17 12:11 ` [PATCH net-next v2 10/17] nvme: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage David Howells
2023-06-18 16:47   ` Willem de Bruijn
2023-06-18 17:28   ` David Howells
2023-06-19  8:25     ` Sagi Grimberg
2023-06-20 13:00       ` Sagi Grimberg [this message]
2023-06-19  9:28     ` David Howells
2023-06-19 11:46     ` Willem de Bruijn
2023-06-17 12:11 ` [PATCH net-next v2 11/17] smc: Drop smc_sendpage() in favour of smc_sendmsg() + MSG_SPLICE_PAGES David Howells
2023-06-17 12:11 ` [PATCH net-next v2 12/17] ocfs2: Use sendmsg(MSG_SPLICE_PAGES) rather than sendpage() David Howells
2023-06-17 12:11 ` [PATCH net-next v2 13/17] drbd: " David Howells
2023-06-17 12:11 ` [PATCH net-next v2 14/17] drdb: Send an entire bio in a single sendmsg David Howells
2023-06-17 12:11 ` [PATCH net-next v2 15/17] iscsi: Use sendmsg(MSG_SPLICE_PAGES) rather than sendpage David Howells
2023-06-17 12:11 ` [PATCH net-next v2 16/17] sock: Remove ->sendpage*() in favour of sendmsg(MSG_SPLICE_PAGES) David Howells
2023-06-17 12:11 ` [PATCH net-next v2 17/17] net: Kill MSG_SENDPAGE_NOTLAST David Howells
2023-06-18 16:54   ` Willem de Bruijn
2023-06-19 12:05   ` David Howells
2023-06-20 12:59     ` Willem de Bruijn

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=028fd6f1-6fc2-6e8d-3de0-71c2b8f6b754@grimberg.me \
    --to=sagi@grimberg.me \
    --cc=alexander.duyck@gmail.com \
    --cc=axboe@fb.com \
    --cc=axboe@kernel.dk \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=hch@lst.de \
    --cc=kbusch@kernel.org \
    --cc=kch@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=willemdebruijn.kernel@gmail.com \
    --cc=willy@infradead.org \
    /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

all inboxes | Powered by JetHome®