[RFC PATCH] mm/filemap: make release_folio mandatory and add block_release_folio

[RFC PATCH] mm/filemap: make release_folio mandatory and add block_release_folio

[Deepakkumar Karn]

Currently, filemap_release_folio() falls back to try_to_free_buffers()
when a filesystem's address_space_operations doesn't provide a
release_folio callback. This creates an inconsistent API where some
buffer_head-based filesystems define release_folio while others rely
on the fallback behavior.

Make the release_folio callback mandatory by:

1. Adding block_release_folio() in fs/buffer.c as the default
   implementation for buffer_head-based filesystems. This simply
   calls try_to_free_buffers().

2. Updating all buffer_head-based filesystems that use
   buffer_migrate_folio() to set .release_folio = block_release_folio
   in their address_space_operations.

3. Removing the NULL check and try_to_free_buffers() fallback in
   filemap_release_folio(). Now it always calls the release_folio
   callback when a mapping exists.

With these changes:
- Makes the pattern explicit across all buffer_head filesystems
- Prevents future bugs from missing release_folio implementations

Filesystems updated: adfs, affs, bfs, exfat, ext2, fat, hpfs, isofs,
jfs, minix, nilfs2, ntfs3, omfs, udf, ufs, plus block/fops.c

Filesystems with existing custom implementations (ext4, gfs2, hfs,
hfsplus, jfs metapage, ocfs2) are unchanged.

Suggested-by: Matthew Wilcox <willy@infradead.org>
Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Deepakkumar Karn <dkarn@redhat.com>
---
 block/fops.c                |  1 +
 fs/adfs/inode.c             |  1 +
 fs/affs/file.c              |  3 ++-
 fs/bfs/file.c               |  1 +
 fs/buffer.c                 | 17 +++++++++++++++++
 fs/exfat/inode.c            |  1 +
 fs/ext2/inode.c             |  1 +
 fs/fat/inode.c              |  1 +
 fs/hpfs/file.c              |  1 +
 fs/isofs/inode.c            |  3 ++-
 fs/jfs/inode.c              |  1 +
 fs/minix/inode.c            |  3 ++-
 fs/nilfs2/inode.c           |  1 +
 fs/nilfs2/mdt.c             |  1 +
 fs/ntfs3/inode.c            |  1 +
 fs/omfs/file.c              |  1 +
 fs/udf/inode.c              |  1 +
 fs/ufs/inode.c              |  3 ++-
 include/linux/buffer_head.h |  2 ++
 mm/filemap.c                |  4 ++--
 20 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/block/fops.c b/block/fops.c
index 4d32785b31d9..8adccc5fcb34 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -532,6 +532,7 @@ const struct address_space_operations def_blk_aops = {
 	.write_end	= blkdev_write_end,
 	.migrate_folio	= buffer_migrate_folio_norefs,
 	.is_dirty_writeback = buffer_check_dirty_writeback,
+	.release_folio = block_release_folio,
 };
 #else /* CONFIG_BUFFER_HEAD */
 static int blkdev_read_folio(struct file *file, struct folio *folio)
