[v7,19/19] xfs: Remove xfs_attr_rmtval_remove
diff mbox series

Message ID 20200223020611.1802-20-allison.henderson@oracle.com
State New
Headers show
Series
  • xfs: Delayed Ready Attrs
Related show

Commit Message

Allison Collins Feb. 23, 2020, 2:06 a.m. UTC
xfs_attr_rmtval_remove is no longer used.  Clear it out now

Signed-off-by: Allison Collins <allison.henderson@oracle.com>
---
 fs/xfs/libxfs/xfs_attr_remote.c | 42 -----------------------------------------
 fs/xfs/xfs_trace.h              |  1 -
 2 files changed, 43 deletions(-)

Comments

Amir Goldstein Feb. 23, 2020, 1:54 p.m. UTC | #1
On Sun, Feb 23, 2020 at 4:07 AM Allison Collins
<allison.henderson@oracle.com> wrote:
>
> xfs_attr_rmtval_remove is no longer used.  Clear it out now
>
> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
> ---

Patch 12/19 add a new function similar to this one called
xfs_attr_rmtval_unmap() and now this function is removed.
I wonder if it wouldn't have been simpler to keep the original function
name and change its behavior to that of xfs_attr_rmtval_unmap().

Unless the function name change makes the logic change more clear
for the future users???

