[PATCH] fs: efs: namei: Use __free() over manual resource management

[PATCH] fs: efs: namei: Use __free() over manual resource management

[Maxwell Doose]

Define a __free() for brelse() called brelease.

The current code uses manual management of the file buffer heads. Remove
manual brelse statements and initialize bh with __free(brelease)
which removes and modernizes code.

Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 fs/efs/namei.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/efs/namei.c b/fs/efs/namei.c
index 38961ee1d1af..ce7794d55e47 100644
--- a/fs/efs/namei.c
+++ b/fs/efs/namei.c
@@ -8,15 +8,15 @@
  */
 
 #include <linux/buffer_head.h>
+#include <linux/cleanup.h>
 #include <linux/string.h>
 #include <linux/exportfs.h>
 #include "efs.h"
 
+DEFINE_FREE(brelease, struct buffer_head *, if (_T) brelse(_T))
 
 static efs_ino_t efs_find_entry(struct inode *inode, const char *name, int len)
 {
-	struct buffer_head *bh;
-
 	int			slot, namelen;
 	char			*nameptr;
 	struct efs_dir		*dirblock;
@@ -30,7 +30,8 @@ static efs_ino_t efs_find_entry(struct inode *inode, const char *name, int len)
 
 	for(block = 0; block < inode->i_blocks; block++) {
 
-		bh = sb_bread(inode->i_sb, efs_bmap(inode, block));
+		struct buffer_head *bh __free(brelease) = sb_bread(inode->i_sb,
+								   bfs_bmap(inode, block));
 		if (!bh) {
 			pr_err("%s(): failed to read dir block %d\n",
 			       __func__, block);
@@ -41,7 +42,6 @@ static efs_ino_t efs_find_entry(struct inode *inode, const char *name, int len)
 
 		if (be16_to_cpu(dirblock->magic) != EFS_DIRBLK_MAGIC) {
 			pr_err("%s(): invalid directory block\n", __func__);
-			brelse(bh);
 			return 0;
 		}
 
@@ -53,11 +53,9 @@ static efs_ino_t efs_find_entry(struct inode *inode, const char *name, int len)
 
 			if ((namelen == len) && (!memcmp(name, nameptr, len))) {
 				inodenum = be32_to_cpu(dirslot->inode);
-				brelse(bh);
 				return inodenum;
 			}
 		}
-		brelse(bh);
 	}
 	return 0;
 }
-- 
2.54.0

Re: [PATCH] fs: efs: namei: Use __free() over manual resource management

[Maxwell Doose]