diff --git a/fs/adfs/inode.c b/fs/adfs/inode.c
index 6830f8bc8d4e..ca7411c40ace 100644
--- a/fs/adfs/inode.c
+++ b/fs/adfs/inode.c
@@ -83,6 +83,7 @@ static const struct address_space_operations adfs_aops = {
 	.write_end	= generic_write_end,
 	.migrate_folio	= buffer_migrate_folio,
 	.bmap		= _adfs_bmap,
+	.release_folio	= block_release_folio,
 };
 
 /*
diff --git a/fs/affs/file.c b/fs/affs/file.c
index 765c3443663e..c03ac926c3a5 100644
--- a/fs/affs/file.c
+++ b/fs/affs/file.c
@@ -464,7 +464,8 @@ const struct address_space_operations affs_aops = {
 	.write_end = affs_write_end,
 	.direct_IO = affs_direct_IO,
 	.migrate_folio = buffer_migrate_folio,
-	.bmap = _affs_bmap
+	.bmap = _affs_bmap,
+	.release_folio = block_release_folio,
 };
 
 static inline struct buffer_head *
diff --git a/fs/bfs/file.c b/fs/bfs/file.c
index d33d6bde992b..34ab8ad50302 100644
--- a/fs/bfs/file.c
+++ b/fs/bfs/file.c
@@ -198,6 +198,7 @@ const struct address_space_operations bfs_aops = {
 	.write_end	= generic_write_end,
 	.migrate_folio	= buffer_migrate_folio,
 	.bmap		= bfs_bmap,
+	.release_folio	= block_release_folio,
 };
 
 const struct inode_operations bfs_file_inops;
diff --git a/fs/buffer.c b/fs/buffer.c
index 838c0c571022..eb856901ac3a 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2914,6 +2914,23 @@ drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free)
 	return false;
 }
 
+/**
+ * block_release_folio - Release buffers attached to a BH-based filesystems.
+ * @folio: The folio.
+ * @gfp: The gfp parameter.
+ *
+ * This is the default release_folio implementation for buffer_head based
+ * filesystems. Filesystems with special requirements
+ * should implement their own release_folio callback.
+ *
+ * Return: true if buffers were released, false otherwise.
+ */
+bool block_release_folio(struct folio *folio, gfp_t gfp)
+{
+	return try_to_free_buffers(folio);
+}
+EXPORT_SYMBOL(block_release_folio);
+
 /**
  * try_to_free_buffers - Release buffers attached to this folio.
  * @folio: The folio.
diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c
index f9501c3a3666..2586fee189d6 100644
--- a/fs/exfat/inode.c
+++ b/fs/exfat/inode.c
@@ -567,6 +567,7 @@ static const struct address_space_operations exfat_aops = {
 	.direct_IO	= exfat_direct_IO,
 	.bmap		= exfat_aop_bmap,
 	.migrate_folio	= buffer_migrate_folio,
+	.release_folio	= block_release_folio,
 };
 
 static inline unsigned long exfat_hash(loff_t i_pos)
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index dbfe9098a124..632463a203e4 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -980,6 +980,7 @@ const struct address_space_operations ext2_aops = {
 	.migrate_folio		= buffer_migrate_folio,
 	.is_partially_uptodate	= block_is_partially_uptodate,
 	.error_remove_folio	= generic_error_remove_folio,
+	.release_folio = block_release_folio,
 };
 
 static const struct address_space_operations ext2_dax_aops = {
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 0b6009cd1844..b6f13927e75b 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -348,6 +348,7 @@ static const struct address_space_operations fat_aops = {
 	.direct_IO	= fat_direct_IO,
 	.bmap		= _fat_bmap,
 	.migrate_folio	= buffer_migrate_folio,
+	.release_folio = block_release_folio,
 };
 
 /*
diff --git a/fs/hpfs/file.c b/fs/hpfs/file.c
index 29e876705369..b242439bf52f 100644
--- a/fs/hpfs/file.c
+++ b/fs/hpfs/file.c
@@ -252,6 +252,7 @@ const struct address_space_operations hpfs_aops = {
 	.write_end = hpfs_write_end,
 	.bmap = _hpfs_bmap,
 	.migrate_folio = buffer_migrate_folio,
+	.release_folio = block_release_folio,
 };
 
 const struct file_operations hpfs_file_ops =
diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
index b7cbe126faf3..d79865bc86b3 100644
--- a/fs/isofs/inode.c
+++ b/fs/isofs/inode.c
@@ -1162,7 +1162,8 @@ static sector_t _isofs_bmap(struct address_space *mapping, sector_t block)
 static const struct address_space_operations isofs_aops = {
 	.read_folio = isofs_read_folio,
 	.readahead = isofs_readahead,
-	.bmap = _isofs_bmap
+	.bmap = _isofs_bmap,
+	.release_folio = block_release_folio,
 };
 
 static int isofs_read_level3_size(struct inode *inode)
diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
index 4709762713ef..f4c9c84fd75a 100644
--- a/fs/jfs/inode.c
+++ b/fs/jfs/inode.c
@@ -364,6 +364,7 @@ const struct address_space_operations jfs_aops = {
 	.bmap		= jfs_bmap,
 	.direct_IO	= jfs_direct_IO,
 	.migrate_folio	= buffer_migrate_folio,
+	.release_folio	= block_release_folio,
 };
 
 /*
diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index 51ea9bdc813f..1d351d01a0c7 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -486,7 +486,8 @@ static const struct address_space_operations minix_aops = {
 	.write_end = generic_write_end,
 	.migrate_folio = buffer_migrate_folio,
 	.bmap = minix_bmap,
-	.direct_IO = noop_direct_IO
+	.direct_IO = noop_direct_IO,
+	.release_folio = block_release_folio,
 };
 
 static const struct inode_operations minix_symlink_inode_operations = {
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 51bde45d5865..b8263da21038 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -280,6 +280,7 @@ const struct address_space_operations nilfs_aops = {
 	.direct_IO		= nilfs_direct_IO,
 	.migrate_folio		= buffer_migrate_folio_norefs,
 	.is_partially_uptodate  = block_is_partially_uptodate,
+	.release_folio		= block_release_folio,
 };
 
 const struct address_space_operations nilfs_buffer_cache_aops = {
diff --git a/fs/nilfs2/mdt.c b/fs/nilfs2/mdt.c
index 946b0d3534a5..bc1c620e2dce 100644
--- a/fs/nilfs2/mdt.c
+++ b/fs/nilfs2/mdt.c
@@ -443,6 +443,7 @@ static const struct address_space_operations def_mdt_aops = {
 	.invalidate_folio	= block_invalidate_folio,
 	.writepages		= nilfs_mdt_writeback,
 	.migrate_folio		= buffer_migrate_folio_norefs,
+	.release_folio		= block_release_folio,
 };
 
 static const struct inode_operations def_mdt_iops;
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 0a9ac5efeb67..b671803147b5 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -2102,6 +2102,7 @@ const struct address_space_operations ntfs_aops = {
 	.dirty_folio	= block_dirty_folio,
 	.migrate_folio	= buffer_migrate_folio,
 	.invalidate_folio = block_invalidate_folio,
+	.release_folio	= block_release_folio,
 };
 
 const struct address_space_operations ntfs_aops_cmpr = {
diff --git a/fs/omfs/file.c b/fs/omfs/file.c
index 49a1de5a827f..25f88a2f36de 100644
--- a/fs/omfs/file.c
+++ b/fs/omfs/file.c
@@ -376,5 +376,6 @@ const struct address_space_operations omfs_aops = {
 	.write_end = generic_write_end,
 	.bmap = omfs_bmap,
 	.migrate_folio = buffer_migrate_folio,
+	.release_folio = block_release_folio,
 };
 
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 7fae8002344a..5ba17373f9b1 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -335,6 +335,7 @@ const struct address_space_operations udf_aops = {
 	.direct_IO	= udf_direct_IO,
 	.bmap		= udf_bmap,
 	.migrate_folio	= buffer_migrate_folio,
+	.release_folio	= block_release_folio,
 };
 
 /*
diff --git a/fs/ufs/inode.c b/fs/ufs/inode.c
index e2b0a35de2a7..4eed8f530737 100644
--- a/fs/ufs/inode.c
+++ b/fs/ufs/inode.c
@@ -514,7 +514,8 @@ const struct address_space_operations ufs_aops = {
 	.write_begin = ufs_write_begin,
 	.write_end = ufs_write_end,
 	.migrate_folio = buffer_migrate_folio,
-	.bmap = ufs_bmap
+	.bmap = ufs_bmap,
+	.release_folio = block_release_folio,
 };
 
 static void ufs_set_inode_ops(struct inode *inode)
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index b16b88bfbc3e..16c793dad6f6 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -515,6 +515,7 @@ bool block_dirty_folio(struct address_space *mapping, struct folio *folio);
 
 void buffer_init(void);
 bool try_to_free_buffers(struct folio *folio);
+bool block_release_folio(struct folio *folio, gfp_t gfp);
 int inode_has_buffers(struct inode *inode);
 void invalidate_inode_buffers(struct inode *inode);
 int remove_inode_buffers(struct inode *inode);
@@ -528,6 +529,7 @@ extern int buffer_heads_over_limit;
 
 static inline void buffer_init(void) {}
 static inline bool try_to_free_buffers(struct folio *folio) { return true; }
+static inline bool block_release_folio(struct folio *folio, gfp_t gfp) { return true; }
 static inline int inode_has_buffers(struct inode *inode) { return 0; }
 static inline void invalidate_inode_buffers(struct inode *inode) {}
 static inline int remove_inode_buffers(struct inode *inode) { return 1; }
diff --git a/mm/filemap.c b/mm/filemap.c
index ebd75684cb0a..5af38d81498d 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -4490,9 +4490,9 @@ bool filemap_release_folio(struct folio *folio, gfp_t gfp)
 	if (folio_test_writeback(folio))
 		return false;
 
-	if (mapping && mapping->a_ops->release_folio)
+	if (mapping)
 		return mapping->a_ops->release_folio(folio, gfp);
-	return try_to_free_buffers(folio);
+	return true;
 }
 EXPORT_SYMBOL(filemap_release_folio);
 
-- 
2.52.0

Re: [RFC PATCH] mm/filemap: make release_folio mandatory and add block_release_folio

[Jan Kara]

On Sat 20-12-25 01:07:52, Deepakkumar Karn wrote:
> Currently, filemap_release_folio() falls back to try_to_free_buffers()
> when a filesystem's address_space_operations doesn't provide a
> release_folio callback. This creates an inconsistent API where some
> buffer_head-based filesystems define release_folio while others rely
> on the fallback behavior.
> 
> Make the release_folio callback mandatory by:
> 
> 1. Adding block_release_folio() in fs/buffer.c as the default
>    implementation for buffer_head-based filesystems. This simply
>    calls try_to_free_buffers().
> 
> 2. Updating all buffer_head-based filesystems that use
>    buffer_migrate_folio() to set .release_folio = block_release_folio
>    in their address_space_operations.
> 
> 3. Removing the NULL check and try_to_free_buffers() fallback in
>    filemap_release_folio(). Now it always calls the release_folio
>    callback when a mapping exists.
> 
> With these changes:
> - Makes the pattern explicit across all buffer_head filesystems
> - Prevents future bugs from missing release_folio implementations
> 
> Filesystems updated: adfs, affs, bfs, exfat, ext2, fat, hpfs, isofs,
> jfs, minix, nilfs2, ntfs3, omfs, udf, ufs, plus block/fops.c
> 
> Filesystems with existing custom implementations (ext4, gfs2, hfs,
> hfsplus, jfs metapage, ocfs2) are unchanged.
> 
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Deepakkumar Karn <dkarn@redhat.com>

...

> diff --git a/fs/affs/file.c b/fs/affs/file.c
> index 765c3443663e..c03ac926c3a5 100644
> --- a/fs/affs/file.c
> +++ b/fs/affs/file.c
> @@ -464,7 +464,8 @@ const struct address_space_operations affs_aops = {
>  	.write_end = affs_write_end,
>  	.direct_IO = affs_direct_IO,
>  	.migrate_folio = buffer_migrate_folio,
> -	.bmap = _affs_bmap
> +	.bmap = _affs_bmap,
> +	.release_folio = block_release_folio,
>  };
>  

I think affs_aops_ofs need .release_folio as well...

Also befs_aops, ecryptfs_aops, efs_aops, vxfs_aops, hfs_aops, hfsplus_aops,
nilfs_buffer_cache_aops, qnx4_aops, qnx6_aops definitely need .release_folio. ntfs_aops_cmpr
might need it as well - not sure.

> diff --git a/fs/buffer.c b/fs/buffer.c
> index 838c0c571022..eb856901ac3a 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2914,6 +2914,23 @@ drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free)
>  	return false;
>  }
>  
> +/**
> + * block_release_folio - Release buffers attached to a BH-based filesystems.
> + * @folio: The folio.
	      ^^ "The folio with attached buffers."

> + * @gfp: The gfp parameter.
	    ^^ "Memory allocation flags (and I/O mode)."

> + *
> + * This is the default release_folio implementation for buffer_head based
> + * filesystems. Filesystems with special requirements
> + * should implement their own release_folio callback.
> + *
> + * Return: true if buffers were released, false otherwise.
> + */

