[RFC: 2.6 patch] fs/jbd/journal.c: possible cleanups

[RFC: 2.6 patch] fs/jbd/journal.c: possible cleanups

[Adrian Bunk]

This patch contains the following possible cleanups:
- make the following needlessly global function static:
  - journal_check_used_features()
- remove the following unused EXPORT_SYMBOL's:
  - journal_set_features
  - journal_update_superblock

Signed-off-by: Adrian Bunk <bunk@stusta.de>

---

This patch was already sent on:
- 1 May 2006
- 23 Apr 2006

 fs/jbd/journal.c    |    9 ++++-----
 include/linux/jbd.h |    2 --
 2 files changed, 4 insertions(+), 7 deletions(-)

--- linux-2.6.17-rc1-mm3-full/include/linux/jbd.h.old	2006-04-23 13:29:20.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/include/linux/jbd.h	2006-04-23 13:29:35.000000000 +0200
@@ -908,8 +908,6 @@
 				int start, int len, int bsize);
 extern journal_t * journal_init_inode (struct inode *);
 extern int	   journal_update_format (journal_t *);
-extern int	   journal_check_used_features 
-		   (journal_t *, unsigned long, unsigned long, unsigned long);
 extern int	   journal_check_available_features 
 		   (journal_t *, unsigned long, unsigned long, unsigned long);
 extern int	   journal_set_features 
--- linux-2.6.17-rc1-mm3-full/fs/jbd/journal.c.old	2006-04-23 13:29:42.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/fs/jbd/journal.c	2006-04-23 13:30:37.000000000 +0200
@@ -62,13 +62,10 @@
 EXPORT_SYMBOL(journal_init_dev);
 EXPORT_SYMBOL(journal_init_inode);
 EXPORT_SYMBOL(journal_update_format);
-EXPORT_SYMBOL(journal_check_used_features);
 EXPORT_SYMBOL(journal_check_available_features);
-EXPORT_SYMBOL(journal_set_features);
 EXPORT_SYMBOL(journal_create);
 EXPORT_SYMBOL(journal_load);
 EXPORT_SYMBOL(journal_destroy);
-EXPORT_SYMBOL(journal_update_superblock);
 EXPORT_SYMBOL(journal_abort);
 EXPORT_SYMBOL(journal_errno);
 EXPORT_SYMBOL(journal_ack_err);
@@ -1169,8 +1166,10 @@
  * features.  Return true (non-zero) if it does. 
  **/
 
