linux-ext4 - Re: [PATCH 2/5] ext4: Avoid unnecessary spreading of allocations among groups

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220908092449.dl5ar4wbhm5cxii2@riteshh-domain>
Date:   Thu, 8 Sep 2022 14:54:49 +0530
From:   "Ritesh Harjani (IBM)" <ritesh.list@...il.com>
To:     Jan Kara <jack@...e.cz>
Cc:     Ted Tso <tytso@....edu>, linux-ext4@...r.kernel.org,
        Thorsten Leemhuis <regressions@...mhuis.info>,
        Ojaswin Mujoo <ojaswin@...ux.ibm.com>,
        Stefan Wahren <stefan.wahren@...e.com>,
        Andreas Dilger <adilger.kernel@...ger.ca>
Subject: Re: [PATCH 2/5] ext4: Avoid unnecessary spreading of allocations
 among groups

On 22/09/08 10:57AM, Jan Kara wrote:
> On Wed 07-09-22 23:35:07, Ritesh Harjani (IBM) wrote:
> > 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@...il.com>
> 
> Thanks for review!
> 
> > 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?
> 
> So I was also wondering about this somewhat. I think that goal group will
> take care of keeping file data together so head/tail insertion should not
> matter too much for one file. Maybe if the allocation comes from a
> different inode, then the head/tail insertion matters but then it is not
> certain whether the allocation is actually related and what its order is
> (depending on that we might prefer same / different group) so I've decided
> to just keep things as they are. I agree it might be interesting to
> investigate and experiment with various workloads and see whether the
> head/tail insertion makes a difference for some workload but I think it's a
> separate project.
> 

Sure. Make sense.
Thanks for still sharing your thoughts on it.

-ritesh