mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Miller <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,
	agrover@redhat.com, clm@fb.com, zab@zabbo.net,
	ajaykumar.hotchandani@oracle.com,
	sergei.shtylyov@cogentembedded.com
Subject: Re: [PATCH v4 3/3] RDS: make sure not to loop forever inside rds_send_xmit
Date: Tue, 07 Apr 2015 17:26:07 -0400 (EDT)	[thread overview]
Message-ID: <20150407.172607.869971800253661690.davem@davemloft.net> (raw)
In-Reply-To: <a757317ae49acd938022254b5b0d899262d56008.1428437150.git.sowmini.varadhan@oracle.com>

From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Date: Tue,  7 Apr 2015 16:38:04 -0400

> @@ -157,6 +160,17 @@ int rds_send_xmit(struct rds_connection *conn)
>  	}
>  
>  	/*
> +	 * 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.

Still very unhappy with this patch series submission, as I still find
new problems every time I look at these changes.  Are you evaluating
them and double checking all of the claims in the commit log message
and comments, and logic in these, or are you just passing them off
upstream after testing and leaving the checking to people like me?

  reply	other threads:[~2015-04-07 21:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-07 20:38 [PATCH v4 0/3] RDS: RDS-core fixes Sowmini Varadhan
2015-04-07 20:38 ` [PATCH v4 1/3] Document AF_RDS, PF_RDS and SOL_RDS correctly Sowmini Varadhan
2015-04-07 20:38 ` [PATCH v4 2/3] RDS: only use passive connections when addresses match Sowmini Varadhan
2015-04-07 20:38 ` [PATCH v4 3/3] RDS: make sure not to loop forever inside rds_send_xmit Sowmini Varadhan
2015-04-07 21:26   ` David Miller [this message]
2015-04-07 21:56     ` Sowmini Varadhan
2015-04-07 22:27       ` David Miller

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=20150407.172607.869971800253661690.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=agrover@redhat.com \
    --cc=ajaykumar.hotchandani@oracle.com \
    --cc=chien.yen@oracle.com \
    --cc=clm@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rds-devel@oss.oracle.com \
    --cc=sergei.shtylyov@cogentembedded.com \
    --cc=sowmini.varadhan@oracle.com \
    --cc=zab@zabbo.net \
    /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®