From: Dexuan Cui <decui@microsoft.com>
To: David Miller <davem@davemloft.net>
CC: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "stephen@networkplumber.org" <stephen@networkplumber.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "driverdev-devel@linuxdriverproject.org" 
	<driverdev-devel@linuxdriverproject.org>,
        "olaf@aepfle.de" <olaf@aepfle.de>,
        "apw@canonical.com" <apw@canonical.com>,
        "jasowang@redhat.com" <jasowang@redhat.com>,
        KY Srinivasan <kys@microsoft.com>,
        "pebolle@tiscali.nl" <pebolle@tiscali.nl>
Subject: RE: [V2 6/7] hvsock: introduce Hyper-V VM Sockets feature
Thread-Topic: [V2 6/7] hvsock: introduce Hyper-V VM Sockets feature
Thread-Index: AQHQv36Y9y32eRwibUSucK6NFvYMs53d8qnw
Date: Thu, 16 Jul 2015 11:19:01 +0000
Message-ID: <d1e303ece500475b8e422d55f82fbffd@SIXPR30MB031.064d.mgd.msft.net>
References: <1436868048-17875-1-git-send-email-decui@microsoft.com>
 <20150715.211916.1258342954251964451.davem@davemloft.net>
In-Reply-To: <20150715.211916.1258342954251964451.davem@davemloft.net>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
CY1PR0301MB0698: X-MS-Exchange-Organization-RulesExecuted
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Jul 2015 11:19:08.7934
 (UTC)
X-MS-Exchange-CrossTenant-Id: 72f988bf-86f1-41af-91ab-2d7cd011db47
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=72f988bf-86f1-41af-91ab-2d7cd011db47;Ip=[206.191.229.116];Helo=[064-smtp-out.microsoft.com]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR0301MB0698
X-Microsoft-Exchange-Diagnostics: 1;CY1PR0301MB1625;2:bDh7MA/vw0W/GRLb3+0oL4vdKSZ/ADY6kIqNcm7QSL+r+uMJYNWuCzjXNK/PjTTs;3:C7Qaly1OoIpGTNYJcEiqCr1rRB7KMD/urASaKau7YoHYYA7Pgt5ZLSbICpebaUvEALtTzF9/ass2iUb/PPNLrceSohLFc497Ules6bDrB7svdf+gE89JUybLGMZX5BBGdZ2Bi3Ez1bcPrgWXgbLEwPkzAOtlMPELGWgFtCBphe+ODVAZ9gkgbhnmUp7lTOdw0BxBzoCoaqdtHHQxmT6cE3LDDSfRZDzjjVcFWMOiBkRUDClnHWqQtHpegfw2yrlr;25:KDo9pAy1ktw/4ur9oGOfDlbW4wLBcg73PgqD+gSzHCmvL/j0t9M51qEAGiTCMOKU+o76avDKaPvopAvlqyxcyNs/WKO5IFMA9vhMyrX/n/VjFISyQuMz18Ubpeum9/75UjwgfUb+UQAF5cuUGdbvGRbukktJfOSnqUHs2QmSW7SUoQ5Gzeu1XWXF1Xoxa96FgmK6a9juiA2zT8whf46Xk/4bh6ZaycOu7Q1P9SLunlk3vRpJCcNksCKGyb2bjXvlUljFSud6nEXZYMQJmUqdfg==;20:a+4SM7GpyiEpd0XvrelY8XlHoEwI8mlaqN1Wbhgzhv/p/TnTlDvlqBqlAb1M+CE8Cdh7g25Xgjz+pM+LknZRkA==;23:vFmYPWjSvqg88b1iDJyr0k/eIY2p1R1/Rx9VW+4C+3ebi6iHtCDPnERIBTg2r5W4nr/iWoPtVQQsMTzIaeDcJWmI6WGvm9Wu51n6pjilWCCdKO9nBOe64uA1vHl3XnGhPHdxbESIPGweHVemwyJ9L9X1toiEYOGrMS8Lg33ojEzv/czHLUEqMUYspnFEzJpZOnu7TFMPt3wjM+bVlFa9VRk/khYBwyFLNgLyozh9XS3Y+oUKlfD03dncO9Hs8HA/
CY1PR0301MB1625: X-MS-Exchange-Organization-RulesExecuted
X-OriginatorOrg: microsoft.com
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1192
Lines: 42

> From: David Miller
> Sent: Thursday, July 16, 2015 12:19
> 
> From: Dexuan Cui
> Date: Tue, 14 Jul 2015 03:00:48 -0700
> 
> > +	pr_debug("hvsock_sk_destruct: called\n");
> 
> Debug logging just to state that a function is called is not appropriate,
> we have very sophisticated tracing facilities in the kernel that can do
> that transparently, and more.
> 
> Please remove this.
OK. 
 
> > +	if (hvsk->channel) {
> > +		pr_debug("hvsock_sk_destruct: calling vmbus_close()\n");
> 
> Likewise, these kinds of debug logs are totally inappropriate.
OK, I'll remove all the pr_debug() in the patch.
 
> > +static int hvsock_release(struct socket *sock)
> > +{
> > +	/* sock->sk is NULL, if accept() is interrupted by a signal */
> > +	if (sock->sk) {
> > +		__hvsock_release(sock->sk);
> > +		sock->sk = NULL;
> > +	}
> > +
> > +	sock->state = SS_FREE;
> > +	pr_debug("hvsock_release called\n\n");
> 
> Likewise.
OK. Will fix it.

Thanks,
-- Dexuan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/