From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:37560 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725862AbeIENwI (ORCPT ); Wed, 5 Sep 2018 09:52:08 -0400 Date: Wed, 5 Sep 2018 11:22:32 +0200 From: David Sterba To: Sasha Levin Cc: "stable@vger.kernel.org" , Nikolay Borisov Subject: Re: [PATCH AUTOSEL 4.18 107/113] btrfs: Rewrite retry logic in do_chunk_alloc Message-ID: <20180905092232.GU24025@suse.cz> Reply-To: dsterba@suse.cz References: <20180830180714.36167-1-alexander.levin@microsoft.com> <20180830180714.36167-41-alexander.levin@microsoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180830180714.36167-41-alexander.levin@microsoft.com> Sender: stable-owner@vger.kernel.org List-ID: On Thu, Aug 30, 2018 at 06:08:33PM +0000, Sasha Levin wrote: > From: Nikolay Borisov > > [ Upstream commit 2556fbb0bead7929ddf67f8b4184f434cee4e7d7 ] > > do_chunk_alloc implements logic to detect whether there is currently > pending chunk allocation (by means of space_info->chunk_alloc being > set) and if so it loops around to the 'again' label. Additionally, > based on the state of the space_info (e.g. whether it's full or not) > and the return value of should_alloc_chunk() it decides whether this > is a "hard" error (ENOSPC) or we can just return 0. > > This patch refactors all of this: > > 1. Put order to the scattered ifs handling the various cases in an > easy-to-read if {} else if{} branches. This makes clear the various > cases we are interested in handling. > > 2. Call should_alloc_chunk only once and use the result in the > if/else if constructs. All of this is done under space_info->lock, so > even before multiple calls of should_alloc_chunk were unnecessary. > > 3. Rewrite the "do {} while()" loop currently implemented via label > into an explicit loop construct. > > 4. Move the mutex locking for the case where the caller is the one doing > the allocation. For the case where the caller needs to wait a concurrent > allocation, introduce a pair of mutex_lock/mutex_unlock to act as a > barrier and reword the comment. > > 5. Switch local vars to bool type where pertinent. > > All in all this shouldn't introduce any functional changes. Unless there are other followup patches "No functional changes" is a hint for autosel not to pick the patch.