Skip to contentSkip to conversation

uselex.rb as a tiny tool to find dead code

PATCH In review Sergei Trofimovich 5 participants 18 messages Aug 7, 2013 – Aug 8, 2013 Btrfs file system

lore
Share & tools
lore

Sergei Trofimovich

author
#0 lore raw

cover uselex.rb as a tiny tool to find dead code

From: Sergei Trofimovich <[email protected]>

Hi guys!

TL;DR:

the patches remove (currently or already) dead code and localize symbol visibility used only in one module.

Thanks!

The Story:

Once upon a time I've got very simple idea to find dead code in large C/C++ mixed project at work.

The things of my primary interest were things of too large visibility scope, like:
- externally visible global variables used only in module defining the variable
- and functions of the same property

Once you identify those functions and mark as 'static' you achieve two goals:

- minor: give compiler possibility to inline code a bit better (especially for one-caller case)
- more interesting: possibility to spot that the symbol is no longer used at all

Hence the project idea:
1. scan all intermediate object (.o) files for exported symbols
2. find such symbols that are not imported into any .o files

Meet uselex.rb: one-file script to parse 'nm' output:

https://github.com/trofi/uselex/blob/master/uselex.rb

For linux's kernel it's use is as simple as:

    $ make $config && make
    $ uselex.rb `find -name '*.o'`

and you've got material to look at!

