[2.6 patch] fs/jbd/: possible cleanups

[2.6 patch] fs/jbd/: possible cleanups

[Adrian Bunk]

This patch contains the following possible cleanups:
- make needlessly global functions static
- #if 0 the following unused global functions:
  - journal.c: __journal_internal_check
  - journal.c: journal_ack_err
- remove the following write-only global variable:
  - journal.c: current_journal
- remove the following unneeded EXPORT_SYMBOL's:
  - journal.c: journal_check_used_features
  - journal.c: journal_recover
  - journal.c: journal_ack_err

Please check which of these changes do make sense.

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

---

 fs/jbd/journal.c    |   28 ++++++++++++++--------------
 fs/jbd/revoke.c     |    3 ++-
 include/linux/jbd.h |    4 ----
 3 files changed, 16 insertions(+), 19 deletions(-)

--- linux-2.6.12-rc2-mm3-full/include/linux/jbd.h.old	2005-04-21 03:39:15.000000000 +0200
+++ linux-2.6.12-rc2-mm3-full/include/linux/jbd.h	2005-04-21 03:43:21.000000000 +0200
@@ -900,8 +900,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 
@@ -914,10 +912,8 @@
 extern int	   journal_skip_recovery	(journal_t *);
 extern void	   journal_update_superblock	(journal_t *, int);
 extern void	   __journal_abort_hard	(journal_t *);
-extern void	   __journal_abort_soft	(journal_t *, int);
 extern void	   journal_abort      (journal_t *, int);
 extern int	   journal_errno      (journal_t *);
-extern void	   journal_ack_err    (journal_t *);
 extern int	   journal_clear_err  (journal_t *);
 extern int	   journal_bmap(journal_t *, unsigned long, unsigned long *);
 extern int	   journal_force_commit(journal_t *);
--- linux-2.6.12-rc2-mm3-full/fs/jbd/journal.c.old	2005-04-21 03:39:53.000000000 +0200
+++ linux-2.6.12-rc2-mm3-full/fs/jbd/journal.c	2005-04-21 03:45:07.000000000 +0200
@@ -59,17 +59,14 @@
 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_recover);
 EXPORT_SYMBOL(journal_update_superblock);
 EXPORT_SYMBOL(journal_abort);
 EXPORT_SYMBOL(journal_errno);
-EXPORT_SYMBOL(journal_ack_err);
 EXPORT_SYMBOL(journal_clear_err);
 EXPORT_SYMBOL(log_wait_commit);
 EXPORT_SYMBOL(journal_start_commit);
@@ -81,6 +78,7 @@
 EXPORT_SYMBOL(journal_force_commit);
 
 static int journal_convert_superblock_v1(journal_t *, journal_superblock_t *);
+static void __journal_abort_soft (journal_t *journal, int errno);
 
 /*
  * Helper function used to manage commit timeouts
@@ -96,12 +94,14 @@
 /* Static check for data structure consistency.  There's no code
  * invoked --- we'll just get a linker failure if things aren't right.
  */
+#if 0
 void __journal_internal_check(void)
 {
 	extern void journal_bad_superblock_size(void);
 	if (sizeof(struct journal_superblock_s) != 1024)
 		journal_bad_superblock_size();
 }
+#endif  /*  0  */
 
 /*
  * kjournald: The main thread function used to manage a logging device
@@ -119,16 +119,12 @@
  *    known as checkpointing, and this thread is responsible for that job.
  */
 
