Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Date:   Fri, 1 Dec 2023 07:20:36 +0000
From:   Carlos Llamas <cmllamas@google.com>
To:     Alice Ryhl <aliceryhl@google.com>
Cc:     Arve =?iso-8859-1?B?SGr4bm5lduVn?= <arve@android.com>,
        Christian Brauner <brauner@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Joel Fernandes <joel@joelfernandes.org>,
        kernel-team@android.com, linux-kernel@vger.kernel.org,
        Martijn Coenen <maco@android.com>,
        Suren Baghdasaryan <surenb@google.com>,
        Todd Kjos <tkjos@android.com>
Subject: Re: [PATCH 17/21] binder: malloc new_buffer outside of locks
Message-ID: <ZWmJRPZRYEtlBQNi@google.com>
References: <20231102185934.773885-18-cmllamas@google.com>
 <20231107090838.260730-1-aliceryhl@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231107090838.260730-1-aliceryhl@google.com>
Precedence: bulk

On Tue, Nov 07, 2023 at 09:08:38AM +0000, Alice Ryhl wrote:
> Carlos Llamas <cmllamas@google.com> writes:
> > Preallocate new_buffer before acquiring the alloc->mutex and hand it
> > down to binder_alloc_new_buf_locked(). The new buffer will be used in
> > the vast majority of requests (measured at 98.2% in field data). The
> > buffer is discarded otherwise. This change is required in preparation
> > for transitioning alloc->mutex into a spinlock in subsequent commits.
> > 
> > Signed-off-by: Carlos Llamas <cmllamas@google.com>
> 
> You also need to free the new buffer here:
> 
> 	if (unlikely(!best_fit)) {
> 		binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
> 				   "%d: binder_alloc_buf size %zd failed, no address space\n",
> 				   alloc->pid, size);
> 		debug_no_space_locked(alloc);
> 		return ERR_PTR(-ENOSPC);
> 	}

Ouch! this is true and there is a second instance that needs the kfree
as well. Thanks for catching it.

--
Carlos Llamas