Otherwise the patch looks good to me.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [RFC PATCH] mm/filemap: make release_folio mandatory and add block_release_folio

[Matthew Wilcox]

On Mon, Dec 22, 2025 at 01:38:51PM +0100, Jan Kara wrote:
> > Filesystems updated: adfs, affs, bfs, exfat, ext2, fat, hpfs, isofs,
> > jfs, minix, nilfs2, ntfs3, omfs, udf, ufs, plus block/fops.c
> 
> I think affs_aops_ofs need .release_folio as well...
> 
> Also befs_aops, ecryptfs_aops, efs_aops, vxfs_aops, hfs_aops, hfsplus_aops,
> nilfs_buffer_cache_aops, qnx4_aops, qnx6_aops definitely need .release_folio. ntfs_aops_cmpr
> might need it as well - not sure.

If we were using a real programming language, we'd have a class for
BH-based filesystems, inherit from it and override each method.  But since
we aren't, let's see what we can do in C ...

#define BH_DEFAULT_AOPS						\
	.dirty_folio		= block_dirty_folio,		\
	.invalidate_folio	= block_invalidate_folio,	\
	.migrate_folio		= buffer_migrate_folio,		\
	.is_partially_uptodate  = block_is_partially_uptodate,	\
	.error_remove_folio     = generic_error_remove_folio,

