Message-ID: <20240718093943.qtyc2bdt4oerjuek@quack3>
Date: Thu, 18 Jul 2024 11:39:43 +0200
From: Jan Kara <jack@...e.cz>
To: Mirsad Todorovac <mtodorovac69@...il.com>
Cc: Jan Kara <jack@...e.cz>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"Gustavo A. R. Silva" <gustavoars@...nel.org>,
	Kees Cook <kees@...nel.org>, Christian Brauner <brauner@...nel.org>,
	"Matthew Wilcox (Oracle)" <willy@...radead.org>,
	Al Viro <viro@...iv.linux.org.uk>, Jeff Layton <jlayton@...nel.org>,
	reiserfs-devel@...r.kernel.org
Subject: Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]

On Thu 18-07-24 00:14:24, Mirsad Todorovac wrote:
> 
> 
> On 7/17/24 17:44, Jan Kara wrote:
> > On Tue 16-07-24 19:17:05, Mirsad Todorovac wrote:
> >> On 7/15/24 19:28, Jan Kara wrote:
> >>> Hello Mirsad!
> >>>
> >>> On Wed 10-07-24 20:09:27, Mirsad Todorovac wrote:
> >>>> On the linux-next vanilla next-20240709 tree, I have attempted the seed KCONFIG_SEED=0xEE7AB52F
> >>>> which was known from before to trigger various errors in compile and build process.
> >>>>
> >>>> Though this might seem as contributing to channel noise, Linux refuses to build this config,
> >>>> treating warnings as errors, using this build line:
> >>>>
> >>>> $ time nice make W=1 -k -j 36 |& tee ../err-next-20230709-01a.log; date
> >>>>
> >>>> As I know that the Chief Penguin doesn't like warnings, but I am also aware that there are plenty
> >>>> left, there seems to be more tedious work ahead to make the compilers happy.
> >>>>
> >>>> The compiler output is:
> >>>>
> >>>> ---------------------------------------------------------------------------------------------------------
> >>>> fs/reiserfs/do_balan.c: In function ‘balance_leaf_new_nodes_paste_whole’:
> >>>> fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
> >>>>  1147 |         int leaf_mi;
> >>>>       |             ^~~~~~~
> >>>
> >>> Frankly, I wouldn't bother with reiserfs. The warning is there for ages,
> >>> the code is going to get removed in two releases, so I guess we can live
> >>> with these warnings for a few more months...
> >>
> >> In essence I agree with you, but for sentimental reasons I would like to
> >> keep it because it is my first journaling Linux system on Knoppix 🙂
> > 
> > As much as I understand your sentiment (I have a bit of history with that
> > fs as well) the maintenance cost isn't really worth it and most fs folks
> > will celebrate when it's removed. We have already announced the removal
> > year and half ago and I'm fully for executing that plan at the end of this
> > year.
> > 
> >> Patch is also simple and a no-brainer, as proposed by Mr. Cook:
> >>
> >> -------------------------------><------------------------------------------
> >> diff --git a/fs/reiserfs/do_balan.c b/fs/reiserfs/do_balan.c
> >> index 5129efc6f2e6..fbe73f267853 100644
> >> --- a/fs/reiserfs/do_balan.c
> >> +++ b/fs/reiserfs/do_balan.c
> >> @@ -1144,7 +1144,9 @@ static void balance_leaf_new_nodes_paste_whole(struct tree_balance *tb,
> >>  {
> >>  	struct buffer_head *tbS0 = PATH_PLAST_BUFFER(tb->tb_path);
> >>  	int n = B_NR_ITEMS(tbS0);
> >> +#ifdef CONFIG_REISERFS_CHECK
> >>  	int leaf_mi;
> >> +#endif
> > 
> > Well, I would not like this even for actively maintained code ;) If you
> > want to silence these warnings in this dead code, then I could live with
> > something like:
> > 
> > #if defined( CONFIG_REISERFS_CHECK )
> > #define RFALSE(cond, format, args...) __RASSERT(!(cond), ....)
> > #else
> > - #define RFALSE( cond, format, args... ) do {;} while( 0 )
> > + #define RFALSE( cond, format, args... ) do { (void)cond; } while( 0 )
> > #endif
> 
> Yes, one line change is much smarter than 107 line patch of mine :-)
> 
> Verified, and this line solved all the warnings:
> 
>   CC      fs/reiserfs/bitmap.o
>   CC      fs/reiserfs/do_balan.o
>   CC      fs/reiserfs/namei.o
>   CC      fs/reiserfs/inode.o
>   CC      fs/reiserfs/file.o
>   CC      fs/reiserfs/dir.o
>   CC      fs/reiserfs/fix_node.o
>   CC      fs/reiserfs/super.o
>   CC      fs/reiserfs/prints.o
>   CC      fs/reiserfs/objectid.o
>   CC      fs/reiserfs/lbalance.o
>   CC      fs/reiserfs/ibalance.o
>   CC      fs/reiserfs/stree.o
>   CC      fs/reiserfs/hashes.o
>   CC      fs/reiserfs/tail_conversion.o
>   CC      fs/reiserfs/journal.o
>   CC      fs/reiserfs/resize.o
>   CC      fs/reiserfs/item_ops.o
>   CC      fs/reiserfs/ioctl.o
>   CC      fs/reiserfs/xattr.o
>   CC      fs/reiserfs/lock.o
>   CC      fs/reiserfs/procfs.o
>   AR      fs/reiserfs/built-in.a
> 
> Just FWIW, back then in year 2000/2001 a journaling file system on my
> Knoppix box was a quantum leap - it would simply replay the journal if
> there was a power loss before shutdown. No several minutes of fsck.

Well, there was also ext3 at that time already :-) That's where I became
familiar with the idea of journalling. Reiserfs was interesting to me
because of completely different approach to on-disk format (b-tree with
formatted items), packing of small files / file tails (interesting in 2000,
not so much 20 years later) and reasonable scalability for large
directories.

> I think your idea is great and if you wish a patch could be submitted
> after the merge window.

I'll leave it up to you. If the warnings annoy you, send the patch along
the lines I've proposed (BTW (void)cond should better be (void)(cond) for
macro safety) and I'll push it to Linus.

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

Powered by blists - more mailing lists