>  fs/xfs/libxfs/xfs_attr_remote.c | 42 -----------------------------------------
>  fs/xfs/xfs_trace.h              |  1 -
>  2 files changed, 43 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> index a0e79db..0cc0ec1 100644
> --- a/fs/xfs/libxfs/xfs_attr_remote.c
> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> @@ -734,48 +734,6 @@ xfs_attr_rmtval_invalidate(
>  }
>
>  /*
> - * Remove the value associated with an attribute by deleting the
> - * out-of-line buffer that it is stored on.
> - */
> -int
> -xfs_attr_rmtval_remove(
> -       struct xfs_da_args      *args)
> -{
> -       xfs_dablk_t             lblkno;
> -       int                     blkcnt;
> -       int                     error = 0;
> -       int                     done = 0;
> -
> -       trace_xfs_attr_rmtval_remove(args);
> -
> -       error = xfs_attr_rmtval_invalidate(args);
> -       if (error)
> -               return error;
> -       /*
> -        * Keep de-allocating extents until the remote-value region is gone.
> -        */
> -       lblkno = args->rmtblkno;
> -       blkcnt = args->rmtblkcnt;
> -       while (!done) {
> -               error = xfs_bunmapi(args->trans, args->dp, lblkno, blkcnt,
> -                                   XFS_BMAPI_ATTRFORK, 1, &done);
> -               if (error)
> -                       return error;
> -               error = xfs_defer_finish(&args->trans);
> -               if (error)
> -                       return error;
> -
> -               /*
> -                * Close out trans and start the next one in the chain.
> -                */
> -               error = xfs_trans_roll_inode(&args->trans, args->dp);
> -               if (error)
> -                       return error;
> -       }
> -       return 0;
> -}
> -
> -/*
>   * Remove the value associated with an attribute by deleting the out-of-line
>   * buffer that it is stored on. Returns EAGAIN for the caller to refresh the
>   * transaction and recall the function
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 159b8af..bf9a683 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -1775,7 +1775,6 @@ DEFINE_ATTR_EVENT(xfs_attr_refillstate);
>
>  DEFINE_ATTR_EVENT(xfs_attr_rmtval_get);
>  DEFINE_ATTR_EVENT(xfs_attr_rmtval_set);
> -DEFINE_ATTR_EVENT(xfs_attr_rmtval_remove);
>
>  #define DEFINE_DA_EVENT(name) \
>  DEFINE_EVENT(xfs_da_class, name, \
> --
> 2.7.4
>
Allison Collins Feb. 23, 2020, 6:50 p.m. UTC | #2
On 2/23/20 6:54 AM, Amir Goldstein wrote:
> On Sun, Feb 23, 2020 at 4:07 AM Allison Collins
> <allison.henderson@oracle.com> wrote:
>>
>> xfs_attr_rmtval_remove is no longer used.  Clear it out now
>>
>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>> ---
> 
> Patch 12/19 add a new function similar to this one called
> xfs_attr_rmtval_unmap() and now this function is removed.
> I wonder if it wouldn't have been simpler to keep the original function
> name and change its behavior to that of xfs_attr_rmtval_unmap().
I thought about this, but then decided against it.  Really what's 
happening is that the original function disappears across patches 13 and 
14, and I wanted to keep those patches focused on the state machine 
rather than lumping the helpers in with it.

> 
> Unless the function name change makes the logic change more clear
> for the future users???

I think they are about equivalent myself.  But the name difference helps 
the old function to stick around until it is completely phased out.  If 
folks feel strongly though, please chime in :-)

Thanks for the reviews!  I know it's a lot!

Allison

> 
>>   fs/xfs/libxfs/xfs_attr_remote.c | 42 -----------------------------------------
>>   fs/xfs/xfs_trace.h              |  1 -
>>   2 files changed, 43 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
>> index a0e79db..0cc0ec1 100644
>> --- a/fs/xfs/libxfs/xfs_attr_remote.c
>> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
>> @@ -734,48 +734,6 @@ xfs_attr_rmtval_invalidate(
>>   }
>>
>>   /*
>> - * Remove the value associated with an attribute by deleting the
>> - * out-of-line buffer that it is stored on.
>> - */
>> -int
>> -xfs_attr_rmtval_remove(
>> -       struct xfs_da_args      *args)
>> -{
>> -       xfs_dablk_t             lblkno;
>> -       int                     blkcnt;
>> -       int                     error = 0;
>> -       int                     done = 0;
>> -
>> -       trace_xfs_attr_rmtval_remove(args);
>> -
>> -       error = xfs_attr_rmtval_invalidate(args);
>> -       if (error)
>> -               return error;
>> -       /*
>> -        * Keep de-allocating extents until the remote-value region is gone.
>> -        */
>> -       lblkno = args->rmtblkno;
>> -       blkcnt = args->rmtblkcnt;
>> -       while (!done) {
>> -               error = xfs_bunmapi(args->trans, args->dp, lblkno, blkcnt,
>> -                                   XFS_BMAPI_ATTRFORK, 1, &done);
>> -               if (error)
>> -                       return error;
>> -               error = xfs_defer_finish(&args->trans);
>> -               if (error)
>> -                       return error;
>> -
>> -               /*
>> -                * Close out trans and start the next one in the chain.
>> -                */
>> -               error = xfs_trans_roll_inode(&args->trans, args->dp);
>> -               if (error)
>> -                       return error;
>> -       }
>> -       return 0;
>> -}
>> -
>> -/*
>>    * Remove the value associated with an attribute by deleting the out-of-line
>>    * buffer that it is stored on. Returns EAGAIN for the caller to refresh the
>>    * transaction and recall the function
>> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
>> index 159b8af..bf9a683 100644
>> --- a/fs/xfs/xfs_trace.h
>> +++ b/fs/xfs/xfs_trace.h
>> @@ -1775,7 +1775,6 @@ DEFINE_ATTR_EVENT(xfs_attr_refillstate);
>>
>>   DEFINE_ATTR_EVENT(xfs_attr_rmtval_get);
>>   DEFINE_ATTR_EVENT(xfs_attr_rmtval_set);
>> -DEFINE_ATTR_EVENT(xfs_attr_rmtval_remove);
>>
>>   #define DEFINE_DA_EVENT(name) \
>>   DEFINE_EVENT(xfs_da_class, name, \
>> --
>> 2.7.4
>>

Patch
diff mbox series

diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index a0e79db..0cc0ec1 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -734,48 +734,6 @@  xfs_attr_rmtval_invalidate(
 }
 
 /*
- * Remove the value associated with an attribute by deleting the
- * out-of-line buffer that it is stored on.
- */
-int
-xfs_attr_rmtval_remove(
-	struct xfs_da_args      *args)
-{
-	xfs_dablk_t		lblkno;
-	int			blkcnt;
-	int			error = 0;
-	int			done = 0;
-
-	trace_xfs_attr_rmtval_remove(args);
-
-	error = xfs_attr_rmtval_invalidate(args);
-	if (error)
-		return error;
-	/*
-	 * Keep de-allocating extents until the remote-value region is gone.
-	 */
-	lblkno = args->rmtblkno;
-	blkcnt = args->rmtblkcnt;
-	while (!done) {
-		error = xfs_bunmapi(args->trans, args->dp, lblkno, blkcnt,
-				    XFS_BMAPI_ATTRFORK, 1, &done);
-		if (error)
-			return error;
-		error = xfs_defer_finish(&args->trans);
-		if (error)
-			return error;
-
-		/*
-		 * Close out trans and start the next one in the chain.
-		 */
-		error = xfs_trans_roll_inode(&args->trans, args->dp);
-		if (error)
-			return error;
-	}
-	return 0;
-}
-
-/*
  * Remove the value associated with an attribute by deleting the out-of-line
  * buffer that it is stored on. Returns EAGAIN for the caller to refresh the
  * transaction and recall the function
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 159b8af..bf9a683 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -1775,7 +1775,6 @@  DEFINE_ATTR_EVENT(xfs_attr_refillstate);
 
 DEFINE_ATTR_EVENT(xfs_attr_rmtval_get);
 DEFINE_ATTR_EVENT(xfs_attr_rmtval_set);
-DEFINE_ATTR_EVENT(xfs_attr_rmtval_remove);
 
 #define DEFINE_DA_EVENT(name) \
 DEFINE_EVENT(xfs_da_class, name, \