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:55:22 +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 4/5] ext4: Use locality group preallocation for small
 closed files
Message-ID: <20220907182522.mvciqy4wrtmnjqv4@riteshh-domain>
References: <20220906150803.375-1-jack@suse.cz>
 <20220906152920.25584-4-jack@suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220906152920.25584-4-jack@suse.cz>
Precedence: bulk

On 22/09/06 05:29PM, Jan Kara wrote:
> Curently we don't use any preallocation when a file is already closed
> when allocating blocks (from writeback code when converting delayed
> allocation). However for small files, using locality group preallocation

I had always wondered about this case. But yes for small files it completely
make sense to enable preallocations for smaller files even though they are
closed.


> is actually desirable as that is not specific to a particular file.
> Rather it is a method to pack small files together to reduce
> fragmentation and for that the fact the file is closed is actually even
> stronger hint the file would benefit from packing. So change the logic
> to allow locality group preallocation in this case.
> 

One thing which comes to mind is that we never discard lg preallocations.
With this change we will always enable lg preallocations for small files. 
These preallocations will then be only discarded when some allocation request
fails which will be retried after doing discard preallocations. 

Though it is not a problem, since any small file allocation will benefit out of
these lgs. But it shouldn't be too large that it starts causing performance hits 
for large files. 
Not for this patch but something to remember maybe ^^^^. 
(While we are internally looking at preallocation space for few optimizations, 
above is something to keep a note of.)


> Signed-off-by: Jan Kara <jack@suse.cz>

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