From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933236AbcGLMvE (ORCPT ); Tue, 12 Jul 2016 08:51:04 -0400 Received: from mga02.intel.com ([134.134.136.20]:8546 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751293AbcGLMvB (ORCPT ); Tue, 12 Jul 2016 08:51:01 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,351,1464678000"; d="scan'208";a="1020115935" Subject: Re: [PATCH v2] relay: Add global mode support for buffer-only channels To: Chris Wilson , Andrew Morton References: <1468221456-15537-1-git-send-email-akash.goel@intel.com> <20160711131709.4b2417f54b2eb9fd0fa19900@linux-foundation.org> <20160712092445.GN6157@nuc-i3427.alporthouse.com> Cc: linux-kernel@vger.kernel.org, viro@zeniv.linux.org.uk, Eduard - Gabriel Munteanu , Tom Zanussi , akash.goel@intel.com From: "Goel, Akash" Message-ID: <62e36dd1-85b3-2fa9-550e-c43780b5b33b@intel.com> Date: Tue, 12 Jul 2016 18:20:06 +0530 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160712092445.GN6157@nuc-i3427.alporthouse.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 7/12/2016 2:54 PM, Chris Wilson wrote: > On Mon, Jul 11, 2016 at 01:17:09PM -0700, Andrew Morton wrote: >> On Mon, 11 Jul 2016 12:47:36 +0530 akash.goel@intel.com wrote: >> >>> From: Akash Goel >>> >>> The following patch added support to use channels with no associated files. >>> relay: add buffer-only channels; useful for early logging >> >> hm, 8 years ago. Normally we refer to previous commits using the form >> 20d8b67c06fa5e74f44e ("relay: add buffer-only channels; useful for >> early logging"). But this one is so old that we should inform readers >> about its vintage, so this form: >> >> commit 20d8b67c06fa5e74f44e80b0a0fd68c8327f7c6a >> Author: Eduard - Gabriel Munteanu >> Date: Fri Jul 25 19:45:12 2008 -0700 >> >> relay: add buffer-only channels; useful for early logging >> >> would be better. >> >>> This is useful when the exact location of relay file is not known or the >>> the parent directory of relay file is not available, while creating the >>> channel and the logging has to start right from the boot. >>> >>> But there was no provision to use global mode with buffer-only channels, >>> which is added by this patch, without modifying the interface where initially >>> there will be a dummy invocation of create_buf_file callback through which >>> kernel client can convey the need of a global buffer. >>> >>> For the use case where drivers/kernel clients want a simple interface for the >>> userspace, which enables them to capture data/logs from relay file in order & >>> without any post processing, support of Global buffer mode is warranted. >>> >>> ... >>> >>> @@ -706,6 +727,7 @@ int relay_late_setup_files(struct rchan *chan, >>> >>> return err; >>> } >>> +EXPORT_SYMBOL_GPL(relay_late_setup_files); >> >> The export is unneeded and undocumented. > > Something like: > > diff --git a/kernel/relay.c b/kernel/relay.c > index 04d7cf3ef8cf..fd86f01de4b2 100644 > --- a/kernel/relay.c > +++ b/kernel/relay.c > @@ -562,6 +562,10 @@ static int relay_hotcpu_callback(struct notifier_block *nb, > * attributes specified. The created channel buffer files > * will be named base_filename0...base_filenameN-1. File > * permissions will be %S_IRUSR. > + * > + * If opening a buffer (@parent = NULL) that you later wish to register > + * in a filesystem, call relay_late_setup_files() once the @parent dentry > + * is available. > */ > struct rchan *relay_open(const char *base_filename, > struct dentry *parent, > @@ -640,8 +644,12 @@ static void __relay_set_buf_dentry(void *info) > * > * Returns 0 if successful, non-zero otherwise. > * > - * Use to setup files for a previously buffer-only channel. > - * Useful to do early tracing in kernel, before VFS is up, for example. > + * Use to setup files for a previously buffer-only channel created > + * by relay_open() with a NULL parent dentry. > + * > + * For example, this is useful for perfomring early tracing in kernel, > + * before VFS is up and then exposing the early results once the dentry > + * is available. > */ > int relay_late_setup_files(struct rchan *chan, > const char *base_filename, > > > with a comment in the changelog that modules using relay_open() in early > init also want to later register their buffer-only relays once debugfs is > available, e.g. i915. Thanks much, will update the documentation as well as the changelog as per the above. But an export of symbol relay_late_setup_files() is still needed, just like relay_open() is exported, in order to make it accessible to modules like i915 ? Best regards Akash > -Chris >