Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 7 Sep 2022 23:35:07 +0530
From:   "Ritesh Harjani (IBM)" <ritesh.list@gmail.com>
To:     Jan Kara <jack@suse.cz>
Cc:     Ted Tso <tytso@mit.edu>, linux-ext4@vger.kernel.org,
        Thorsten Leemhuis <regressions@leemhuis.info>,
        Ojaswin Mujoo <ojaswin@linux.ibm.com>,
        Stefan Wahren <stefan.wahren@i2se.com>,
        Andreas Dilger <adilger.kernel@dilger.ca>
Subject: Re: [PATCH 2/5] ext4: Avoid unnecessary spreading of allocations
 among groups
Message-ID: <20220907180507.3byq5uts42e6dpkn@riteshh-domain>
References: <20220906150803.375-1-jack@suse.cz>
 <20220906152920.25584-2-jack@suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220906152920.25584-2-jack@suse.cz>
Precedence: bulk

On 22/09/06 05:29PM, Jan Kara wrote:
> mb_set_largest_free_order() updates lists containing groups with largest
> chunk of free space of given order. The way it updates it leads to
> always moving the group to the tail of the list. Thus allocations
> looking for free space of given order effectively end up cycling through
> all groups (and due to initialization in last to first order). This
> spreads allocations among block groups which reduces performance for
> rotating disks or low-end flash media. Change
> mb_set_largest_free_order() to only update lists if the order of the
> largest free chunk in the group changed.

Nice and clear explaination. Thanks :)

This change also looks good to me.
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>


One other thought to further optimize - 
Will it make a difference if rather then adding the group to the tail of the list, 
we add that group to the head of sbi->s_mb_largest_free_orders[new_order]. 

This is because this group is the latest from where blocks were allocated/freed,
and hence the next allocation should first try from this group in order to keep 
the files/extents blocks close to each other? 
(That sometimes might help with disk firmware to avoid doing discards if the freed 
block can be reused?)

Or does goal block will always cover that case by default and we might never
require this? Maybe in a case of a new file within the same directory where 
the goal group has no free blocks, but the last group attempted should be 
retried first?

-ritesh