From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753504AbbDGW1l (ORCPT ); Tue, 7 Apr 2015 18:27:41 -0400 Received: from shards.monkeyblade.net ([149.20.54.216]:60161 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753087AbbDGW1j (ORCPT ); Tue, 7 Apr 2015 18:27:39 -0400 Date: Tue, 07 Apr 2015 18:27:37 -0400 (EDT) Message-Id: <20150407.182737.2046311186569701796.davem@davemloft.net> To: sowmini.varadhan@oracle.com Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, chien.yen@oracle.com, rds-devel@oss.oracle.com, zab@zabbo.net, ajaykumar.hotchandani@oracle.com Subject: Re: [PATCH v4 3/3] RDS: make sure not to loop forever inside rds_send_xmit From: David Miller In-Reply-To: <20150407215605.GN29504@oracle.com> References: <20150407.172607.869971800253661690.davem@davemloft.net> <20150407215605.GN29504@oracle.com> X-Mailer: Mew version 6.6 on Emacs 24.4 / Mule 6.0 (HANACHIRUSATO) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.12 (shards.monkeyblade.net [149.20.54.216]); Tue, 07 Apr 2015 15:27:39 -0700 (PDT) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Sowmini Varadhan Date: Tue, 7 Apr 2015 17:56:05 -0400 > On (04/07/15 17:26), David Miller wrote: >> > /* >> > + * we record the send generation after doing the xmit acquire. >> > + * if someone else manages to jump in and do some work, we'll use >> > + * this to avoid a goto restart farther down. >> > + * >> > + * we don't need a lock because the counter is only incremented >> > + * while we have the in_xmit bit held. >> > + */ >> > + conn->c_send_gen++; >> > + send_gen = conn->c_send_gen; >> >> This increment does need to either be changed to be an atomic_t >> or covered by a lock. >> >> Otherwise two concurrent callers can both try to increment it at >> the same time, and it only effectively increments once. That's >> corrupted state and will break all of the new logic added here. > I'm afraid I dont follow what race condiiton you are seeing? Prior > to this line, the "acquire_in_xmit" check would have only allowed > one thread to successfully increment c_send_gen, right? What did I > miss? Then there is locking protecting this counter increment and the comment is wrong.