On Sat, Jun 6, 2026 at 1:20 PM Maxwell Doose <m32285159@gmail.com> wrote:
>
> Define a __free() for brelse() called brelease.
>
> The current code uses manual management of the file buffer heads. Remove
> manual brelse statements and initialize bh with __free(brelease)
> which removes and modernizes code.
>
> Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> ---
>  fs/efs/namei.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/fs/efs/namei.c b/fs/efs/namei.c
> index 38961ee1d1af..ce7794d55e47 100644
> --- a/fs/efs/namei.c
> +++ b/fs/efs/namei.c
> @@ -8,15 +8,15 @@
>   */
>
>  #include <linux/buffer_head.h>
> +#include <linux/cleanup.h>
>  #include <linux/string.h>
>  #include <linux/exportfs.h>
>  #include "efs.h"
>
> +DEFINE_FREE(brelease, struct buffer_head *, if (_T) brelse(_T))
>
>  static efs_ino_t efs_find_entry(struct inode *inode, const char *name, int len)
>  {
> -       struct buffer_head *bh;
> -
>         int                     slot, namelen;
>         char                    *nameptr;
>         struct efs_dir          *dirblock;
> @@ -30,7 +30,8 @@ static efs_ino_t efs_find_entry(struct inode *inode, const char *name, int len)
>
>         for(block = 0; block < inode->i_blocks; block++) {
>
> -               bh = sb_bread(inode->i_sb, efs_bmap(inode, block));
> +               struct buffer_head *bh __free(brelease) = sb_bread(inode->i_sb,
> +                                                                  bfs_bmap(inode, block));

Made a mistake here while moving :( should be efs_bmap(). Can send a
v2 if preferred.

-- 
best regards,
max



>                 if (!bh) {
>                         pr_err("%s(): failed to read dir block %d\n",
>                                __func__, block);
> @@ -41,7 +42,6 @@ static efs_ino_t efs_find_entry(struct inode *inode, const char *name, int len)
>
>                 if (be16_to_cpu(dirblock->magic) != EFS_DIRBLK_MAGIC) {
>                         pr_err("%s(): invalid directory block\n", __func__);
> -                       brelse(bh);
>                         return 0;
>                 }
>
> @@ -53,11 +53,9 @@ static efs_ino_t efs_find_entry(struct inode *inode, const char *name, int len)
>
>                         if ((namelen == len) && (!memcmp(name, nameptr, len))) {
>                                 inodenum = be32_to_cpu(dirslot->inode);
> -                               brelse(bh);
>                                 return inodenum;
>                         }
>                 }
> -               brelse(bh);
>         }
>         return 0;
>  }
> --
> 2.54.0
>

Re: [PATCH] fs: efs: namei: Use __free() over manual resource management

[Jan Kara]

On Sat 06-06-26 21:58:15, Maxwell Doose wrote:
> On Sat, Jun 6, 2026 at 1:20 PM Maxwell Doose <m32285159@gmail.com> wrote:
> >
> > Define a __free() for brelse() called brelease.
> >
> > The current code uses manual management of the file buffer heads. Remove
> > manual brelse statements and initialize bh with __free(brelease)
> > which removes and modernizes code.
> >
> > Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> > ---
> >  fs/efs/namei.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/efs/namei.c b/fs/efs/namei.c
> > index 38961ee1d1af..ce7794d55e47 100644
> > --- a/fs/efs/namei.c
> > +++ b/fs/efs/namei.c
> > @@ -8,15 +8,15 @@
> >   */
> >
> >  #include <linux/buffer_head.h>
> > +#include <linux/cleanup.h>
> >  #include <linux/string.h>
> >  #include <linux/exportfs.h>
> >  #include "efs.h"
> >
> > +DEFINE_FREE(brelease, struct buffer_head *, if (_T) brelse(_T))
> >
> >  static efs_ino_t efs_find_entry(struct inode *inode, const char *name, int len)
> >  {
> > -       struct buffer_head *bh;
> > -
> >         int                     slot, namelen;
> >         char                    *nameptr;
> >         struct efs_dir          *dirblock;
> > @@ -30,7 +30,8 @@ static efs_ino_t efs_find_entry(struct inode *inode, const char *name, int len)
> >
> >         for(block = 0; block < inode->i_blocks; block++) {
> >
> > -               bh = sb_bread(inode->i_sb, efs_bmap(inode, block));
> > +               struct buffer_head *bh __free(brelease) = sb_bread(inode->i_sb,
> > +                                                                  bfs_bmap(inode, block));
> 
> Made a mistake here while moving :( should be efs_bmap(). Can send a
> v2 if preferred.

Well, that happens but that also shows you didn't even try to compile the
thing before sending the patch. This is not how the kernel development
should work, let alone maintenance... Please test your changes before
sending them.

Separate thing is that I don't think changes like this to a dead filesystem
as EFS are really useful. So as others have suggested the most useful
action you could do for EFS would be to add deprecation notice on kconfig
and print deprecation message on mount (like we did with other filesystems
we removed). Set the removal date to the end of 2026 and see if some user
pops up. Because I suspect nobody used this code for years.

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

Re: [PATCH] fs: efs: namei: Use __free() over manual resource management

[Maxwell Doose]

On Tue, 9 Jun 2026 11:58:40 +0200
Jan Kara <jack@suse.cz> wrote:

> On Sat 06-06-26 21:58:15, Maxwell Doose wrote:
> > On Sat, Jun 6, 2026 at 1:20 PM Maxwell Doose <m32285159@gmail.com> wrote:  
> > >
> > > Define a __free() for brelse() called brelease.
> > >
> > > The current code uses manual management of the file buffer heads. Remove
> > > manual brelse statements and initialize bh with __free(brelease)
> > > which removes and modernizes code.
> > >
> > > Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> > > ---
> > >  fs/efs/namei.c | 10 ++++------
> > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/fs/efs/namei.c b/fs/efs/namei.c
> > > index 38961ee1d1af..ce7794d55e47 100644
> > > --- a/fs/efs/namei.c
> > > +++ b/fs/efs/namei.c
> > > @@ -8,15 +8,15 @@
> > >   */
> > >
> > >  #include <linux/buffer_head.h>
> > > +#include <linux/cleanup.h>
> > >  #include <linux/string.h>
> > >  #include <linux/exportfs.h>
> > >  #include "efs.h"
> > >
> > > +DEFINE_FREE(brelease, struct buffer_head *, if (_T) brelse(_T))
> > >
> > >  static efs_ino_t efs_find_entry(struct inode *inode, const char *name, int len)
> > >  {
> > > -       struct buffer_head *bh;
> > > -
> > >         int                     slot, namelen;
> > >         char                    *nameptr;
> > >         struct efs_dir          *dirblock;
> > > @@ -30,7 +30,8 @@ static efs_ino_t efs_find_entry(struct inode *inode, const char *name, int len)
> > >
> > >         for(block = 0; block < inode->i_blocks; block++) {
> > >
> > > -               bh = sb_bread(inode->i_sb, efs_bmap(inode, block));
> > > +               struct buffer_head *bh __free(brelease) = sb_bread(inode->i_sb,
> > > +                                                                  bfs_bmap(inode, block));  
> > 
> > Made a mistake here while moving :( should be efs_bmap(). Can send a
> > v2 if preferred.  
> 
> Well, that happens but that also shows you didn't even try to compile the
> thing before sending the patch. This is not how the kernel development
> should work, let alone maintenance... Please test your changes before
> sending them.
>

I did and added the fix for it but I think I forgot to amend my commit
:(

>
> Separate thing is that I don't think changes like this to a dead filesystem
> as EFS are really useful. So as others have suggested the most useful
> action you could do for EFS would be to add deprecation notice on kconfig
> and print deprecation message on mount (like we did with other filesystems
> we removed). Set the removal date to the end of 2026 and see if some user
> pops up. Because I suspect nobody used this code for years.
> 

Alright then. If that's the consensus then I can do that.

-- 
best regards,
max