diff mbox

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

Message ID 1375911803-17318-7-git-send-email-slyich@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sergei Trofimovich Aug. 7, 2013, 9:43 p.m. UTC
From: Sergei Trofimovich <slyfox@gentoo.org>

Found by uselex.rb:
> btrfs_reada_detach: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/built-in.o fs/btrfs/reada.o

Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
---
 fs/btrfs/ctree.h | 1 -
 fs/btrfs/reada.c | 9 +--------
 2 files changed, 1 insertion(+), 9 deletions(-)

Comments

Arne Jansen Aug. 8, 2013, 7:33 a.m. UTC | #1
On 07.08.2013 23:43, Sergei Trofimovich wrote:
> From: Sergei Trofimovich <slyfox@gentoo.org>
> 
> Found by uselex.rb:
>> 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

> 
> Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
> ---
>  fs/btrfs/ctree.h | 1 -
>  fs/btrfs/reada.c | 9 +--------
>  2 files changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index e91ab9e..f35e086 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -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);
>  
> diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
> index 1031b69..c41d470 100644
> --- a/fs/btrfs/reada.c
> +++ b/fs/btrfs/reada.c
> @@ -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);
> -}

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba Aug. 8, 2013, 1:26 p.m. UTC | #2
On Thu, Aug 08, 2013 at 09:33:14AM +0200, Arne Jansen wrote:
> On 07.08.2013 23:43, Sergei Trofimovich wrote:
> > From: Sergei Trofimovich <slyfox@gentoo.org>
> > 
> > Found by uselex.rb:
> >> 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/linux-btrfs@vger.kernel.org/msg24047.html
please keep the function.

david
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zach Brown Aug. 8, 2013, 5:46 p.m. UTC | #3
> > 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/linux-btrfs@vger.kernel.org/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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arne Jansen Aug. 8, 2013, 7:11 p.m. UTC | #4
On 08/08/13 19:46, Zach Brown wrote:
>>> 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/linux-btrfs@vger.kernel.org/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

> 
> - z
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba Aug. 8, 2013, 10 p.m. UTC | #5
On Thu, Aug 08, 2013 at 09:11:01PM +0200, Arne Jansen wrote:
> On 08/08/13 19:46, Zach Brown wrote:
> >>> 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/linux-btrfs@vger.kernel.org/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.
>  */

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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index e91ab9e..f35e086 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -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);
 
diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
index 1031b69..c41d470 100644
--- a/fs/btrfs/reada.c
+++ b/fs/btrfs/reada.c
@@ -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);
-}