From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp1.cs.Stanford.EDU (smtp1.cs.stanford.edu [171.64.64.25]) by dpdk.org (Postfix) with ESMTP id 55A2C2BCE for ; Tue, 11 Oct 2016 18:31:12 +0200 (CEST) Received: from mail-io0-f180.google.com ([209.85.223.180]:33317) by smtp1.cs.Stanford.EDU with esmtpsa (TLSv1.2:AES128-GCM-SHA256:128) (Exim 4.84_2) (envelope-from ) id 1btzxN-0003Ry-Ow for dev@dpdk.org; Tue, 11 Oct 2016 09:31:11 -0700 Received: by mail-io0-f180.google.com with SMTP id q192so30747710iod.0 for ; Tue, 11 Oct 2016 09:31:09 -0700 (PDT) X-Gm-Message-State: AA6/9RlmK8r5L+43Ewd34sYGtqQwWJoAZJwCiPvxoxK/Bb5yY5RO/UL+20IvIuX8QZl0ArVyOTW0MwVb5LDl5w== X-Received: by 10.107.7.135 with SMTP id g7mr7127389ioi.15.1476203469387; Tue, 11 Oct 2016 09:31:09 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.98.67 with HTTP; Tue, 11 Oct 2016 09:30:28 -0700 (PDT) In-Reply-To: <2532748.dRiGlJefCg@xps13> References: <20160928204244.8288-1-ouster@cs.stanford.edu> <20161010223933.5924-1-ouster@cs.stanford.edu> <2532748.dRiGlJefCg@xps13> From: John Ousterhout Date: Tue, 11 Oct 2016 09:30:28 -0700 X-Gmail-Original-Message-ID: Message-ID: To: Thomas Monjalon Cc: dev@dpdk.org X-Spam-Score: -1.0 X-Spam-Level: X-Spam-Checker-Version: SpamAssassin on smtp1.cs.Stanford.EDU X-Scan-Signature: 0ea9888c921c024d7a70b5fc6384e66b Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v2] log: respect rte_openlog_stream calls before rte_eal_init X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Oct 2016 16:31:12 -0000 On Tue, Oct 11, 2016 at 1:08 AM, Thomas Monjalon wrote: > > 2016-10-10 15:39, John Ousterhout: > > ... > > > > Note: I see from the code that Linux and BSD set different default streams: > > Linux uses stdout, while BSD uses stderr. This patch retains the distinction, > > though I'm not sure why it is there. > > I don't know either. > What is best between stdout and stderr for logs? I would guess that stdout makes more sense, since most log entries describe normal operation, not errors. I'm happy to make these consistent, but this would introduce a behavior change for BSD (which currently uses stderr); would that be considered antisocial? > [...] > > -int > > -rte_eal_log_early_init(void) > > -{ > > - rte_openlog_stream(stderr); > > - return 0; > > + rte_eal_set_default(stderr); > > It should be rte_eal_log_set_default. Oops, right; will fix. > > [...] > > /* > > - * called by environment-specific log init function > > + * Called by environment-specific initialization functions. > > */ > > -int > > -rte_eal_common_log_init(FILE *default_log) > > +void > > +rte_eal_log_set_default(FILE *default_log) > > { > > default_log_stream = default_log; > > - rte_openlog_stream(default_log); > > > > #if RTE_LOG_LEVEL >= RTE_LOG_DEBUG > > RTE_LOG(NOTICE, EAL, "Debug logs available - lower performance\n"); > > #endif > > - > > - return 0; > > } > > Do we really need a function for that? > Why not just initialize default_log_stream statically? Right now, different platforms have different defaults. BSD uses stderr always. Linux starts out with stdout as the default, but later during initialization it switches to a different default that logs messages both to standard output and also to syslog. I don't have enough experience with DPDK to know whether a single approach is really right for all systems (or which approach it should be), and since I'm a DPDK newbie I thought it best to take a more conservative approach and avoid behavioral changes. My personal preference would be to minimize mission creep with this patch and leave that behavior in place for someone with more expertise to deal with later (and this issue is orthogonal to the main goal of the patch). But, if unifying the log defaults is considered essential to the patch (and is noncontroversial), I'm willing to implement it. > [...] > > /** > > - * Common log initialization function (private to eal). > > + * Common log initialization function (private to eal). Determines > > + * where log data is written when no call to eal_openlog_stream is > > + * in effect. > > It should be rte_openlog_stream. Oops; fixed. Thanks for the comments. -John-