2006-05-16 17:46:06

by Adrian Bunk

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

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 <[email protected]>

---

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;



2006-05-16 19:28:04

by Andrew Morton

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

Adrian Bunk <[email protected]> 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.

2006-05-16 19:39:59

by Adrian Bunk

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

On Tue, May 16, 2006 at 12:27:31PM -0700, Andrew Morton wrote:
> Adrian Bunk <[email protected]> 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

2006-05-16 19:51:25

by Andrew Morton

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

Adrian Bunk <[email protected]> wrote:
>
> On Tue, May 16, 2006 at 12:27:31PM -0700, Andrew Morton wrote:
> > Adrian Bunk <[email protected]> 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..

2006-05-16 21:14:46

by Theodore Ts'o

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

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 [email protected]. 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

2006-05-16 21:33:46

by Adrian Bunk

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

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 [email protected]. 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 [email protected] 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

2006-05-16 22:26:23

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH] Update ext2/ext3/jbd MAINTAINERS entries

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 [email protected] 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
[email protected], which is mainly for user questions/support.

Signed-off-by: "Theodore Ts'o" <[email protected]>

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: [email protected], [email protected], [email protected]
-L: [email protected]
+L: [email protected]
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: [email protected], [email protected]
+L: [email protected]
+S: Maintained
+
KCONFIG
P: Roman Zippel
M: [email protected]

2006-05-17 01:33:54

by Arjan van de Ven

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

On Tue, 2006-05-16 at 12:27 -0700, Andrew Morton wrote:
> Adrian Bunk <[email protected]> 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 ;(


2006-05-17 01:47:22

by Andrew Morton

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

Arjan van de Ven <[email protected]> wrote:
>
> On Tue, 2006-05-16 at 12:27 -0700, Andrew Morton wrote:
> > Adrian Bunk <[email protected]> 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.

2006-05-17 01:52:12

by Arjan van de Ven

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


> 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...



2006-05-17 02:04:57

by Andrew Morton

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

Arjan van de Ven <[email protected]> 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.

2006-05-18 11:50:16

by Stephen C. Tweedie

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

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
> [email protected], which is mainly for user questions/support.

Right, looks fine to me.

--Stephen


2006-05-18 11:53:25

by Stephen C. Tweedie

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

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