From patchwork Fri Sep 30 16:11:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 101726 Delivered-To: patch@linaro.org Received: by 10.140.106.72 with SMTP id d66csp376264qgf; Fri, 30 Sep 2016 09:12:55 -0700 (PDT) X-Received: by 10.107.176.144 with SMTP id z138mr9486904ioe.133.1475251975214; Fri, 30 Sep 2016 09:12:55 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id i8si20620807pap.189.2016.09.30.09.12.55; Fri, 30 Sep 2016 09:12:55 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of netdev-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of netdev-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=netdev-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933296AbcI3QMi (ORCPT + 4 others); Fri, 30 Sep 2016 12:12:38 -0400 Received: from mout.kundenserver.de ([212.227.126.187]:57872 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933938AbcI3QMf (ORCPT ); Fri, 30 Sep 2016 12:12:35 -0400 Received: from wuerfel.lan. ([78.43.20.153]) by mrelayeu.kundenserver.de (mreue003) with ESMTPA (Nemesis) id 0M7nzI-1b3YZV1PQe-00vOei; Fri, 30 Sep 2016 18:12:25 +0200 From: Arnd Bergmann To: "David S. Miller" , David Howells Cc: Arnd Bergmann , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] rxrpc: split up rxrpc_send_call_packet() Date: Fri, 30 Sep 2016 18:11:59 +0200 Message-Id: <20160930161218.4178124-1-arnd@arndb.de> X-Mailer: git-send-email 2.9.0 X-Provags-ID: V03:K0:eGvABC7LFQLOcqGpoOUlsR1K0+ZTlgson+lTPPL5RRzYsel13f3 lSOzojUpD8V6TWgSl8qDTvwoi8PBLu5e8F/+mNRn6NLGC67tX/Ai3yvYrShLTHfiXzB/8yp 1pSMo5WQ/YV+jxGHWiDDKxjyXcCJhFkCgnEW8D1UpiKd1c5s6lCkbtOjQhHtYTXad9oZI0M vXK0XEbbnIsUDR1YSCYcg== X-UI-Out-Filterresults: notjunk:1; V01:K0:DB0L6MBfPzA=:WU6ft/BmISrD2TpPlO0bvH SWWXZXfPBc4FYW6w13+TLMhZNB4aOL2fNOU9ZndBuZym/AiioYDjJ25cwDgbRZnQiHZd3ydLv atg89ddzEJLNeBZaW0aBR1Ls0eRn7+MbbyaN5XqcYzdfQLdr6Ni1+pihnbuZccztjvUmS3nED 65jhubokNfK5hxdnO6pTE4N+s54MDa9HxlCkNYEf8pqLOoS0VdK/Oh1rylN3FkFWqz9ku7biG GosVVG2UdrJDqQyi6ZgxOTgjalppVJLC3B5XYg47D11Rkcruvy8jlwvemCOuNbsb/tARIhdap xuhoHfonupqUNJ6/V+s9rWc7afSxoM0dwfI4G833bATA6FyslT8rlPHO1LHlwdzuJdBYI68ZS aRxSsVyJBwXYe33k4MwQiabZzJ5Tv5NlgzuF7YVMEV97vzDn3mZ4yxeqZiF4lAVUY+UWw7CN5 IKg5g5aP1cUWCRiS1cOi3Oi64BYAOSNWe0VBBDEXiVHHUb+qiVTB0IGAM79tvhwLqQjR6H6rx qyiikJMe9J/ZZsCH06escUwkpZMI7McimSvCnOnx1+iw/+jsZd9vwhYz7/JkpbSJ30d6kUstV psUwEiOkJYJO9q98i38DHPs2PkMLy6qvuXjGU/b0louM0+Hh6FeCaNpRVzHIWNi4g+gIaYT6m EdDrbIgTBoMykiTx3UACiBqHkfOwWO/uwlY1TPz8KOb/0au0NjEDmy7W2JCku9XVT7aI= Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org A number of reworks went into rxrpc_send_call_packet() recently, which introduced another warning when built with -Wmaybe-uninitialized: In file included from ../net/rxrpc/output.c:20:0: net/rxrpc/output.c: In function 'rxrpc_send_call_packet': net/rxrpc/ar-internal.h:1187:27: error: 'top' may be used uninitialized in this function [-Werror=maybe-uninitialized] net/rxrpc/output.c:103:24: note: 'top' was declared here net/rxrpc/output.c:225:25: error: 'hard_ack' may be used uninitialized in this function [-Werror=maybe-uninitialized] This is a false positive, but it's also an indication that the function is getting complex enough that the compiler cannot figure out what it does. This splits out a rxrpc_send_ack_packet() function for one part of it, making it more understandable by both humans and the compiler and avoiding the warning. Signed-off-by: Arnd Bergmann --- Unfortunately, this is a larger rework that I was hoping for, and I have only build tested it, so please review carefully, or just discard it and treat it as feedback to the original patch. --- net/rxrpc/output.c | 164 ++++++++++++++++++++++++++++------------------------- 1 file changed, 88 insertions(+), 76 deletions(-) -- 2.9.0 diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c index cf43a715685e..821af21c7159 100644 --- a/net/rxrpc/output.c +++ b/net/rxrpc/output.c @@ -90,6 +90,86 @@ static size_t rxrpc_fill_out_ack(struct rxrpc_call *call, return top - hard_ack + 3; } +static int rxrpc_send_ack_packet(struct rxrpc_call *call, + struct rxrpc_connection *conn, + struct msghdr *msg, + struct rxrpc_pkt_buffer *pkt, + struct kvec iov[2]) +{ + bool ping; + size_t len, n; + rxrpc_serial_t serial; + rxrpc_seq_t hard_ack, top; + int ioc, ret; + + spin_lock_bh(&call->lock); + if (!call->ackr_reason) { + spin_unlock_bh(&call->lock); + ret = 0; + goto out; + } + ping = (call->ackr_reason == RXRPC_ACK_PING); + n = rxrpc_fill_out_ack(call, pkt, &hard_ack, &top); + call->ackr_reason = 0; + + spin_unlock_bh(&call->lock); + + pkt->whdr.flags |= RXRPC_SLOW_START_OK; + + iov[0].iov_len += sizeof(pkt->ack) + n; + iov[1].iov_base = &pkt->ackinfo; + iov[1].iov_len = sizeof(pkt->ackinfo); + len = sizeof(pkt->whdr) + sizeof(pkt->ack) + n + sizeof(pkt->ackinfo); + ioc = 2; + serial = atomic_inc_return(&conn->serial); + pkt->whdr.serial = htonl(serial); + + trace_rxrpc_tx_ack(call, serial, + ntohl(pkt->ack.firstPacket), + ntohl(pkt->ack.serial), + pkt->ack.reason, pkt->ack.nAcks); + + if (ping) { + call->ackr_ping = serial; + smp_wmb(); + /* We need to stick a time in before we send the packet in case + * the reply gets back before kernel_sendmsg() completes - but + * asking UDP to send the packet can take a relatively long + * time, so we update the time after, on the assumption that + * the packet transmission is more likely to happen towards the + * end of the kernel_sendmsg() call. + */ + call->ackr_ping_time = ktime_get_real(); + set_bit(RXRPC_CALL_PINGING, &call->flags); + trace_rxrpc_rtt_tx(call, rxrpc_rtt_tx_ping, serial); + } + ret = kernel_sendmsg(conn->params.local->socket, + msg, iov, ioc, len); + if (ping) + call->ackr_ping_time = ktime_get_real(); + + if (call->state < RXRPC_CALL_COMPLETE) { + if (ret < 0) { + clear_bit(RXRPC_CALL_PINGING, &call->flags); + rxrpc_propose_ACK(call, pkt->ack.reason, + ntohs(pkt->ack.maxSkew), + ntohl(pkt->ack.serial), + true, true, + rxrpc_propose_ack_retry_tx); + } else { + spin_lock_bh(&call->lock); + if (after(hard_ack, call->ackr_consumed)) + call->ackr_consumed = hard_ack; + if (after(top, call->ackr_seen)) + call->ackr_seen = top; + spin_unlock_bh(&call->lock); + } + } +out: + return ret; +} + + /* * Send an ACK or ABORT call packet. */ @@ -99,11 +179,9 @@ int rxrpc_send_call_packet(struct rxrpc_call *call, u8 type) struct rxrpc_pkt_buffer *pkt; struct msghdr msg; struct kvec iov[2]; - rxrpc_serial_t serial; - rxrpc_seq_t hard_ack, top; - size_t len, n; - bool ping = false; + size_t len; int ioc, ret; + rxrpc_serial_t serial; u32 abort_code; _enter("%u,%s", call->debug_id, rxrpc_pkts[type]); @@ -140,96 +218,30 @@ int rxrpc_send_call_packet(struct rxrpc_call *call, u8 type) iov[0].iov_base = pkt; iov[0].iov_len = sizeof(pkt->whdr); - len = sizeof(pkt->whdr); switch (type) { case RXRPC_PACKET_TYPE_ACK: - spin_lock_bh(&call->lock); - if (!call->ackr_reason) { - spin_unlock_bh(&call->lock); - ret = 0; - goto out; - } - ping = (call->ackr_reason == RXRPC_ACK_PING); - n = rxrpc_fill_out_ack(call, pkt, &hard_ack, &top); - call->ackr_reason = 0; - - spin_unlock_bh(&call->lock); - - - pkt->whdr.flags |= RXRPC_SLOW_START_OK; - - iov[0].iov_len += sizeof(pkt->ack) + n; - iov[1].iov_base = &pkt->ackinfo; - iov[1].iov_len = sizeof(pkt->ackinfo); - len += sizeof(pkt->ack) + n + sizeof(pkt->ackinfo); - ioc = 2; + ret = rxrpc_send_ack_packet(call, conn, &msg, pkt, iov); break; case RXRPC_PACKET_TYPE_ABORT: abort_code = call->abort_code; pkt->abort_code = htonl(abort_code); iov[0].iov_len += sizeof(pkt->abort_code); - len += sizeof(pkt->abort_code); + len = sizeof(pkt->whdr) + sizeof(pkt->abort_code); ioc = 1; + serial = atomic_inc_return(&conn->serial); + pkt->whdr.serial = htonl(serial); + ret = kernel_sendmsg(conn->params.local->socket, + &msg, iov, ioc, len); break; default: BUG(); ret = -ENOANO; - goto out; - } - - serial = atomic_inc_return(&conn->serial); - pkt->whdr.serial = htonl(serial); - switch (type) { - case RXRPC_PACKET_TYPE_ACK: - trace_rxrpc_tx_ack(call, serial, - ntohl(pkt->ack.firstPacket), - ntohl(pkt->ack.serial), - pkt->ack.reason, pkt->ack.nAcks); break; } - if (ping) { - call->ackr_ping = serial; - smp_wmb(); - /* We need to stick a time in before we send the packet in case - * the reply gets back before kernel_sendmsg() completes - but - * asking UDP to send the packet can take a relatively long - * time, so we update the time after, on the assumption that - * the packet transmission is more likely to happen towards the - * end of the kernel_sendmsg() call. - */ - call->ackr_ping_time = ktime_get_real(); - set_bit(RXRPC_CALL_PINGING, &call->flags); - trace_rxrpc_rtt_tx(call, rxrpc_rtt_tx_ping, serial); - } - ret = kernel_sendmsg(conn->params.local->socket, - &msg, iov, ioc, len); - if (ping) - call->ackr_ping_time = ktime_get_real(); - - if (type == RXRPC_PACKET_TYPE_ACK && - call->state < RXRPC_CALL_COMPLETE) { - if (ret < 0) { - clear_bit(RXRPC_CALL_PINGING, &call->flags); - rxrpc_propose_ACK(call, pkt->ack.reason, - ntohs(pkt->ack.maxSkew), - ntohl(pkt->ack.serial), - true, true, - rxrpc_propose_ack_retry_tx); - } else { - spin_lock_bh(&call->lock); - if (after(hard_ack, call->ackr_consumed)) - call->ackr_consumed = hard_ack; - if (after(top, call->ackr_seen)) - call->ackr_seen = top; - spin_unlock_bh(&call->lock); - } - } - -out: rxrpc_put_connection(conn); kfree(pkt); return ret;