As I understand C, later initialisers override earlier ones [1].

GCC does have an optional warning (-Woverride-init) which is included in
-W but not -Wall.  Some parts of the kernel explicitly turn this off, either
like this:

arch/arm64/kvm/Makefile:CFLAGS_sys_regs.o += -Wno-override-init

or like this:

drivers/gpu/drm/i915/display/intel_display_device.c:__diag_ignore_all("-Woverride-init", "Allow field initialization overrides for display info");

I'd be happy to do either of those for filesystems.


[1] Test program:

struct ops { void (*frob)(void); };

static void my_frob(void) { return; }
void generic_frob(void);

struct ops my_ops = {
        .frob = generic_frob,
        .frob = my_frob,
};

int main(void)
{
        return 0;
}

$ gcc -Wall -o test2 test2.c

links without any missing symbols

Re: [RFC PATCH] mm/filemap: make release_folio mandatory and add block_release_folio

[Jan Kara]

On Mon 22-12-25 15:13:26, Matthew Wilcox wrote:
> On Mon, Dec 22, 2025 at 01:38:51PM +0100, Jan Kara wrote:
> > > Filesystems updated: adfs, affs, bfs, exfat, ext2, fat, hpfs, isofs,
> > > jfs, minix, nilfs2, ntfs3, omfs, udf, ufs, plus block/fops.c
> > 
> > I think affs_aops_ofs need .release_folio as well...
> > 
> > Also befs_aops, ecryptfs_aops, efs_aops, vxfs_aops, hfs_aops, hfsplus_aops,
> > nilfs_buffer_cache_aops, qnx4_aops, qnx6_aops definitely need .release_folio. ntfs_aops_cmpr
> > might need it as well - not sure.
> 
> If we were using a real programming language, we'd have a class for
> BH-based filesystems, inherit from it and override each method.  But since
> we aren't, let's see what we can do in C ...
> 
> #define BH_DEFAULT_AOPS						\
> 	.dirty_folio		= block_dirty_folio,		\
> 	.invalidate_folio	= block_invalidate_folio,	\
> 	.migrate_folio		= buffer_migrate_folio,		\
> 	.is_partially_uptodate  = block_is_partially_uptodate,	\
> 	.error_remove_folio     = generic_error_remove_folio,
> 
> As I understand C, later initialisers override earlier ones [1].

