From: Dexuan Cui <decui@microsoft.com>
To: Vitaly Kuznetsov <vkuznets@redhat.com>
CC: David Miller <davem@davemloft.net>, "olaf@aepfle.de" <olaf@aepfle.de>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "jasowang@redhat.com" <jasowang@redhat.com>,
        "driverdev-devel@linuxdriverproject.org" 
	<driverdev-devel@linuxdriverproject.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stephen@networkplumber.org" <stephen@networkplumber.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "apw@canonical.com" <apw@canonical.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: AQHQwKHe+mzxAOlkeU+W2yltYmiT5p3juSfw
Date: Mon, 20 Jul 2015 05:30:50 +0000
Message-ID: <c56c82d2455f4183a51cd414edc0a9b1@SIXPR30MB031.064d.mgd.msft.net>
References: <1436868048-17875-1-git-send-email-decui@microsoft.com>
	<20150715.211916.1258342954251964451.davem@davemloft.net>
	<d1e303ece500475b8e422d55f82fbffd@SIXPR30MB031.064d.mgd.msft.net>
 <87fv4mn7v9.fsf@vitty.brq.redhat.com>
In-Reply-To: <87fv4mn7v9.fsf@vitty.brq.redhat.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
BN1PR0301MB0611: X-MS-Exchange-Organization-RulesExecuted
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Jul 2015 05:30:55.1315
 (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: BN1PR0301MB0611
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1768
Lines: 54

> -----Original Message-----
> From: Vitaly Kuznetsov
> Sent: Friday, July 17, 2015 23:04
> To: Dexuan Cui
> Dexuan Cui writes:
> 
> >> 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.
> >
> 
> I'd suggest we rather use something like net_dbg_ratelimited()
> intead. The driver is new so issues are expected. Some debugging may
> be useful)
> 
>   Vitaly

The net_*ratelimited() functions may lose message if the messages are
printed very frequently, like in hvsock_sendmsg()/hvsock_recvmsg().

I used pr_debug() because I saw it's widely used in net/ipv4/.
Usually CONFIG_DYNAMIC_DEBUG=y, so pr_debug() means 
dynamic_pr_debug(), which won't output anything by default.

Sorry, I'm not sure about the "sophisticated tracing facilities in the
kernel that can do that transparently, and more" mentioned by David.

Anyway, I don't mind  simply removing the pr_debug() usages.
I will send out V3 later today.

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/