linux-kernel - Re: [PATCH 12/12 v2] ring-buffer: Add cached pages when freeing reader page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 13 May 2010 19:23:14 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	linux-kernel@...r.kernel.org
Cc:	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Frederic Weisbecker <fweisbec@...il.com>
Subject: Re: [PATCH 12/12 v2] ring-buffer: Add cached pages when freeing
 reader page

On Thu, 2010-05-13 at 18:10 -0400, Steven Rostedt wrote:
> plain text document attachment
> (0012-ring-buffer-Add-cached-pages-when-freeing-reader-pag.patch)
> From: Steven Rostedt <srostedt@...hat.com>
> 
> When the pages are removed from the ring buffer for things like
> splice they are freed with ring_buffer_free_read_page().
> They are also allocated with ring_buffer_alloc_read_page().
> 
> Currently the ring buffer does not take advantage of this situation.
> Every time the page is freed, the ring buffer simply frees it.
> When a new page is needed, it allocates it. This means that reading
> several pages with splice will cause a page to be freed and allocated
> several times. This is simply a waste.
> 
> This patch adds a cache of the pages freed (16 max). This allows
> the pages to be reused quickly without need to go back to the memory
> pool.

Ah,

Silly me forgot to add locking. I was thinking that these were called
under a lock. They are in the above layers, but we can't rely on that
here.

Since this is the last patch, I'll just remove it from the branch. So
you can pull this branch still, and it will not include this patch.

Thanks!

-- Steve

> Reported-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
> Signed-off-by: Steven Rostedt <rostedt@...dmis.org>
> ---
>  kernel/trace/ring_buffer.c |   41 +++++++++++++++++++++++++++++++++++------
>  1 files changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 7f6059c..40667b2 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -157,6 +157,8 @@ static unsigned long ring_buffer_flags __read_mostly = RB_BUFFERS_ON;
>  
>  #define BUF_PAGE_HDR_SIZE offsetof(struct buffer_data_page, data)
>  
> +#define RB_MAX_FREE_PAGES	16
> +
>  /**
>   * tracing_on - enable all tracing buffers
>   *
> @@ -325,7 +327,10 @@ EXPORT_SYMBOL_GPL(ring_buffer_event_data);
>  #define RB_MISSED_STORED	(1 << 30)
>  
>  struct buffer_data_page {
> -	u64		 time_stamp;	/* page time stamp */
> +	union {
> +		struct buffer_data_page	*next;	/* for free pages */
> +		u64		 time_stamp;	/* page time stamp */
> +	};
>  	local_t		 commit;	/* write committed index */
>  	unsigned char	 data[];	/* data of buffer page */
>  };
> @@ -472,6 +477,9 @@ struct ring_buffer {
>  	atomic_t			record_disabled;
>  	cpumask_var_t			cpumask;
>  
> +	struct buffer_data_page		*free_pages;
> +	int				nr_free_pages;
> +
>  	struct lock_class_key		*reader_lock_key;
>  
>  	struct mutex			mutex;
> @@ -1184,6 +1192,7 @@ EXPORT_SYMBOL_GPL(__ring_buffer_alloc);
>  void
>  ring_buffer_free(struct ring_buffer *buffer)
>  {
> +	struct buffer_data_page *bpage;
>  	int cpu;
>  
>  	get_online_cpus();
> @@ -1200,6 +1209,11 @@ ring_buffer_free(struct ring_buffer *buffer)
>  	kfree(buffer->buffers);
>  	free_cpumask_var(buffer->cpumask);
>  
> +	while (buffer->free_pages) {
> +		bpage = buffer->free_pages;
> +		buffer->free_pages = bpage->next;
> +		free_page((unsigned long)bpage);
> +	};
>  	kfree(buffer);
>  }
>  EXPORT_SYMBOL_GPL(ring_buffer_free);
> @@ -3717,11 +3731,17 @@ void *ring_buffer_alloc_read_page(struct ring_buffer *buffer)
>  	struct buffer_data_page *bpage;
>  	unsigned long addr;
>  
> -	addr = __get_free_page(GFP_KERNEL);
> -	if (!addr)
> -		return NULL;
> +	if (!buffer->free_pages) {
> +		addr = __get_free_page(GFP_KERNEL);
> +		if (!addr)
> +			return NULL;
>  
> -	bpage = (void *)addr;
> +		bpage = (void *)addr;
> +	} else {
> +		bpage = buffer->free_pages;
> +		buffer->free_pages = bpage->next;
> +		buffer->nr_free_pages--;
> +	}
>  
>  	rb_init_page(bpage);
>  
> @@ -3738,7 +3758,16 @@ EXPORT_SYMBOL_GPL(ring_buffer_alloc_read_page);
>   */
>  void ring_buffer_free_read_page(struct ring_buffer *buffer, void *data)
>  {
> -	free_page((unsigned long)data);
> +	struct buffer_data_page *bpage = data;
> +
> +	if (buffer->nr_free_pages >= RB_MAX_FREE_PAGES) {
> +		free_page((unsigned long)data);
> +		return;
> +	}
> +
> +	bpage->next = buffer->free_pages;
> +	buffer->free_pages = bpage;
> +	buffer->nr_free_pages++;
>  }
>  EXPORT_SYMBOL_GPL(ring_buffer_free_read_page);
>  


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/