Frankly, I'm not 100% convinced this will be a win and there is definitely
a potential for surprises because filesystems tend to do weird things so
this will need to carefully go filesystem by filesystem... So I wouldn't
complicate this patch with a change like this. Let's clean up
.release_folio first and we can look into cutting boilerplate
initializations later.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [RFC PATCH] mm/filemap: make release_folio mandatory and add block_release_folio

[Deepak Karn]

On Mon, Dec 22, 2025 at 9:18 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 22-12-25 15:13:26, Matthew Wilcox wrote:
> > On Mon, Dec 22, 2025 at 01:38:51PM +0100, Jan Kara wrote:
> > > > Filesystems updated: adfs, affs, bfs, exfat, ext2, fat, hpfs, isofs,
> > > > jfs, minix, nilfs2, ntfs3, omfs, udf, ufs, plus block/fops.c
> > >
> > > I think affs_aops_ofs need .release_folio as well...
> > >
> > > Also befs_aops, ecryptfs_aops, efs_aops, vxfs_aops, hfs_aops, hfsplus_aops,
> > > nilfs_buffer_cache_aops, qnx4_aops, qnx6_aops definitely need .release_folio. ntfs_aops_cmpr
> > > might need it as well - not sure.
> >
> > If we were using a real programming language, we'd have a class for
> > BH-based filesystems, inherit from it and override each method.  But since
> > we aren't, let's see what we can do in C ...
> >
> > #define BH_DEFAULT_AOPS                                               \
> >       .dirty_folio            = block_dirty_folio,            \
> >       .invalidate_folio       = block_invalidate_folio,       \
> >       .migrate_folio          = buffer_migrate_folio,         \
> >       .is_partially_uptodate  = block_is_partially_uptodate,  \
> >       .error_remove_folio     = generic_error_remove_folio,
> >
> > As I understand C, later initialisers override earlier ones [1].
>
> Frankly, I'm not 100% convinced this will be a win and there is definitely
> a potential for surprises because filesystems tend to do weird things so
> this will need to carefully go filesystem by filesystem... So I wouldn't
> complicate this patch with a change like this. Let's clean up
> .release_folio first and we can look into cutting boilerplate
> initializations later.
>

Thank you Jan and Matthew for the review.

@Jan, I will address the feedback for adding release_folio, in missing
filesystems. I will check
and add it wherever I find it missing/needed.

   - affs_aops_ofs
   - befs_aops
   - ecryptfs_aops
   - efs_aops
   - vxfs_aops
   - hfs_aops
   - hfsplus_aops
   - nilfs_buffer_cache_aops
   - qnx4_aops
   - qnx6_aops
   - ntfs_aops_cmpr

As there are more changes, and to be honest this being one of my first
patches of this scale, I am
trying to test it as thoroughly as possible.

@Matthew, I see what you were suggesting for the BH_DEFAULT_AOPS
macro, it would be
nice to have, but let me first try to get all the filesystems that
need the .release_folio
cleanup first. Once these changes are acceptable I can explore
reducing boilerplate
suggestions as a follow-up.


Thanks,
Deepakkumar Karn