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@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> >  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@xxxxxxxx>
SUSE Labs, CR