It outputs 3000 symbols thus I've started small:

    $ uselex.rb `find fs/btrfs -name '*.o'

To get an impression it reports such things:

set_state_private: [R]: exported from: fs/btrfs/built-in.o fs/btrfs/btrfs.o fs/btrfs/extent_io.o
btrfs_get_inode_ref_index: [R]: exported from: fs/btrfs/built-in.o fs/btrfs/inode-item.o fs/btrfs/btrfs.o
btrfs_print_tree: [R]: exported from: fs/btrfs/built-in.o fs/btrfs/print-tree.o fs/btrfs/btrfs.o
btrfs_reada_detach: [R]: exported from: fs/btrfs/built-in.o fs/btrfs/reada.o fs/btrfs/btrfs.o __btrfs_getxattr: [R]: exported from: fs/btrfs/built-in.o fs/btrfs/xattr.o fs/btrfs/btrfs.o __btrfs_setxattr: [R]: exported from: fs/btrfs/built-in.o fs/btrfs/xattr.o fs/btrfs/btrfs.o
btrfs_read_root_item: [R]: exported from: fs/btrfs/root-tree.o fs/btrfs/built-in.o fs/btrfs/btrfs.o __tracepoint_btrfs_sync_fs: [R]: exported from: fs/btrfs/super.o fs/btrfs/built-in.o fs/btrfs/btrfs.o btrfs_start_transaction_lflush: [R]: exported from: fs/btrfs/transaction.o fs/btrfs/built-in.o fs/btrfs/btrfs.o btrfs_write_and_wait_marked_extents: [R]: exported from: fs/btrfs/transaction.o fs/btrfs/built-in.o fs/btrfs/btrfs.o
ulist_fini: [R]: exported from: fs/btrfs/ulist.o fs/btrfs/built-in.o fs/btrfs/btrfs.o
ulist_init: [R]: exported from: fs/btrfs/ulist.o fs/btrfs/built-in.o fs/btrfs/btrfs.o

You just need to make sure it does not lie too much.

For example '__btrfs_getxattr' needs to be brought under '#ifdef CONFIG_BTRFS_FS_POSIX_ACL' and not removed entirely.

Thanks for your patience!

author
#1 lore raw

PATCH 1/7 btrfs: cleanup: mark 'btrfs_lookup_fs_root' as static

From: Sergei Trofimovich <[email protected]>

Found by uselex.rb:

Quoted 1 line

btrfs_lookup_fs_root: [R]: exported from: fs/btrfs/disk-io.o fs/btrfs/built-in.o fs/btrfs/btrfs.o

Signed-off-by Sergei Trofimovich <[email protected]>
---
 fs/btrfs/disk-io.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
fs/btrfs/disk-io.c +2 -2
index 8816847..2338209 100644
@@ -1529,8 +1529,8 @@ fail:
 	return ret;
 }
 
-struct btrfs_root *btrfs_lookup_fs_root(struct btrfs_fs_info *fs_info,
-					u64 root_id)
+static struct btrfs_root *btrfs_lookup_fs_root(struct btrfs_fs_info *fs_info,
+					       u64 root_id)
 {
 	struct btrfs_root *root;
Signature · 1 line
1.8.3.2
author
#2 lore raw

PATCH 2/7 btrfs: cleanup: mark 'set_state_private' as static

In reply to Sergei Trofimovich

From: Sergei Trofimovich <[email protected]>

Found by uselex.rb:

Quoted 1 line

set_state_private: [R]: exported from: fs/btrfs/disk-io.o fs/btrfs/built-in.o fs/btrfs/btrfs.o

Signed-off-by Sergei Trofimovich <[email protected]>
---
 fs/btrfs/extent_io.c | 2 +-
 fs/btrfs/extent_io.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)
fs/btrfs/extent_io.c +1 -1
index 583d98b..3912396 100644
@@ -1810,7 +1810,7 @@ out:
  * set the private field for a given byte offset in the tree.  If there isn't
  * an extent_state there already, this does nothing.
  */
-int set_state_private(struct extent_io_tree *tree, u64 start, u64 private)
+static int set_state_private(struct extent_io_tree *tree, u64 start, u64 private)
 {
 	struct rb_node *node;
 	struct extent_state *state;
fs/btrfs/extent_io.h +0 -1
index 3b8c4e2..bb1f75d 100644
@@ -261,7 +261,6 @@ int extent_readpages(struct extent_io_tree *tree,
 		     get_extent_t get_extent);
 int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 		__u64 start, __u64 len, get_extent_t *get_extent);
-int set_state_private(struct extent_io_tree *tree, u64 start, u64 private);
 void extent_cache_csums_dio(struct extent_io_tree *tree, u64 start, u32 csums[],
 			    int count);
 void extent_cache_csums(struct extent_io_tree *tree, struct bio *bio,
Signature · 1 line
1.8.3.2
author
#3 lore raw

PATCH 3/7 btrfs: cleanup: mark 'btrfs_write_and_wait_marked_extents' as static

In reply to Sergei Trofimovich

From: Sergei Trofimovich <[email protected]>

Found by uselex.rb:

Quoted 1 line

btrfs_write_and_wait_marked_extents: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/transaction.o fs/btrfs/built-in.o

Signed-off-by Sergei Trofimovich <[email protected]>
---
 fs/btrfs/transaction.c | 4 ++--
 fs/btrfs/transaction.h | 2 --
 2 files changed, 2 insertions(+), 4 deletions(-)
fs/btrfs/transaction.c +2 -2
index d58cce7..ff891d2 100644
@@ -837,8 +837,8 @@ int btrfs_wait_marked_extents(struct btrfs_root *root,
  * them in one of two extent_io trees.  This is used to make sure all of
  * those extents are on disk for transaction or log commit
  */
-int btrfs_write_and_wait_marked_extents(struct btrfs_root *root,
-				struct extent_io_tree *dirty_pages, int mark)
+static int btrfs_write_and_wait_marked_extents(struct btrfs_root *root,
+					       struct extent_io_tree *dirty_pages, int mark)
 {
 	int ret;
 	int ret2;
fs/btrfs/transaction.h +0 -2
index 005b037..af8293f 100644
@@ -160,8 +160,6 @@ int btrfs_should_end_transaction(struct btrfs_trans_handle *trans,
 void btrfs_throttle(struct btrfs_root *root);
 int btrfs_record_root_in_trans(struct btrfs_trans_handle *trans,
 				struct btrfs_root *root);
-int btrfs_write_and_wait_marked_extents(struct btrfs_root *root,
-				struct extent_io_tree *dirty_pages, int mark);
 int btrfs_write_marked_extents(struct btrfs_root *root,
 				struct extent_io_tree *dirty_pages, int mark);
 int btrfs_wait_marked_extents(struct btrfs_root *root,
Signature · 1 line
1.8.3.2
#4 lore raw

Re: [PATCH 3/7] btrfs: cleanup: mark 'btrfs_write_and_wait_marked_extents' as static

Quoted 16 lines · Sergei Trofimovich

From: Sergei Trofimovich <[email protected]>

Found by uselex.rb:

Quoted 1 line

btrfs_write_and_wait_marked_extents: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/transaction.o fs/btrfs/built-in.o

Signed-off-by Sergei Trofimovich <[email protected]>
---
 fs/btrfs/transaction.c | 4 ++--
 fs/btrfs/transaction.h | 2 --
 2 files changed, 2 insertions(+), 4 deletions(-)
fs/btrfs/transaction.c +0 -0
index d58cce7..ff891d2 100644

fs/btrfs/transaction.c int btrfs_wait_marked_extents(struct btrfs_root *root, · line 841

Quoted 7 lines
fs/btrfs/transaction.c +2 -2
@@ -837,8 +837,8 @@ int btrfs_wait_marked_extents(struct btrfs_root *root,
  * them in one of two extent_io trees.  This is used to make sure all of
  * those extents are on disk for transaction or log commit
  */
-int btrfs_write_and_wait_marked_extents(struct btrfs_root *root,
-				struct extent_io_tree *dirty_pages, int mark)
+static int btrfs_write_and_wait_marked_extents(struct btrfs_root *root,
+					       struct extent_io_tree *dirty_pages, int mark)

You may want to run the output through checkpatch.pl and fix obvious style violations (line too long).

david

author
#5 lore raw

PATCH 4/7 btrfs: cleanup: removed unused 'btrfs_start_transaction_lflush'

In reply to Sergei Trofimovich

From: Sergei Trofimovich <[email protected]>

Found by uselex.rb:

Quoted 1 line

btrfs_start_transaction_lflush: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/transaction.o fs/btrfs/built-in.o

Signed-off-by Sergei Trofimovich <[email protected]>
---
 fs/btrfs/transaction.c | 7 -------
 fs/btrfs/transaction.h | 2 --
 2 files changed, 9 deletions(-)
fs/btrfs/transaction.c +0 -7
index ff891d2..57c3239 100644
@@ -497,13 +497,6 @@ struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root,
 				 BTRFS_RESERVE_FLUSH_ALL);
 }
 
-struct btrfs_trans_handle *btrfs_start_transaction_lflush(
-					struct btrfs_root *root, int num_items)
-{
-	return start_transaction(root, num_items, TRANS_START,
-				 BTRFS_RESERVE_FLUSH_LIMIT);
-}
-
 struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root)
 {
 	return start_transaction(root, 0, TRANS_JOIN, 0);
fs/btrfs/transaction.h +0 -2
index af8293f..05d4be1 100644
@@ -131,8 +131,6 @@ int btrfs_end_transaction(struct btrfs_trans_handle *trans,
 			  struct btrfs_root *root);
 struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root,
 						   int num_items);
-struct btrfs_trans_handle *btrfs_start_transaction_lflush(
-					struct btrfs_root *root, int num_items);
 struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root);
 struct btrfs_trans_handle *btrfs_join_transaction_nolock(struct btrfs_root *root);
 struct btrfs_trans_handle *btrfs_attach_transaction(struct btrfs_root *root);
Signature · 1 line
1.8.3.2
#6 lore raw

Re: [PATCH 4/7] btrfs: cleanup: removed unused 'btrfs_start_transaction_lflush'

Quoted 4 lines · Sergei Trofimovich

From: Sergei Trofimovich <[email protected]>

Found by uselex.rb:

Quoted 1 line

btrfs_start_transaction_lflush: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/transaction.o fs/btrfs/built-in.o

http://www.mail-archive.com/[email protected]/msg24047.html

"> btrfs_start_transaction_lflush()

Transcaction API, removing the func does not make sense without removing BTRFS_RESERVE_FLUSH_LIMIT at the same time.
"

Miao introduced this function in 08e007d2e57744472a9424735a to enhance flushing logic to avoid deadlocks.

david

author
#7 lore raw

PATCH 5/7 btrfs: cleanup: mark 'btrfs_read_root_item' as static

In reply to Sergei Trofimovich

From: Sergei Trofimovich <[email protected]>

Found by uselex.rb:

Quoted 1 line

btrfs_read_root_item: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/built-in.o fs/btrfs/root-tree.o

Signed-off-by Sergei Trofimovich <[email protected]>
---
 fs/btrfs/ctree.h     | 2 --
 fs/btrfs/root-tree.c | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)
fs/btrfs/ctree.h +0 -2
index e795bf1..e91ab9e 100644
@@ -3414,8 +3414,6 @@ int __must_check btrfs_update_root(struct btrfs_trans_handle *trans,
 				   struct btrfs_root *root,
 				   struct btrfs_key *key,
 				   struct btrfs_root_item *item);
-void btrfs_read_root_item(struct extent_buffer *eb, int slot,
-			  struct btrfs_root_item *item);
 int btrfs_find_root(struct btrfs_root *root, struct btrfs_key *search_key,
 		    struct btrfs_path *path, struct btrfs_root_item *root_item,
 		    struct btrfs_key *root_key);
fs/btrfs/root-tree.c +2 -2
index ffb1036..65ecc15 100644
@@ -29,8 +29,8 @@
  * generation numbers as then we know the root was once mounted with an older
  * kernel that was not aware of the root item structure change.
  */
-void btrfs_read_root_item(struct extent_buffer *eb, int slot,
-			  struct btrfs_root_item *item)
+static void btrfs_read_root_item(struct extent_buffer *eb, int slot,
+				 struct btrfs_root_item *item)
 {
 	uuid_le uuid;
 	int len;
Signature · 1 line
1.8.3.2
author
#8 lore raw

PATCH 6/7 btrfs: cleanup: removed unused 'btrfs_reada_detach'

In reply to Sergei Trofimovich

From: Sergei Trofimovich <[email protected]>

Found by uselex.rb:

Quoted 1 line

btrfs_reada_detach: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/built-in.o fs/btrfs/reada.o

Signed-off-by Sergei Trofimovich <[email protected]>
---
 fs/btrfs/ctree.h | 1 -
 fs/btrfs/reada.c | 9 +--------
 2 files changed, 1 insertion(+), 9 deletions(-)
fs/btrfs/ctree.h +0 -1
index e91ab9e..f35e086 100644
@@ -3861,7 +3861,6 @@ struct reada_control {
 struct reada_control *btrfs_reada_add(struct btrfs_root *root,
 			      struct btrfs_key *start, struct btrfs_key *end);
 int btrfs_reada_wait(void *handle);
-void btrfs_reada_detach(void *handle);
 int btree_readahead_hook(struct btrfs_root *root, struct extent_buffer *eb,
 			 u64 start, int err);
fs/btrfs/reada.c +1 -8
index 1031b69..c41d470 100644
@@ -37,7 +37,7 @@
  * To trigger a readahead, btrfs_reada_add must be called. It will start
  * a read ahead for the given range [start, end) on tree root. The returned
  * handle can either be used to wait on the readahead to finish
- * (btrfs_reada_wait), or to send it to the background (btrfs_reada_detach).
+ * (btrfs_reada_wait).
  *
  * The read ahead works as follows:
  * On btrfs_reada_add, the root of the tree is inserted into a radix_tree.

@@ -979,10 +979,3 @@ int btrfs_reada_wait(void *handle)
 	return 0;
 }
 #endif
-
-void btrfs_reada_detach(void *handle)
-{
-	struct reada_control *rc = handle;
-
-	kref_put(&rc->refcnt, reada_control_release);
-}
Signature · 1 line
1.8.3.2
#9 lore raw

Re: [PATCH 6/7] btrfs: cleanup: removed unused 'btrfs_reada_detach'

Quoted 4 lines · Sergei Trofimovich

From: Sergei Trofimovich <[email protected]>

Found by uselex.rb:

Quoted 1 line

btrfs_reada_detach: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/built-in.o fs/btrfs/reada.o

even though the function is currently unused, I'm hesitating to remove it as it's part of the reada-API and might be handy for anyone going to use the API in the future.

-Arne

Quoted 43 lines
Signed-off-by Sergei Trofimovich <[email protected]>
---
 fs/btrfs/ctree.h | 1 -
 fs/btrfs/reada.c | 9 +--------
 2 files changed, 1 insertion(+), 9 deletions(-)
fs/btrfs/ctree.h +0 -1
index e91ab9e..f35e086 100644
@@ -3861,7 +3861,6 @@ struct reada_control {
 struct reada_control *btrfs_reada_add(struct btrfs_root *root,
 			      struct btrfs_key *start, struct btrfs_key *end);
 int btrfs_reada_wait(void *handle);
-void btrfs_reada_detach(void *handle);
 int btree_readahead_hook(struct btrfs_root *root, struct extent_buffer *eb,
 			 u64 start, int err);
fs/btrfs/reada.c +1 -8
index 1031b69..c41d470 100644
@@ -37,7 +37,7 @@
  * To trigger a readahead, btrfs_reada_add must be called. It will start
  * a read ahead for the given range [start, end) on tree root. The returned
  * handle can either be used to wait on the readahead to finish
- * (btrfs_reada_wait), or to send it to the background (btrfs_reada_detach).
+ * (btrfs_reada_wait).
  *
  * The read ahead works as follows:
  * On btrfs_reada_add, the root of the tree is inserted into a radix_tree.

@@ -979,10 +979,3 @@ int btrfs_reada_wait(void *handle)
 	return 0;
 }
 #endif
-
-void btrfs_reada_detach(void *handle)
-{
-	struct reada_control *rc = handle;
-
-	kref_put(&rc->refcnt, reada_control_release);
-}
#10 lore raw

Re: [PATCH 6/7] btrfs: cleanup: removed unused 'btrfs_reada_detach'

Quoted 9 lines · Arne Jansen
Quoted 4 lines · Sergei Trofimovich

From: Sergei Trofimovich <[email protected]>

Found by uselex.rb:

Quoted 1 line

btrfs_reada_detach: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/built-in.o fs/btrfs/reada.o

even though the function is currently unused, I'm hesitating to remove it as it's part of the reada-API and might be handy for anyone going to use the API in the future.

I agree. As replied here,
http://www.mail-archive.com/[email protected]/msg24047.html
please keep the function.

david

#11 lore raw

Re: [PATCH 6/7] btrfs: cleanup: removed unused 'btrfs_reada_detach'

Quoted 7 lines
Quoted 3 lines

even though the function is currently unused, I'm hesitating to remove it as it's part of the reada-API and might be handy for anyone going to use the API in the future.

I agree. As replied here,
http://www.mail-archive.com/[email protected]/msg24047.html
please keep the function.

If we're keeping score, put me down for being in favour of removing dead untested code. git ressurection is easy.

- z

#12 lore raw

Re: [PATCH 6/7] btrfs: cleanup: removed unused 'btrfs_reada_detach'

Quoted 10 lines · Zach Brown
Quoted 7 lines
Quoted 3 lines

even though the function is currently unused, I'm hesitating to remove it as it's part of the reada-API and might be handy for anyone going to use the API in the future.

I agree. As replied here,
http://www.mail-archive.com/[email protected]/msg24047.html
please keep the function.

If we're keeping score, put me down for being in favour of removing dead untested code. git ressurection is easy.

It's not really untested, it has been in use some time ago. But of course there's a chance that some changes broke it.
Yes, git ressurection is easy. To inform potential users, you might just leave a comment like this:

/*
* There has been a function once to detach from a running reada.
* If you need such functionality, just revert the commit that
* added this comment.
*/

-Arne

Quoted 3 lines

- z

#13 lore raw

Re: [PATCH 6/7] btrfs: cleanup: removed unused 'btrfs_reada_detach'

Quoted 17 lines · Arne Jansen
Quoted 10 lines · Zach Brown
Quoted 7 lines
Quoted 3 lines

even though the function is currently unused, I'm hesitating to remove it as it's part of the reada-API and might be handy for anyone going to use the API in the future.

I agree. As replied here,
http://www.mail-archive.com/[email protected]/msg24047.html
please keep the function.

If we're keeping score, put me down for being in favour of removing dead untested code. git ressurection is easy.

It's not really untested, it has been in use some time ago. But of course there's a chance that some changes broke it.
Yes, git ressurection is easy. To inform potential users, you might just leave a comment like this:

Quoted 5 lines

/*
* There has been a function once to detach from a running reada.
* If you need such functionality, just revert the commit that
* added this comment.
*/

And please write the exact commit sha1 instead of 'the commit' :)

I've used the _detach function when prototyping readdir readahead, that did not bring the speedup as expected so more work is needed, that's why I'm concerned about removing it. But, if Arne is ok with that, so be it.

david

author
#14 lore raw

PATCH 7/7 btrfs: cleanup: removed unused 'btrfs_get_inode_ref_index'

In reply to Sergei Trofimovich

From: Sergei Trofimovich <[email protected]>

Found by uselex.rb:

Quoted 1 line

btrfs_get_inode_ref_index: [R]: exported from: fs/btrfs/inode-item.o fs/btrfs/btrfs.o fs/btrfs/built-in.o

Signed-off-by Sergei Trofimovich <[email protected]>
---
 fs/btrfs/ctree.h      |  6 -----
 fs/btrfs/inode-item.c | 65 ---------------------------------------------------
 2 files changed, 71 deletions(-)
fs/btrfs/ctree.h +0 -6
index f35e086..4822c84 100644
@@ -3480,12 +3480,6 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans,
 			   struct btrfs_root *root,
 			   const char *name, int name_len,
 			   u64 inode_objectid, u64 ref_objectid, u64 *index);
-int btrfs_get_inode_ref_index(struct btrfs_trans_handle *trans,
-			      struct btrfs_root *root,
-			      struct btrfs_path *path,
-			      const char *name, int name_len,
-			      u64 inode_objectid, u64 ref_objectid, int mod,
-			      u64 *ret_index);
 int btrfs_insert_empty_inode(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *root,
 			     struct btrfs_path *path, u64 objectid);
fs/btrfs/inode-item.c +0 -65

77 lines not on this page · raw message

Signature · 1 line
1.8.3.2
#16 lore raw
In reply to Sergei Trofimovich

On 8/7/13 4:43 PM, Sergei Trofimovich wrote:

...

Quoted 3 lines

Meet uselex.rb: one-file script to parse 'nm' output:

https://github.com/trofi/uselex/blob/master/uselex.rb

Nice to meet you! I think I've met your close relative, ftp://ftp.samba.org/pub/unpacked/junkcode/findstatic.pl :)

#!/usr/bin/perl -w

# find a list of fns and variables in the code that could be static
# usually called with something like this:
#    findstatic.pl `find . -name "*.o"`
# Andrew Tridgell <[email protected]>

(just FWIW)

-Eric

author
#17 lore raw
Quoted 10 lines · On Wed, 07 Aug 2013 17:02:23 -0500 Eric Sandeen <[email protected]> wrote:

On 8/7/13 4:43 PM, Sergei Trofimovich wrote:

...

Quoted 3 lines

Meet uselex.rb: one-file script to parse 'nm' output:

https://github.com/trofi/uselex/blob/master/uselex.rb

Nice to meet you! I think I've met your close relative, ftp://ftp.samba.org/pub/unpacked/junkcode/findstatic.pl :)

Yeah, even finds the same.

Thanks!

Signature · 1 line
  Sergei