-int journal_check_used_features (journal_t *journal, unsigned long compat,
-				 unsigned long ro, unsigned long incompat)
+static int journal_check_used_features(journal_t *journal,
+				       unsigned long compat,
+				       unsigned long ro,
+				       unsigned long incompat)
 {
 	journal_superblock_t *sb;
 

Re: [RFC: 2.6 patch] fs/jbd/journal.c: possible cleanups

[Andrew Morton]

Adrian Bunk <bunk@stusta.de> wrote:
>
> - remove the following unused EXPORT_SYMBOL's:
>   - journal_set_features
>   - journal_update_superblock

These might be used by lustre - dunno.

But I'm ducking all patches which alter exports, as usual.  If you can get
them through the subsystem maintainer then good luck.

I'd suggest that we pursue the approach of getting the module loader to
spit a warning when someone uses a going-away-soon export.

Arjan had a patch which did that, but he removed basically _every_
presently-unused export.  I don't think we should do that.  If we're
exporting, for example, the kthread API and we happen to notice that
kthread_bind() isn't presently used by any modules then I don't believe it
makes much sense to unexport kthread_bind().  It makes more sense to export
the kthread API as a whole.

Re: [RFC: 2.6 patch] fs/jbd/journal.c: possible cleanups

[Adrian Bunk]

On Tue, May 16, 2006 at 12:27:31PM -0700, Andrew Morton wrote:
> Adrian Bunk <bunk@stusta.de> wrote:
> >
> > - remove the following unused EXPORT_SYMBOL's:
> >   - journal_set_features
> >   - journal_update_superblock
> 
> These might be used by lustre - dunno.

I don't see this.

> But I'm ducking all patches which alter exports, as usual.  If you can get
> them through the subsystem maintainer then good luck.
>...

Since you replied to this patch:
Who is the subsystem maintainer for jbd?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [RFC: 2.6 patch] fs/jbd/journal.c: possible cleanups

[Andrew Morton]

Adrian Bunk <bunk@stusta.de> wrote:
>
> On Tue, May 16, 2006 at 12:27:31PM -0700, Andrew Morton wrote:
> > Adrian Bunk <bunk@stusta.de> wrote:
> > >
> > > - remove the following unused EXPORT_SYMBOL's:
> > >   - journal_set_features
> > >   - journal_update_superblock
> > 
> > These might be used by lustre - dunno.
> 
> I don't see this.

Lustre doesn't use these?

Still, the jbd API is exported for other filesystems to use.  If these
functions are considered part of that API (they are) then I'd suggest that
they should be exported.

> > But I'm ducking all patches which alter exports, as usual.  If you can get
> > them through the subsystem maintainer then good luck.
> >...
> 
> Since you replied to this patch:
> Who is the subsystem maintainer for jbd?

A few people, including me..

Re: [RFC: 2.6 patch] fs/jbd/journal.c: possible cleanups

[Theodore Tso]

On Tue, May 16, 2006 at 09:39:56PM +0200, Adrian Bunk wrote:
> Since you replied to this patch:
> Who is the subsystem maintainer for jbd?

I'd suggest sending mail to ext2-devel@lists.sourceforge.net.  There's
actually a lot of work going on right now with both ext3 and jbd right
now, including finally getting the 64-bit support for jbd merged in.

						- Ted

Re: [RFC: 2.6 patch] fs/jbd/journal.c: possible cleanups

[Adrian Bunk]

On Tue, May 16, 2006 at 05:14:30PM -0400, Theodore Tso wrote:
> On Tue, May 16, 2006 at 09:39:56PM +0200, Adrian Bunk wrote:
> > Since you replied to this patch:
> > Who is the subsystem maintainer for jbd?
> 
> I'd suggest sending mail to ext2-devel@lists.sourceforge.net.  There's
> actually a lot of work going on right now with both ext3 and jbd right
> now, including finally getting the 64-bit support for jbd merged in.

What about getting a MAINTAINERS entry for jbd?

The only current entry listing ext2-devel@lists.sourceforge.net is for 
ext2 which doesn't use jbd...

> 						- Ted

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

[PATCH] Update ext2/ext3/jbd MAINTAINERS entries

[Theodore Tso]

On Tue, May 16, 2006 at 11:33:44PM +0200, Adrian Bunk wrote:
> What about getting a MAINTAINERS entry for jbd?
> 
> The only current entry listing ext2-devel@lists.sourceforge.net is for 
> ext2 which doesn't use jbd...

Ext2-devel is the list where all ext2/ext3/jbd development discussions
have been happening.  So it's a better list than
ext3-users@redhat.com, which is mainly for user questions/support.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

diff --git a/MAINTAINERS b/MAINTAINERS
index d6a8e5b..b24cf8b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -969,7 +969,7 @@ S:	Maintained
 EXT3 FILE SYSTEM
 P:	Stephen Tweedie, Andrew Morton
 M:	sct@redhat.com, akpm@osdl.org, adilger@clusterfs.com
-L:	ext3-users@redhat.com
+L:	ext2-devel@lists.sourceforge.net
 S:	Maintained
 
 F71805F HARDWARE MONITORING DRIVER
@@ -1529,6 +1529,12 @@ W:	http://jfs.sourceforge.net/
 T:	git kernel.org:/pub/scm/linux/kernel/git/shaggy/jfs-2.6.git
 S:	Supported
 
+JOURNALLING LAYER FOR BLOCK DEVICS (JBD)
+P:	Stephen Tweedie, Andrew Morton
+M:	sct@redhat.com, akpm@osdl.org
+L:	ext2-devel@lists.sourceforge.net
+S:	Maintained
+
 KCONFIG
 P:	Roman Zippel
 M:	zippel@linux-m68k.org

Re: [RFC: 2.6 patch] fs/jbd/journal.c: possible cleanups

[Arjan van de Ven]

On Tue, 2006-05-16 at 12:27 -0700, Andrew Morton wrote:
> Adrian Bunk <bunk@stusta.de> wrote:
> >
> > - remove the following unused EXPORT_SYMBOL's:
> >   - journal_set_features
> >   - journal_update_superblock
> 
> These might be used by lustre - dunno.
> 
> But I'm ducking all patches which alter exports, as usual.  If you can get
> them through the subsystem maintainer then good luck.
> 
> I'd suggest that we pursue the approach of getting the module loader to
> spit a warning when someone uses a going-away-soon export.
> 
> Arjan had a patch which did that, but he removed basically _every_
> presently-unused export.  

NOT IN THE SAME PATCH!
I made the infrastructure a very nicely separate patch.... and I thought
I explained that to you as well ;(

Re: [RFC: 2.6 patch] fs/jbd/journal.c: possible cleanups

[Andrew Morton]

Arjan van de Ven <arjan@infradead.org> wrote:
>
> On Tue, 2006-05-16 at 12:27 -0700, Andrew Morton wrote:
> > Adrian Bunk <bunk@stusta.de> wrote:
> > >
> > > - remove the following unused EXPORT_SYMBOL's:
> > >   - journal_set_features
> > >   - journal_update_superblock
> > 
> > These might be used by lustre - dunno.
> > 
> > But I'm ducking all patches which alter exports, as usual.  If you can get
> > them through the subsystem maintainer then good luck.
> > 
> > I'd suggest that we pursue the approach of getting the module loader to
> > spit a warning when someone uses a going-away-soon export.
> > 
> > Arjan had a patch which did that, but he removed basically _every_
> > presently-unused export.  
> 
> NOT IN THE SAME PATCH!

I noticed.

> I made the infrastructure a very nicely separate patch.... and I thought
> I explained that to you as well ;(

You did.  I was referring to the EXPORT_SYMBOL patches.

I don't think we should take the scattergun approach here.  Get the
infrastructure merged, then methodically work the EXPORT_SYMBOL patches
with the various maintainers.

Re: [RFC: 2.6 patch] fs/jbd/journal.c: possible cleanups

[Arjan van de Ven]

> I don't think we should take the scattergun approach here.  Get the
> infrastructure merged,

that should be relatively easy... would you mind doing it via your tree?

>  then methodically work the EXPORT_SYMBOL patches
> with the various maintainers.

yep but that needs step 1 to happen or they won't / can't take them...

Re: [RFC: 2.6 patch] fs/jbd/journal.c: possible cleanups

[Andrew Morton]

Arjan van de Ven <arjan@infradead.org> wrote:
>
> 
> > I don't think we should take the scattergun approach here.  Get the
> > infrastructure merged,
> 
> that should be relatively easy... would you mind doing it via your tree?

Sure.

Re: [PATCH] Update ext2/ext3/jbd MAINTAINERS entries

[Stephen C. Tweedie]

Hi,

On Tue, 2006-05-16 at 18:26 -0400, Theodore Tso wrote:

> Ext2-devel is the list where all ext2/ext3/jbd development discussions
> have been happening.  So it's a better list than
> ext3-users@redhat.com, which is mainly for user questions/support.

Right, looks fine to me.

--Stephen

Re: [RFC: 2.6 patch] fs/jbd/journal.c: possible cleanups

[Stephen C. Tweedie]

Hi,

On Tue, 2006-05-16 at 12:50 -0700, Andrew Morton wrote:

> Still, the jbd API is exported for other filesystems to use.  If these
> functions are considered part of that API (they are) then I'd suggest that
> they should be exported.

Agreed.

Note that on ext2-devel there has been a huge amount of activity over
the past month to get extents, and >32-bit block addressing, ready for
ext3.  One of the things needed for that is a new jbd-level feature for
64-bit capability, so ext3 is going to need more dynamic access to the
jbd feature bits soon.  This is definitely not the right time to be
removing feature access from the API!

--Stephen