[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240305140343.GH2357@kernel.org>
Date: Tue, 5 Mar 2024 14:03:43 +0000
From: Simon Horman <horms@...nel.org>
To: David Howells <dhowells@...hat.com>
Cc: netdev@...r.kernel.org, Marc Dionne <marc.dionne@...istor.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
linux-afs@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 14/21] rxrpc: Do zerocopy using
MSG_SPLICE_PAGES and page frags
On Mon, Mar 04, 2024 at 08:43:11AM +0000, David Howells wrote:
..
> diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
> index ef0849c8329c..e540501a20ad 100644
> --- a/net/rxrpc/rxkad.c
> +++ b/net/rxrpc/rxkad.c
> @@ -145,16 +145,17 @@ static int rxkad_init_connection_security(struct rxrpc_connection *conn,
> /*
> * Work out how much data we can put in a packet.
> */
> -static int rxkad_how_much_data(struct rxrpc_call *call, size_t remain,
> - size_t *_buf_size, size_t *_data_size, size_t *_offset)
> +static struct rxrpc_txbuf *rxkad_alloc_txbuf(struct rxrpc_call *call, size_t remain, gfp_t gfp)
> {
> - size_t shdr, buf_size, chunk;
> + struct rxrpc_txbuf *txb;
> + size_t shdr, space;
> +
> + remain = min(remain, 65535 - sizeof(struct rxrpc_wire_header));
>
> switch (call->conn->security_level) {
> default:
> - buf_size = chunk = min_t(size_t, remain, RXRPC_JUMBO_DATALEN);
> - shdr = 0;
> - goto out;
> + space = min_t(size_t, remain, RXRPC_JUMBO_DATALEN);
> + return rxrpc_alloc_data_txbuf(call, space, 0, GFP_KERNEL);
Hi David,
should gfp be used here in place of GFP_KERNEL?
> case RXRPC_SECURITY_AUTH:
> shdr = sizeof(struct rxkad_level1_hdr);
> break;
> @@ -163,17 +164,15 @@ static int rxkad_how_much_data(struct rxrpc_call *call, size_t remain,
> break;
> }
>
> - buf_size = round_down(RXRPC_JUMBO_DATALEN, RXKAD_ALIGN);
> -
> - chunk = buf_size - shdr;
> - if (remain < chunk)
> - buf_size = round_up(shdr + remain, RXKAD_ALIGN);
> + space = min_t(size_t, round_down(RXRPC_JUMBO_DATALEN, RXKAD_ALIGN), remain + shdr);
> + space = round_up(space, RXKAD_ALIGN);
>
> -out:
> - *_buf_size = buf_size;
> - *_data_size = chunk;
> - *_offset = shdr;
> - return 0;
> + txb = rxrpc_alloc_data_txbuf(call, space, RXKAD_ALIGN, GFP_KERNEL);
Likewise, here too.
Flagged by Smatch.
> + if (txb) {
> + txb->offset += shdr;
> + txb->space -= shdr;
> + }
> + return txb;
nit: I think this would be a more idiomatic construction.
(Completely untested!)
if (!txb)
return NULL;
txb->offset += shdr;
txb->space -= shdr;
return txb;
> }
>
> /*
..
Powered by blists - more mailing lists