From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EA3A9C32751 for ; Wed, 31 Jul 2019 14:50:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9862220659 for ; Wed, 31 Jul 2019 14:50:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1564584630; bh=aYU3hIKMl6ro2wABHJQ55ZwI1pidWn98KYFlfZb0kd0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=d/siVRs+ctZni7T7u6JF9bh/A2qXFKyKK2cESAFp34aJV18GkajjYvLjUIc3bCfA8 Cf00hwDwgxWQAugv7t+yGAaAWlEbIxG0gHrzdrnxuOOL/hoJO1oCVwm9EDfyIvg+gd lMKmNphR3mTeWAsLsYvw+4zkHBePj3KFmtM6BVDY= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387907AbfGaOua (ORCPT ); Wed, 31 Jul 2019 10:50:30 -0400 Received: from mail.kernel.org ([198.145.29.99]:51502 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387593AbfGaOu3 (ORCPT ); Wed, 31 Jul 2019 10:50:29 -0400 Received: from localhost (unknown [77.137.115.125]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 7155F20659; Wed, 31 Jul 2019 14:50:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1564584628; bh=aYU3hIKMl6ro2wABHJQ55ZwI1pidWn98KYFlfZb0kd0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=DGzCDTB9P5NyYiU7XxkYg4kLOkBmqnD6QO2/hdLuOEcElSQO23kA8dHXIUFMB34KA u2CuS6F0UGWBPZkFPIfvb9MjnHKaM9w8rSEEpWTOs4N+7NzV1UpTlHBNXA5a82JAO7 oVQ9Fy22GHU89L9eEQ1OglJz4h6Wl4NipL3kNU0c= Date: Wed, 31 Jul 2019 17:50:22 +0300 From: Leon Romanovsky To: Gal Pressman Cc: Jason Gunthorpe , Doug Ledford , linux-rdma@vger.kernel.org Subject: Re: [PATCH for-next 1/2] RDMA/core: Introduce ratelimited ibdev printk functions Message-ID: <20190731145022.GB4832@mtr-leonro.mtl.com> References: <20190730151834.70993-1-galpress@amazon.com> <20190730151834.70993-2-galpress@amazon.com> <20190730154148.GG4878@mtr-leonro.mtl.com> <20190731074109.GL4878@mtr-leonro.mtl.com> <20190731114609.GS4878@mtr-leonro.mtl.com> <20190731133309.GW4878@mtr-leonro.mtl.com> <4a767c0c-f1cb-3edf-3ad0-7adc07fd2c78@amazon.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4a767c0c-f1cb-3edf-3ad0-7adc07fd2c78@amazon.com> User-Agent: Mutt/1.12.0 (2019-05-25) Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org On Wed, Jul 31, 2019 at 05:19:41PM +0300, Gal Pressman wrote: > On 31/07/2019 16:33, Leon Romanovsky wrote: > > On Wed, Jul 31, 2019 at 03:56:55PM +0300, Gal Pressman wrote: > >> On 31/07/2019 14:46, Leon Romanovsky wrote: > >>> On Wed, Jul 31, 2019 at 01:51:05PM +0300, Gal Pressman wrote: > >>>> On 31/07/2019 10:41, Leon Romanovsky wrote: > >>>>> On Wed, Jul 31, 2019 at 10:22:42AM +0300, Gal Pressman wrote: > >>>>>> On 30/07/2019 18:41, Leon Romanovsky wrote: > >>>>>>> On Tue, Jul 30, 2019 at 06:18:33PM +0300, Gal Pressman wrote: > >>>>>>>> Add ratelimited helpers to the ibdev_* printk functions. > >>>>>>>> Implementation inspired by counterpart dev_*_ratelimited functions. > >>>>>>>> > >>>>>>>> Signed-off-by: Gal Pressman > >>>>>>>> --- > >>>>>>>> include/rdma/ib_verbs.h | 51 +++++++++++++++++++++++++++++++++++++++++ > >>>>>>>> 1 file changed, 51 insertions(+) > >>>>>>>> > >>>>>>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > >>>>>>>> index c5f8a9f17063..356e6a105366 100644 > >>>>>>>> --- a/include/rdma/ib_verbs.h > >>>>>>>> +++ b/include/rdma/ib_verbs.h > >>>>>>>> @@ -107,6 +107,57 @@ static inline > >>>>>>>> void ibdev_dbg(const struct ib_device *ibdev, const char *format, ...) {} > >>>>>>>> #endif > >>>>>>>> > >>>>>>>> +#define ibdev_level_ratelimited(ibdev_level, ibdev, fmt, ...) \ > >>>>>>>> +do { \ > >>>>>>>> + static DEFINE_RATELIMIT_STATE(_rs, \ > >>>>>>>> + DEFAULT_RATELIMIT_INTERVAL, \ > >>>>>>>> + DEFAULT_RATELIMIT_BURST); \ > >>>>>>>> + if (__ratelimit(&_rs)) \ > >>>>>>>> + ibdev_level(ibdev, fmt, ##__VA_ARGS__); \ > >>>>>>>> +} while (0) > >>>>>>>> + > >>>>>>>> +#define ibdev_emerg_ratelimited(ibdev, fmt, ...) \ > >>>>>>>> + ibdev_level_ratelimited(ibdev_emerg, ibdev, fmt, ##__VA_ARGS__) > >>>>>>>> +#define ibdev_alert_ratelimited(ibdev, fmt, ...) \ > >>>>>>>> + ibdev_level_ratelimited(ibdev_alert, ibdev, fmt, ##__VA_ARGS__) > >>>>>>>> +#define ibdev_crit_ratelimited(ibdev, fmt, ...) \ > >>>>>>>> + ibdev_level_ratelimited(ibdev_crit, ibdev, fmt, ##__VA_ARGS__) > >>>>>>>> +#define ibdev_err_ratelimited(ibdev, fmt, ...) \ > >>>>>>>> + ibdev_level_ratelimited(ibdev_err, ibdev, fmt, ##__VA_ARGS__) > >>>>>>>> +#define ibdev_warn_ratelimited(ibdev, fmt, ...) \ > >>>>>>>> + ibdev_level_ratelimited(ibdev_warn, ibdev, fmt, ##__VA_ARGS__) > >>>>>>>> +#define ibdev_notice_ratelimited(ibdev, fmt, ...) \ > >>>>>>>> + ibdev_level_ratelimited(ibdev_notice, ibdev, fmt, ##__VA_ARGS__) > >>>>>>>> +#define ibdev_info_ratelimited(ibdev, fmt, ...) \ > >>>>>>>> + ibdev_level_ratelimited(ibdev_info, ibdev, fmt, ##__VA_ARGS__) > >>>>>>>> + > >>>>>>>> +#if defined(CONFIG_DYNAMIC_DEBUG) > >>>>>>>> +/* descriptor check is first to prevent flooding with "callbacks suppressed" */ > >>>>>>>> +#define ibdev_dbg_ratelimited(ibdev, fmt, ...) \ > >>>>>>>> +do { \ > >>>>>>>> + static DEFINE_RATELIMIT_STATE(_rs, \ > >>>>>>>> + DEFAULT_RATELIMIT_INTERVAL, \ > >>>>>>>> + DEFAULT_RATELIMIT_BURST); \ > >>>>>>>> + DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \ > >>>>>>>> + if (DYNAMIC_DEBUG_BRANCH(descriptor) && __ratelimit(&_rs)) \ > >>>>>>>> + __dynamic_ibdev_dbg(&descriptor, ibdev, fmt, \ > >>>>>>>> + ##__VA_ARGS__); \ > >>>>>>>> +} while (0) > >>>>>>>> +#elif defined(DEBUG) > >>>>>>> > >>>>>>> When will you see this CONFIG_DEBUG set? I suspect only in private > >>>>>>> out-of-tree builds which we are not really care. Also I can't imagine > >>>>>>> system with this CONFIG_DEBUG and without CONFIG_DYNAMIC_DEBUG. > >>>>>> > >>>>>> This is the common way to handle debug prints, see: > >>>>>> https://elixir.bootlin.com/linux/v5.2.1/source/include/linux/printk.h#L331 > >>>>>> https://elixir.bootlin.com/linux/v5.2.1/source/include/linux/device.h#L1493 > >>>>>> https://elixir.bootlin.com/linux/v5.2.1/source/include/linux/netdevice.h#L4743 > >>>>>> https://elixir.bootlin.com/linux/v5.2.1/source/include/linux/net.h#L266 > >>>>> > >>>>> I'm more interested to know the real usage of this copy/paste and > >>>>> understand if it makes sense for drivers/infiniband/* or not. > >>>>> > >>>>> Not everything in netdev is great and worth to borrow. > >>>> > >>>> DEBUG exists since the first commit in the tree, and is used in various parts of > >>>> the kernel (mlx5 as well). Do you think it should be removed from the kernel? > >>> > >>> It is gradually removed when it is spotted, I'll send a patch for mlx5 now. > >> > >> Was there an on-list discussion regarding removal of DEBUG usage? Can you please > >> share a link? > > > > Sorry, but no, I didn't know that I need to save all discussions I see > > in the mailing lists. > > Trying to understand whether "It is gradually removed when it is spotted" is a > well known guideline by the community, should checkpatch produce a warning for this? I didn't see checks in checkpatch about tabs<->spaces mix either which you pointed for hns guys. > > > > >> If so, I agree the DEBUG part should be removed. > >> > >>> > >>>> > >>>> Regarding combination of both, I don't think DEBUG is related to > >>>> CONFIG_DYNAMIC_DEBUG. DEBUG is a generic debug flag (not necessarily to prints) > >>>> while CONFIG_DYNAMIC_DEBUG is specific to the dynamic debug prints infrastructure. > >>> > >>> I know exactly what DEBUG and CONFIG_DYNAMIC_DEBUG mean, but I'm > >>> asking YOU to provide us real and in-tree scenario where DEBUG will > >>> exists and CONFIG_DYNAMIC_DEBUG won't. > >> > >> What's any of this has to do with in-tree? This code and defines are part of the > >> tree. > >> > >> The use case doesn't matter, it's a valid permutation. Is there anything that > >> stops a user from building the kernel this way? > > > > Like everything else, nothing stops from you to do any modifications to > > the source code, before you will build. We are talking about in-tree > > builds and distro kernels. > > Last I checked turning on DEBUG doesn't require source code changes? Exciting, how did you enable DEBUG without recompiling source code? Maybe you find a way to enable DEBUG on running kernel? And how did it come that v5.3 kernel was compiled with DEBUG but without DYNAMIC_DEBUG? Thanks