Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 24 Oct 2019 09:00:37 -0400
From:   Steven Rostedt <rostedt@goodmis.org>
To:     Divya Indi <divya.indi@oracle.com>
Cc:     linux-kernel@vger.kernel.org, Joe Jin <joe.jin@oracle.com>,
        Srinivas Eeda <srinivas.eeda@oracle.com>,
        Aruna Ramakrishna <aruna.ramakrishna@oracle.com>
Subject: Re: [PATCH 4/5] tracing: Handle the trace array ref counter in new
 functions
Message-ID: <20191024090037.78fe9f30@gandalf.local.home>
In-Reply-To: <2b08751a-4028-2130-9a70-c2aa2d76a31c@oracle.com>
References: <1565805327-579-1-git-send-email-divya.indi@oracle.com>
        <1565805327-579-5-git-send-email-divya.indi@oracle.com>
        <20191015190436.65c8c7a3@gandalf.local.home>
        <4cad186e-ba8b-8e1a-731b-4350a095ba5a@oracle.com>
        <20191022225253.4086195c@oasis.local.home>
        <2b08751a-4028-2130-9a70-c2aa2d76a31c@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 23 Oct 2019 15:57:49 -0700
Divya Indi <divya.indi@oracle.com> wrote:

> Hi Steven,                                                                         
>                                                                                  
> A few clarifications on this discussion on reference counter -                   
>                                                                                  
> 1) We will still need to export trace_array_put() to be used for every           
> trace_array_get_by_name() OR trace_array_create() + trace_array_get().        

I'm fine with exporting trace_array_put, and even trace_array_get.
      
>                                                                                  
> How else will we reduce the reference counter [For eg: When multiple modules     
> lookup the same trace array (say, reference counter = 4)]?                       
>                                                                                  
> 2) tr = trace_array_create("my_tr");                                             
>    trace_array_get(tr);                                                          
>                                                                                  
> Both of these functions will iterate through the list of trace arrays to verify  
> whether the trace array exists (redundant, but more intuitive? Does this seem    
> acceptable?)                                                                     
>                                                                                  
> To avoid iterating twice, we went with increasing ref_ctr in trace_array_create. 
> This necessitated a trace_array_put() in instance_mkdir (Or as suggested below,
> we can do this trace_array_put() in instance_rmdir().)                        
>                                                                                                                
>                                                                                  
> 3) A summary of suggested changes (Let me know if this looks good) -                                              
>                                                                                  
> tr = trace_array_get_by_name("foo-bar"); // ref_ctr++.                           
>                                                                                  
> if (!tr)                                                                         
> {                                                                                
>         // instance_mkdir also causes ref_ctr = 1                                

You'll need locking for anyone who does this, and check the return
status below for "foo-bar" existing already (due to another thread
jumping in here).

-- Steve

>         tr = trace_array_create("foo-bar"); // ref_ctr = 1                       
>         trace_array_get(tr); // ref_ctr++                                        
> }                                                                                
>                                                                                  
> trace_array_printk(.....);                                                       
> trace_array_set_clr_event(......);                                               
> ...                                                                              
> ...                                                                              
> ...                                                                              
> // Done using the trace array.                                                   
> trace_array_put(tr); // ref_ctr--                                                
> ...                                                                              
> ...                                                                              
> ...                                                                              
> // We can now remove the trace array via trace_array_destroy or instance_rmdir()
> trace_array_destroy(tr); // ref_ctr > 1 returns -EBUSY.