-journal_t *current_journal;		// AKPM: debug
-
-int kjournald(void *arg)
+static int kjournald(void *arg)
 {
 	journal_t *journal = (journal_t *) arg;
 	transaction_t *transaction;
 	struct timer_list timer;
 
-	current_journal = journal;
-
 	daemonize("kjournald");
 
 	/* Set up an interval timer which can be used to trigger a
@@ -1181,8 +1177,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;
 
@@ -1439,7 +1437,7 @@
  * device this journal is present.
  */
 
-const char *journal_dev_name(journal_t *journal, char *buffer)
+static const char *journal_dev_name(journal_t *journal, char *buffer)
 {
 	struct block_device *bdev;
 
@@ -1485,7 +1483,7 @@
 
 /* Soft abort: record the abort error status in the journal superblock,
  * but don't do any other IO. */
-void __journal_abort_soft (journal_t *journal, int errno)
+static void __journal_abort_soft (journal_t *journal, int errno)
 {
 	if (journal->j_flags & JFS_ABORT)
 		return;
@@ -1601,6 +1599,7 @@
  * An error must be cleared or Acked to take a FS out of readonly
  * mode.
  */
+#if 0
 void journal_ack_err(journal_t *journal)
 {
 	spin_lock(&journal->j_state_lock);
@@ -1608,6 +1607,7 @@
 		journal->j_flags |= JFS_ACK_ERR;
 	spin_unlock(&journal->j_state_lock);
 }
+#endif  /*  0  */
 
 int journal_blocks_per_page(struct inode *inode)
 {
@@ -1888,7 +1888,7 @@
 
 static struct proc_dir_entry *proc_jbd_debug;
 
-int read_jbd_debug(char *page, char **start, off_t off,
+static int read_jbd_debug(char *page, char **start, off_t off,
 			  int count, int *eof, void *data)
 {
 	int ret;
@@ -1898,7 +1898,7 @@
 	return ret;
 }
 
-int write_jbd_debug(struct file *file, const char __user *buffer,
+static int write_jbd_debug(struct file *file, const char __user *buffer,
 			   unsigned long count, void *data)
 {
 	char buf[32];
--- linux-2.6.12-rc2-mm3-full/fs/jbd/revoke.c.old	2005-04-21 03:44:18.000000000 +0200
+++ linux-2.6.12-rc2-mm3-full/fs/jbd/revoke.c	2005-04-21 03:44:31.000000000 +0200
@@ -116,7 +116,8 @@
 		(block << (hash_shift - 12))) & (table->hash_size - 1);
 }
 
-int insert_revoke_hash(journal_t *journal, unsigned long blocknr, tid_t seq)
+static int insert_revoke_hash(journal_t *journal, unsigned long blocknr,
+			      tid_t seq)
 {
 	struct list_head *hash_list;
 	struct jbd_revoke_record_s *record;

Re: [2.6 patch] fs/jbd/: possible cleanups

[Mark Fasheh]

On Sat, Apr 23, 2005 at 01:57:17AM +0200, Adrian Bunk wrote:
> This patch contains the following possible cleanups:
> - make needlessly global functions static
> - #if 0 the following unused global functions:
>   - journal.c: __journal_internal_check
>   - journal.c: journal_ack_err
> - remove the following write-only global variable:
>   - journal.c: current_journal
> - remove the following unneeded EXPORT_SYMBOL's:
>   - journal.c: journal_check_used_features
>   - journal.c: journal_recover
>   - journal.c: journal_ack_err

Well, OCFS2 uses journal_ack_err, so it'd be nice to not remove that :)
Thanks,
	--Mark

> 
> Please check which of these changes do make sense.
> 
> Signed-off-by: Adrian Bunk <bunk@stusta.de>
> 
> ---
> 
>  fs/jbd/journal.c    |   28 ++++++++++++++--------------
>  fs/jbd/revoke.c     |    3 ++-
>  include/linux/jbd.h |    4 ----
>  3 files changed, 16 insertions(+), 19 deletions(-)
> 
> --- linux-2.6.12-rc2-mm3-full/include/linux/jbd.h.old	2005-04-21 03:39:15.000000000 +0200
> +++ linux-2.6.12-rc2-mm3-full/include/linux/jbd.h	2005-04-21 03:43:21.000000000 +0200
> @@ -900,8 +900,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 
> @@ -914,10 +912,8 @@
>  extern int	   journal_skip_recovery	(journal_t *);
>  extern void	   journal_update_superblock	(journal_t *, int);
>  extern void	   __journal_abort_hard	(journal_t *);
> -extern void	   __journal_abort_soft	(journal_t *, int);
>  extern void	   journal_abort      (journal_t *, int);
>  extern int	   journal_errno      (journal_t *);
> -extern void	   journal_ack_err    (journal_t *);
>  extern int	   journal_clear_err  (journal_t *);
>  extern int	   journal_bmap(journal_t *, unsigned long, unsigned long *);
>  extern int	   journal_force_commit(journal_t *);
> --- linux-2.6.12-rc2-mm3-full/fs/jbd/journal.c.old	2005-04-21 03:39:53.000000000 +0200
> +++ linux-2.6.12-rc2-mm3-full/fs/jbd/journal.c	2005-04-21 03:45:07.000000000 +0200
> @@ -59,17 +59,14 @@
>  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_recover);
>  EXPORT_SYMBOL(journal_update_superblock);
>  EXPORT_SYMBOL(journal_abort);
>  EXPORT_SYMBOL(journal_errno);
> -EXPORT_SYMBOL(journal_ack_err);
>  EXPORT_SYMBOL(journal_clear_err);
>  EXPORT_SYMBOL(log_wait_commit);
>  EXPORT_SYMBOL(journal_start_commit);
> @@ -81,6 +78,7 @@
>  EXPORT_SYMBOL(journal_force_commit);
>  
>  static int journal_convert_superblock_v1(journal_t *, journal_superblock_t *);
> +static void __journal_abort_soft (journal_t *journal, int errno);
>  
>  /*
>   * Helper function used to manage commit timeouts
> @@ -96,12 +94,14 @@
>  /* Static check for data structure consistency.  There's no code
>   * invoked --- we'll just get a linker failure if things aren't right.
>   */
> +#if 0
>  void __journal_internal_check(void)
>  {
>  	extern void journal_bad_superblock_size(void);
>  	if (sizeof(struct journal_superblock_s) != 1024)
>  		journal_bad_superblock_size();
>  }
> +#endif  /*  0  */
>  
>  /*
>   * kjournald: The main thread function used to manage a logging device
> @@ -119,16 +119,12 @@
>   *    known as checkpointing, and this thread is responsible for that job.
>   */
>  
> -journal_t *current_journal;		// AKPM: debug
> -
> -int kjournald(void *arg)
> +static int kjournald(void *arg)
>  {
>  	journal_t *journal = (journal_t *) arg;
>  	transaction_t *transaction;
>  	struct timer_list timer;
>  
> -	current_journal = journal;
> -
>  	daemonize("kjournald");
>  
>  	/* Set up an interval timer which can be used to trigger a
> @@ -1181,8 +1177,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;
>  
> @@ -1439,7 +1437,7 @@
>   * device this journal is present.
>   */
>  
> -const char *journal_dev_name(journal_t *journal, char *buffer)
> +static const char *journal_dev_name(journal_t *journal, char *buffer)
>  {
>  	struct block_device *bdev;
>  
> @@ -1485,7 +1483,7 @@
>  
>  /* Soft abort: record the abort error status in the journal superblock,
>   * but don't do any other IO. */
> -void __journal_abort_soft (journal_t *journal, int errno)
> +static void __journal_abort_soft (journal_t *journal, int errno)
>  {
>  	if (journal->j_flags & JFS_ABORT)
>  		return;
> @@ -1601,6 +1599,7 @@
>   * An error must be cleared or Acked to take a FS out of readonly
>   * mode.
>   */
> +#if 0
>  void journal_ack_err(journal_t *journal)
>  {
>  	spin_lock(&journal->j_state_lock);
> @@ -1608,6 +1607,7 @@
>  		journal->j_flags |= JFS_ACK_ERR;
>  	spin_unlock(&journal->j_state_lock);
>  }
> +#endif  /*  0  */
>  
>  int journal_blocks_per_page(struct inode *inode)
>  {
> @@ -1888,7 +1888,7 @@
>  
>  static struct proc_dir_entry *proc_jbd_debug;
>  
> -int read_jbd_debug(char *page, char **start, off_t off,
> +static int read_jbd_debug(char *page, char **start, off_t off,
>  			  int count, int *eof, void *data)
>  {
>  	int ret;
> @@ -1898,7 +1898,7 @@
>  	return ret;
>  }
>  
> -int write_jbd_debug(struct file *file, const char __user *buffer,
> +static int write_jbd_debug(struct file *file, const char __user *buffer,
>  			   unsigned long count, void *data)
>  {
>  	char buf[32];
> --- linux-2.6.12-rc2-mm3-full/fs/jbd/revoke.c.old	2005-04-21 03:44:18.000000000 +0200
> +++ linux-2.6.12-rc2-mm3-full/fs/jbd/revoke.c	2005-04-21 03:44:31.000000000 +0200
> @@ -116,7 +116,8 @@
>  		(block << (hash_shift - 12))) & (table->hash_size - 1);
>  }
>  
> -int insert_revoke_hash(journal_t *journal, unsigned long blocknr, tid_t seq)
> +static int insert_revoke_hash(journal_t *journal, unsigned long blocknr,
> +			      tid_t seq)
>  {
>  	struct list_head *hash_list;
>  	struct jbd_revoke_record_s *record;
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
Mark Fasheh
Senior Software Developer, Oracle
mark.fasheh@oracle.com

Re: [2.6 patch] fs/jbd/: possible cleanups

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]

On Apr 23, 2005  01:57 +0200, Adrian Bunk wrote:
> This patch contains the following possible cleanups:
> - make needlessly global functions static
> - #if 0 the following unused global functions:
>   - journal.c: __journal_internal_check

>  /* Static check for data structure consistency.  There's no code
>   * invoked --- we'll just get a linker failure if things aren't right.

The comment above this function specifically says no code is generated
here - the purpose of this function is to generate an error if the
journal superblock is the wrong size (e.g. someone adds fields without
updating the padding).

> - remove the following write-only global variable:
>   - journal.c: current_journal

Looks to be debugging only, seems OK to remove.

>   - journal.c: journal_check_used_features

I'm not aware of any current users of journal_check_used_features(), but
the complementary function journal_check_available_features() IS used by
ext3 and I can imagine that if we ever need to add some more journaling
features it would be useful instead of mucking in the journal internals.

>   - journal.c: journal_recover

Looks like the correct API is actually journal_load() so it seems OK to
unexport.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [2.6 patch] fs/jbd/: possible cleanups

[Adrian Bunk]

On Mon, Apr 25, 2005 at 04:24:16PM -0600, Andreas Dilger wrote:
> On Apr 23, 2005  01:57 +0200, Adrian Bunk wrote:
> > This patch contains the following possible cleanups:
> > - make needlessly global functions static
> > - #if 0 the following unused global functions:
> >   - journal.c: __journal_internal_check
> 
> >  /* Static check for data structure consistency.  There's no code
> >   * invoked --- we'll just get a linker failure if things aren't right.
> 
> The comment above this function specifically says no code is generated
> here - the purpose of this function is to generate an error if the
> journal superblock is the wrong size (e.g. someone adds fields without
> updating the padding).

Ah, that's the part I didn't understand about it.

I still don't like this creating an empty global function.

What about moving it into one of the other functions (e.g. 
journal_init)?

>...
> >   - journal.c: journal_check_used_features
> 
> I'm not aware of any current users of journal_check_used_features(), but
> the complementary function journal_check_available_features() IS used by
> ext3 and I can imagine that if we ever need to add some more journaling
> features it would be useful instead of mucking in the journal internals.
>...

My patch changes it from a global EXPORT_SYMBOL'ed function to a static 
function.

If ext3 will ever require it undoing my change shouldn't be a problem.

> Cheers, Andreas

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: [2.6 patch] fs/jbd/: possible cleanups

[Andreas Dilger]

On Jul 13, 2005  00:43 +0200, Adrian Bunk wrote:
> On Tue, Jul 12, 2005 at 04:32:44PM -0600, Andreas Dilger wrote:
> > I don't mind removing this function, but it shouldn't be put inside #ifdef
> > JBD_DEBUG, as that would remove the check from the compiler-parsed code
> > and defeat the purpose of the check.
> 
> That's not what my patch is doing.
> 
> journal_init() is not inside an #ifdef JBD_DEBUG.

My bad.  You didn't generate diff with -p (which I normally do and is
incredibly useful when reviewing patches) and I saw "write_jbd_debug()"
above and my brain went on autopilot assuming the code had moved into
that function.  Objection withdrawn.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

Re: [2.6 patch] fs/jbd/: possible cleanups

[Adrian Bunk]

On Tue, Jul 12, 2005 at 04:32:44PM -0600, Andreas Dilger wrote:
> On Jul 12, 2005  22:27 +0200, Adrian Bunk wrote:
>...
> > - journal.c: remove the unused global function __journal_internal_check
> >              and move the check to journal_init
> 
> I don't mind removing this function, but it shouldn't be put inside #ifdef
> JBD_DEBUG, as that would remove the check from the compiler-parsed code
> and defeat the purpose of the check.

???

That's not what my patch is doing.

journal_init() is not inside an #ifdef JBD_DEBUG.

>...
> > - remove the following unneeded EXPORT_SYMBOL's:
> >   - journal.c: journal_check_used_features
> 
> Should be kept for API completeness.
>...

The function itself isn't removed.

Does it really has to stay exported or isn't it enough to re-export it 
when a user appears?

> Cheers, Andreas

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: [2.6 patch] fs/jbd/: possible cleanups

[Andreas Dilger]

On Jul 12, 2005  22:27 +0200, Adrian Bunk wrote:
> - make needlessly global functions static

I had previously commented on this patch:

> - journal.c: remove the unused global function __journal_internal_check
>              and move the check to journal_init

I don't mind removing this function, but it shouldn't be put inside #ifdef
JBD_DEBUG, as that would remove the check from the compiler-parsed code
and defeat the purpose of the check.

> - remove the following write-only global variable:
>   - journal.c: current_journal

Seems fine.

> - remove the following unneeded EXPORT_SYMBOL's:
>   - journal.c: journal_check_used_features

Should be kept for API completeness.

> - remove the following unneeded EXPORT_SYMBOL's:
>   - journal.c: journal_recover

Doesn't appear usable in any case, should be removed.

> Signed-off-by: Adrian Bunk <bunk@stusta.de>
> 
> ---
> 
> This patch was already sent on:
> - 3 Jul 2005
> - 14 Jun 2005
> 
>  fs/jbd/journal.c    |   41 ++++++++++++++++++-----------------------
>  fs/jbd/revoke.c     |    3 ++-
>  include/linux/jbd.h |    3 ---
>  3 files changed, 20 insertions(+), 27 deletions(-)
> 
> --- linux-2.6.12-rc6-mm1-full/include/linux/jbd.h.old	2005-06-14 03:58:20.000000000 +0200
> +++ linux-2.6.12-rc6-mm1-full/include/linux/jbd.h	2005-06-14 04:00:56.000000000 +0200
> @@ -900,8 +900,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 
> @@ -914,7 +912,6 @@
>  extern int	   journal_skip_recovery	(journal_t *);
>  extern void	   journal_update_superblock	(journal_t *, int);
>  extern void	   __journal_abort_hard	(journal_t *);
> -extern void	   __journal_abort_soft	(journal_t *, int);
>  extern void	   journal_abort      (journal_t *, int);
>  extern int	   journal_errno      (journal_t *);
>  extern void	   journal_ack_err    (journal_t *);
> --- linux-2.6.12-rc6-mm1-full/fs/jbd/journal.c.old	2005-06-14 03:57:39.000000000 +0200
> +++ linux-2.6.12-rc6-mm1-full/fs/jbd/journal.c	2005-06-14 04:08:24.000000000 +0200
> @@ -59,13 +59,11 @@
>  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_recover);
>  EXPORT_SYMBOL(journal_update_superblock);
>  EXPORT_SYMBOL(journal_abort);
>  EXPORT_SYMBOL(journal_errno);
> @@ -81,6 +79,7 @@
>  EXPORT_SYMBOL(journal_force_commit);
>  
>  static int journal_convert_superblock_v1(journal_t *, journal_superblock_t *);
> +static void __journal_abort_soft (journal_t *journal, int errno);
>  
>  /*
>   * Helper function used to manage commit timeouts
> @@ -93,16 +92,6 @@
>  	wake_up_process(p);
>  }
>  
> -/* Static check for data structure consistency.  There's no code
> - * invoked --- we'll just get a linker failure if things aren't right.
> - */
> -void __journal_internal_check(void)
> -{
> -	extern void journal_bad_superblock_size(void);
> -	if (sizeof(struct journal_superblock_s) != 1024)
> -		journal_bad_superblock_size();
> -}
> -
>  /*
>   * kjournald: The main thread function used to manage a logging device
>   * journal.
> @@ -119,16 +108,12 @@
>   *    known as checkpointing, and this thread is responsible for that job.
>   */
>  
> -journal_t *current_journal;		// AKPM: debug
> -
> -int kjournald(void *arg)
> +static int kjournald(void *arg)
>  {
>  	journal_t *journal = (journal_t *) arg;
>  	transaction_t *transaction;
>  	struct timer_list timer;
>  
> -	current_journal = journal;
> -
>  	daemonize("kjournald");
>  
>  	/* Set up an interval timer which can be used to trigger a
> @@ -1181,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;
>  
> @@ -1439,7 +1426,7 @@
>   * device this journal is present.
>   */
>  
> -const char *journal_dev_name(journal_t *journal, char *buffer)
> +static const char *journal_dev_name(journal_t *journal, char *buffer)
>  {
>  	struct block_device *bdev;
>  
> @@ -1485,7 +1472,7 @@
>  
>  /* Soft abort: record the abort error status in the journal superblock,
>   * but don't do any other IO. */
> -void __journal_abort_soft (journal_t *journal, int errno)
> +static void __journal_abort_soft (journal_t *journal, int errno)
>  {
>  	if (journal->j_flags & JFS_ABORT)
>  		return;
> @@ -1888,7 +1875,7 @@
>  
>  static struct proc_dir_entry *proc_jbd_debug;
>  
> -int read_jbd_debug(char *page, char **start, off_t off,
> +static int read_jbd_debug(char *page, char **start, off_t off,
>  			  int count, int *eof, void *data)
>  {
>  	int ret;
> @@ -1898,7 +1885,7 @@
>  	return ret;
>  }
>  
> -int write_jbd_debug(struct file *file, const char __user *buffer,
> +static int write_jbd_debug(struct file *file, const char __user *buffer,
>  			   unsigned long count, void *data)
>  {
>  	char buf[32];
> @@ -1987,6 +1974,14 @@
>  {
>  	int ret;
>  
> +/* Static check for data structure consistency.  There's no code
> + * invoked --- we'll just get a linker failure if things aren't right.
> + */
> +	extern void journal_bad_superblock_size(void);
> +	if (sizeof(struct journal_superblock_s) != 1024)
> +		journal_bad_superblock_size();
> +
> +
>  	ret = journal_init_caches();
>  	if (ret != 0)
>  		journal_destroy_caches();
> --- linux-2.6.12-rc6-mm1-full/fs/jbd/revoke.c.old	2005-06-14 03:58:36.000000000 +0200
> +++ linux-2.6.12-rc6-mm1-full/fs/jbd/revoke.c	2005-06-14 03:58:41.000000000 +0200
> @@ -116,7 +116,8 @@
>  		(block << (hash_shift - 12))) & (table->hash_size - 1);
>  }
>  
> -int insert_revoke_hash(journal_t *journal, unsigned long blocknr, tid_t seq)
> +static int insert_revoke_hash(journal_t *journal, unsigned long blocknr,
> +			      tid_t seq)
>  {
>  	struct list_head *hash_list;
>  	struct jbd_revoke_record_s *record;

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

[2.6 patch] fs/jbd/: possible cleanups

[Adrian Bunk]

This patch contains the following possible cleanups:
- make needlessly global functions static
- journal.c: remove the unused global function __journal_internal_check
             and move the check to journal_init
- remove the following write-only global variable:
  - journal.c: current_journal
- remove the following unneeded EXPORT_SYMBOL's:
  - journal.c: journal_check_used_features
  - journal.c: journal_recover

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

---

This patch was already sent on:
- 3 Jul 2005
- 14 Jun 2005

 fs/jbd/journal.c    |   41 ++++++++++++++++++-----------------------
 fs/jbd/revoke.c     |    3 ++-
 include/linux/jbd.h |    3 ---
 3 files changed, 20 insertions(+), 27 deletions(-)

--- linux-2.6.12-rc6-mm1-full/include/linux/jbd.h.old	2005-06-14 03:58:20.000000000 +0200
+++ linux-2.6.12-rc6-mm1-full/include/linux/jbd.h	2005-06-14 04:00:56.000000000 +0200
@@ -900,8 +900,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 
@@ -914,7 +912,6 @@
 extern int	   journal_skip_recovery	(journal_t *);
 extern void	   journal_update_superblock	(journal_t *, int);
 extern void	   __journal_abort_hard	(journal_t *);
-extern void	   __journal_abort_soft	(journal_t *, int);
 extern void	   journal_abort      (journal_t *, int);
 extern int	   journal_errno      (journal_t *);
 extern void	   journal_ack_err    (journal_t *);
--- linux-2.6.12-rc6-mm1-full/fs/jbd/journal.c.old	2005-06-14 03:57:39.000000000 +0200
+++ linux-2.6.12-rc6-mm1-full/fs/jbd/journal.c	2005-06-14 04:08:24.000000000 +0200
@@ -59,13 +59,11 @@
 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_recover);
 EXPORT_SYMBOL(journal_update_superblock);
 EXPORT_SYMBOL(journal_abort);
 EXPORT_SYMBOL(journal_errno);
@@ -81,6 +79,7 @@
 EXPORT_SYMBOL(journal_force_commit);
 
 static int journal_convert_superblock_v1(journal_t *, journal_superblock_t *);
+static void __journal_abort_soft (journal_t *journal, int errno);
 
 /*
  * Helper function used to manage commit timeouts
@@ -93,16 +92,6 @@
 	wake_up_process(p);
 }
 
-/* Static check for data structure consistency.  There's no code
- * invoked --- we'll just get a linker failure if things aren't right.
- */
-void __journal_internal_check(void)
-{
-	extern void journal_bad_superblock_size(void);
-	if (sizeof(struct journal_superblock_s) != 1024)
-		journal_bad_superblock_size();
-}
-
 /*
  * kjournald: The main thread function used to manage a logging device
  * journal.
@@ -119,16 +108,12 @@
  *    known as checkpointing, and this thread is responsible for that job.
  */
 
-journal_t *current_journal;		// AKPM: debug
-
-int kjournald(void *arg)
+static int kjournald(void *arg)
 {
 	journal_t *journal = (journal_t *) arg;
 	transaction_t *transaction;
 	struct timer_list timer;
 
-	current_journal = journal;
-
 	daemonize("kjournald");
 
 	/* Set up an interval timer which can be used to trigger a
@@ -1181,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;
 
@@ -1439,7 +1426,7 @@
  * device this journal is present.
  */
 
-const char *journal_dev_name(journal_t *journal, char *buffer)
+static const char *journal_dev_name(journal_t *journal, char *buffer)
 {
 	struct block_device *bdev;
 
@@ -1485,7 +1472,7 @@
 
 /* Soft abort: record the abort error status in the journal superblock,
  * but don't do any other IO. */
-void __journal_abort_soft (journal_t *journal, int errno)
+static void __journal_abort_soft (journal_t *journal, int errno)
 {
 	if (journal->j_flags & JFS_ABORT)
 		return;
@@ -1888,7 +1875,7 @@
 
 static struct proc_dir_entry *proc_jbd_debug;
 
-int read_jbd_debug(char *page, char **start, off_t off,
+static int read_jbd_debug(char *page, char **start, off_t off,
 			  int count, int *eof, void *data)
 {
 	int ret;
@@ -1898,7 +1885,7 @@
 	return ret;
 }
 
-int write_jbd_debug(struct file *file, const char __user *buffer,
+static int write_jbd_debug(struct file *file, const char __user *buffer,
 			   unsigned long count, void *data)
 {
 	char buf[32];
@@ -1987,6 +1974,14 @@
 {
 	int ret;
 
+/* Static check for data structure consistency.  There's no code
+ * invoked --- we'll just get a linker failure if things aren't right.
+ */
+	extern void journal_bad_superblock_size(void);
+	if (sizeof(struct journal_superblock_s) != 1024)
+		journal_bad_superblock_size();
+
+
 	ret = journal_init_caches();
 	if (ret != 0)
 		journal_destroy_caches();
--- linux-2.6.12-rc6-mm1-full/fs/jbd/revoke.c.old	2005-06-14 03:58:36.000000000 +0200
+++ linux-2.6.12-rc6-mm1-full/fs/jbd/revoke.c	2005-06-14 03:58:41.000000000 +0200
@@ -116,7 +116,8 @@
 		(block << (hash_shift - 12))) & (table->hash_size - 1);
 }
 
-int insert_revoke_hash(journal_t *journal, unsigned long blocknr, tid_t seq)
+static int insert_revoke_hash(journal_t *journal, unsigned long blocknr,
+			      tid_t seq)
 {
 	struct list_head *hash_list;
 	struct jbd_revoke_record_s *record;

[2.6 patch] fs/jbd/: possible cleanups

[Adrian Bunk]

This patch contains the following possible cleanups:
- make needlessly global functions static
- journal.c: remove the unused global function __journal_internal_check
             and move the check to journal_init
- remove the following write-only global variable:
  - journal.c: current_journal
- remove the following unneeded EXPORT_SYMBOL's:
  - journal.c: journal_check_used_features
  - journal.c: journal_recover

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

---

This patch was already sent on:
- 14 Jun 2005

 fs/jbd/journal.c    |   41 ++++++++++++++++++-----------------------
 fs/jbd/revoke.c     |    3 ++-
 include/linux/jbd.h |    3 ---
 3 files changed, 20 insertions(+), 27 deletions(-)

--- linux-2.6.12-rc6-mm1-full/include/linux/jbd.h.old	2005-06-14 03:58:20.000000000 +0200
+++ linux-2.6.12-rc6-mm1-full/include/linux/jbd.h	2005-06-14 04:00:56.000000000 +0200
@@ -900,8 +900,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 
@@ -914,7 +912,6 @@
 extern int	   journal_skip_recovery	(journal_t *);
 extern void	   journal_update_superblock	(journal_t *, int);
 extern void	   __journal_abort_hard	(journal_t *);
-extern void	   __journal_abort_soft	(journal_t *, int);
 extern void	   journal_abort      (journal_t *, int);
 extern int	   journal_errno      (journal_t *);
 extern void	   journal_ack_err    (journal_t *);
--- linux-2.6.12-rc6-mm1-full/fs/jbd/journal.c.old	2005-06-14 03:57:39.000000000 +0200
+++ linux-2.6.12-rc6-mm1-full/fs/jbd/journal.c	2005-06-14 04:08:24.000000000 +0200
@@ -59,13 +59,11 @@
 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_recover);
 EXPORT_SYMBOL(journal_update_superblock);
 EXPORT_SYMBOL(journal_abort);
 EXPORT_SYMBOL(journal_errno);
@@ -81,6 +79,7 @@
 EXPORT_SYMBOL(journal_force_commit);
 
 static int journal_convert_superblock_v1(journal_t *, journal_superblock_t *);
+static void __journal_abort_soft (journal_t *journal, int errno);
 
 /*
  * Helper function used to manage commit timeouts
@@ -93,16 +92,6 @@
 	wake_up_process(p);
 }
 
-/* Static check for data structure consistency.  There's no code
- * invoked --- we'll just get a linker failure if things aren't right.
- */
-void __journal_internal_check(void)
-{
-	extern void journal_bad_superblock_size(void);
-	if (sizeof(struct journal_superblock_s) != 1024)
-		journal_bad_superblock_size();
-}
-
 /*
  * kjournald: The main thread function used to manage a logging device
  * journal.
@@ -119,16 +108,12 @@
  *    known as checkpointing, and this thread is responsible for that job.
  */
 
-journal_t *current_journal;		// AKPM: debug
-
-int kjournald(void *arg)
+static int kjournald(void *arg)
 {
 	journal_t *journal = (journal_t *) arg;
 	transaction_t *transaction;
 	struct timer_list timer;
 
-	current_journal = journal;
-
 	daemonize("kjournald");
 
 	/* Set up an interval timer which can be used to trigger a
@@ -1181,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;
 
@@ -1439,7 +1426,7 @@
  * device this journal is present.
  */
 
-const char *journal_dev_name(journal_t *journal, char *buffer)
+static const char *journal_dev_name(journal_t *journal, char *buffer)
 {
 	struct block_device *bdev;
 
@@ -1485,7 +1472,7 @@
 
 /* Soft abort: record the abort error status in the journal superblock,
  * but don't do any other IO. */
-void __journal_abort_soft (journal_t *journal, int errno)
+static void __journal_abort_soft (journal_t *journal, int errno)
 {
 	if (journal->j_flags & JFS_ABORT)
 		return;
@@ -1888,7 +1875,7 @@
 
 static struct proc_dir_entry *proc_jbd_debug;
 
-int read_jbd_debug(char *page, char **start, off_t off,
+static int read_jbd_debug(char *page, char **start, off_t off,
 			  int count, int *eof, void *data)
 {
 	int ret;
@@ -1898,7 +1885,7 @@
 	return ret;
 }
 
-int write_jbd_debug(struct file *file, const char __user *buffer,
+static int write_jbd_debug(struct file *file, const char __user *buffer,
 			   unsigned long count, void *data)
 {
 	char buf[32];
@@ -1987,6 +1974,14 @@
 {
 	int ret;
 
+/* Static check for data structure consistency.  There's no code
+ * invoked --- we'll just get a linker failure if things aren't right.
+ */
+	extern void journal_bad_superblock_size(void);
+	if (sizeof(struct journal_superblock_s) != 1024)
+		journal_bad_superblock_size();
+
+
 	ret = journal_init_caches();
 	if (ret != 0)
 		journal_destroy_caches();
--- linux-2.6.12-rc6-mm1-full/fs/jbd/revoke.c.old	2005-06-14 03:58:36.000000000 +0200
+++ linux-2.6.12-rc6-mm1-full/fs/jbd/revoke.c	2005-06-14 03:58:41.000000000 +0200
@@ -116,7 +116,8 @@
 		(block << (hash_shift - 12))) & (table->hash_size - 1);
 }
 
-int insert_revoke_hash(journal_t *journal, unsigned long blocknr, tid_t seq)
+static int insert_revoke_hash(journal_t *journal, unsigned long blocknr,
+			      tid_t seq)
 {
 	struct list_head *hash_list;
 	struct jbd_revoke_record_s *record;

[2.6 patch] fs/jbd/: possible cleanups

[Adrian Bunk]

This patch contains the following possible cleanups:
- make needlessly global functions static
- journal.c: remove the unused global function __journal_internal_check
             and move the check to journal_init
- remove the following write-only global variable:
  - journal.c: current_journal
- remove the following unneeded EXPORT_SYMBOL's:
  - journal.c: journal_check_used_features
  - journal.c: journal_recover

Please check which of these changes do make sense.

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

---

 fs/jbd/journal.c    |   41 ++++++++++++++++++-----------------------
 fs/jbd/revoke.c     |    3 ++-
 include/linux/jbd.h |    3 ---
 3 files changed, 20 insertions(+), 27 deletions(-)

--- linux-2.6.12-rc6-mm1-full/include/linux/jbd.h.old	2005-06-14 03:58:20.000000000 +0200
+++ linux-2.6.12-rc6-mm1-full/include/linux/jbd.h	2005-06-14 04:00:56.000000000 +0200
@@ -900,8 +900,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 
@@ -914,7 +912,6 @@
 extern int	   journal_skip_recovery	(journal_t *);
 extern void	   journal_update_superblock	(journal_t *, int);
 extern void	   __journal_abort_hard	(journal_t *);
-extern void	   __journal_abort_soft	(journal_t *, int);
 extern void	   journal_abort      (journal_t *, int);
 extern int	   journal_errno      (journal_t *);
 extern void	   journal_ack_err    (journal_t *);
--- linux-2.6.12-rc6-mm1-full/fs/jbd/journal.c.old	2005-06-14 03:57:39.000000000 +0200
+++ linux-2.6.12-rc6-mm1-full/fs/jbd/journal.c	2005-06-14 04:08:24.000000000 +0200
@@ -59,13 +59,11 @@
 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_recover);
 EXPORT_SYMBOL(journal_update_superblock);
 EXPORT_SYMBOL(journal_abort);
 EXPORT_SYMBOL(journal_errno);
@@ -81,6 +79,7 @@
 EXPORT_SYMBOL(journal_force_commit);
 
 static int journal_convert_superblock_v1(journal_t *, journal_superblock_t *);
+static void __journal_abort_soft (journal_t *journal, int errno);
 
 /*
  * Helper function used to manage commit timeouts
@@ -93,16 +92,6 @@
 	wake_up_process(p);
 }
 
-/* Static check for data structure consistency.  There's no code
- * invoked --- we'll just get a linker failure if things aren't right.
- */
-void __journal_internal_check(void)
-{
-	extern void journal_bad_superblock_size(void);
-	if (sizeof(struct journal_superblock_s) != 1024)
-		journal_bad_superblock_size();
-}
-
 /*
  * kjournald: The main thread function used to manage a logging device
  * journal.
@@ -119,16 +108,12 @@
  *    known as checkpointing, and this thread is responsible for that job.
  */
 
-journal_t *current_journal;		// AKPM: debug
-
-int kjournald(void *arg)
+static int kjournald(void *arg)
 {
 	journal_t *journal = (journal_t *) arg;
 	transaction_t *transaction;
 	struct timer_list timer;
 
-	current_journal = journal;
-
 	daemonize("kjournald");
 
 	/* Set up an interval timer which can be used to trigger a
@@ -1181,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;
 
@@ -1439,7 +1426,7 @@
  * device this journal is present.
  */
 
-const char *journal_dev_name(journal_t *journal, char *buffer)
+static const char *journal_dev_name(journal_t *journal, char *buffer)
 {
 	struct block_device *bdev;
 
@@ -1485,7 +1472,7 @@
 
 /* Soft abort: record the abort error status in the journal superblock,
  * but don't do any other IO. */
-void __journal_abort_soft (journal_t *journal, int errno)
+static void __journal_abort_soft (journal_t *journal, int errno)
 {
 	if (journal->j_flags & JFS_ABORT)
 		return;
@@ -1888,7 +1875,7 @@
 
 static struct proc_dir_entry *proc_jbd_debug;
 
-int read_jbd_debug(char *page, char **start, off_t off,
+static int read_jbd_debug(char *page, char **start, off_t off,
 			  int count, int *eof, void *data)
 {
 	int ret;
@@ -1898,7 +1885,7 @@
 	return ret;
 }
 
-int write_jbd_debug(struct file *file, const char __user *buffer,
+static int write_jbd_debug(struct file *file, const char __user *buffer,
 			   unsigned long count, void *data)
 {
 	char buf[32];
@@ -1987,6 +1974,14 @@
 {
 	int ret;
 
+/* Static check for data structure consistency.  There's no code
+ * invoked --- we'll just get a linker failure if things aren't right.
+ */
+	extern void journal_bad_superblock_size(void);
+	if (sizeof(struct journal_superblock_s) != 1024)
+		journal_bad_superblock_size();
+
+
 	ret = journal_init_caches();
 	if (ret != 0)
 		journal_destroy_caches();
--- linux-2.6.12-rc6-mm1-full/fs/jbd/revoke.c.old	2005-06-14 03:58:36.000000000 +0200
+++ linux-2.6.12-rc6-mm1-full/fs/jbd/revoke.c	2005-06-14 03:58:41.000000000 +0200
@@ -116,7 +116,8 @@
 		(block << (hash_shift - 12))) & (table->hash_size - 1);
 }
 
-int insert_revoke_hash(journal_t *journal, unsigned long blocknr, tid_t seq)
+static int insert_revoke_hash(journal_t *journal, unsigned long blocknr,
+			      tid_t seq)
 {
 	struct list_head *hash_list;
 	struct jbd_revoke_record_s *record;