[v6,13/16] xfs: Add helper function xfs_attr_rmtval_unmap
diff mbox series

Message ID 20200118225035.19503-14-allison.henderson@oracle.com
State Superseded
Headers show
Series
  • xfs: Delay Ready Attributes
Related show

Commit Message

Allison Collins Jan. 18, 2020, 10:50 p.m. UTC
This function is similar to xfs_attr_rmtval_remove, but adapted to
return. EAGAIN for new transactions. We will use this later when we
introduce delayed attributes

Signed-off-by: Allison Collins <allison.henderson@oracle.com>
---
 fs/xfs/libxfs/xfs_attr_remote.c | 27 +++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_attr_remote.h |  1 +
 2 files changed, 28 insertions(+)

Comments

Darrick J. Wong Jan. 21, 2020, 11:24 p.m. UTC | #1
On Sat, Jan 18, 2020 at 03:50:32PM -0700, Allison Collins wrote:
> This function is similar to xfs_attr_rmtval_remove, but adapted to
> return. EAGAIN for new transactions. We will use this later when we

"..to return -EAGAIN"?

> introduce delayed attributes
> 
> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_attr_remote.c | 27 +++++++++++++++++++++++++++
>  fs/xfs/libxfs/xfs_attr_remote.h |  1 +
>  2 files changed, 28 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> index 9b4fa2a..f2ee0b8 100644
> --- a/fs/xfs/libxfs/xfs_attr_remote.c
> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> @@ -684,3 +684,30 @@ xfs_attr_rmtval_remove(
>  	}
>  	return 0;
>  }
> +
> +/*
> + * Unmap value blocks for this attr.  This is similar to
> + * xfs_attr_rmtval_remove, but adapted to to return EAGAIN for new transactions

Do you have to scan for and invalidate any incore buffers for the remote
value?  Or will that be performed by another step?  Hard to tell because
this function doesn't have any callers.

> + */
> +int
> +xfs_attr_rmtval_unmap(
> +	struct xfs_da_args	*args)
> +{
> +	int	error, done;
> +
> +	/*
> +	 * Unmap value blocks for this attr.  This is similar to
> +	 * xfs_attr_rmtval_remove, but open coded here to return EAGAIN
> +	 * for new transactions
> +	 */
> +	error = xfs_bunmapi(args->trans, args->dp,
> +		    args->rmtblkno, args->rmtblkcnt,
> +		    XFS_BMAPI_ATTRFORK, 1, &done);
> +	if (error)
> +		return error;
> +
> +	if (!done)
> +		return -EAGAIN;

return done ? 0 : -EAGAIN; ?

--D

> +
> +	return 0;
> +}
> diff --git a/fs/xfs/libxfs/xfs_attr_remote.h b/fs/xfs/libxfs/xfs_attr_remote.h
> index 85f2593..7ab3770 100644
> --- a/fs/xfs/libxfs/xfs_attr_remote.h
> +++ b/fs/xfs/libxfs/xfs_attr_remote.h
> @@ -12,4 +12,5 @@ int xfs_attr_rmtval_get(struct xfs_da_args *args);
>  int xfs_attr_rmtval_set(struct xfs_da_args *args);
>  int xfs_attr_rmtval_remove(struct xfs_da_args *args);
>  int xfs_attr_rmtval_invalidate(struct xfs_da_args *args);
> +int xfs_attr_rmtval_unmap(struct xfs_da_args *args);
>  #endif /* __XFS_ATTR_REMOTE_H__ */
> -- 
> 2.7.4
>
Allison Collins Jan. 22, 2020, 5:23 a.m. UTC | #2
On 1/21/20 4:24 PM, Darrick J. Wong wrote:
> On Sat, Jan 18, 2020 at 03:50:32PM -0700, Allison Collins wrote:
>> This function is similar to xfs_attr_rmtval_remove, but adapted to
>> return. EAGAIN for new transactions. We will use this later when we
> 
> "..to return -EAGAIN"?
yees, I noticed that after I sent it... will fix :-)
> 
>> introduce delayed attributes
>>
>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>> ---
>>   fs/xfs/libxfs/xfs_attr_remote.c | 27 +++++++++++++++++++++++++++
>>   fs/xfs/libxfs/xfs_attr_remote.h |  1 +
>>   2 files changed, 28 insertions(+)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
>> index 9b4fa2a..f2ee0b8 100644
>> --- a/fs/xfs/libxfs/xfs_attr_remote.c
>> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
>> @@ -684,3 +684,30 @@ xfs_attr_rmtval_remove(
>>   	}
>>   	return 0;
>>   }
>> +
>> +/*
>> + * Unmap value blocks for this attr.  This is similar to
>> + * xfs_attr_rmtval_remove, but adapted to to return EAGAIN for new transactions
note to self: fix too many to's

> 
> Do you have to scan for and invalidate any incore buffers for the remote
> value?  Or will that be performed by another step?  Hard to tell because
> this function doesn't have any callers.
Yes, that happens in xfs_attr_rmtval_invalidate.  Right after we mark 
incomplete, and before we fall into the bunmapi loop I mentioned in the 
last patch.  Basically this function becomes the inside of that loop.


> 
>> + */
>> +int
>> +xfs_attr_rmtval_unmap(
>> +	struct xfs_da_args	*args)
>> +{
>> +	int	error, done;
>> +
>> +	/*
>> +	 * Unmap value blocks for this attr.  This is similar to
>> +	 * xfs_attr_rmtval_remove, but open coded here to return EAGAIN
>> +	 * for new transactions
>> +	 */
>> +	error = xfs_bunmapi(args->trans, args->dp,
>> +		    args->rmtblkno, args->rmtblkcnt,
>> +		    XFS_BMAPI_ATTRFORK, 1, &done);
>> +	if (error)
>> +		return error;
>> +
>> +	if (!done)
>> +		return -EAGAIN;
> 
> return done ? 0 : -EAGAIN; ?
I believe Christoph asked for the return 0;  I am not particular about 
it as long a people are in agreement.  If people feel strongly one way 
or another please chime in.

Allison
> 
> --D
> 


>> +
>> +	return 0;
>> +}
>> diff --git a/fs/xfs/libxfs/xfs_attr_remote.h b/fs/xfs/libxfs/xfs_attr_remote.h
>> index 85f2593..7ab3770 100644
>> --- a/fs/xfs/libxfs/xfs_attr_remote.h
>> +++ b/fs/xfs/libxfs/xfs_attr_remote.h
>> @@ -12,4 +12,5 @@ int xfs_attr_rmtval_get(struct xfs_da_args *args);
>>   int xfs_attr_rmtval_set(struct xfs_da_args *args);
>>   int xfs_attr_rmtval_remove(struct xfs_da_args *args);
>>   int xfs_attr_rmtval_invalidate(struct xfs_da_args *args);
>> +int xfs_attr_rmtval_unmap(struct xfs_da_args *args);
>>   #endif /* __XFS_ATTR_REMOTE_H__ */
>> -- 
>> 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 9b4fa2a..f2ee0b8 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -684,3 +684,30 @@  xfs_attr_rmtval_remove(
 	}
 	return 0;
 }
+
+/*
+ * Unmap value blocks for this attr.  This is similar to
+ * xfs_attr_rmtval_remove, but adapted to to return EAGAIN for new transactions
+ */
+int
+xfs_attr_rmtval_unmap(
+	struct xfs_da_args	*args)
+{
+	int	error, done;
+
+	/*
+	 * Unmap value blocks for this attr.  This is similar to
+	 * xfs_attr_rmtval_remove, but open coded here to return EAGAIN
+	 * for new transactions
+	 */
+	error = xfs_bunmapi(args->trans, args->dp,
+		    args->rmtblkno, args->rmtblkcnt,
+		    XFS_BMAPI_ATTRFORK, 1, &done);
+	if (error)
+		return error;
+
+	if (!done)
+		return -EAGAIN;
+
+	return 0;
+}
diff --git a/fs/xfs/libxfs/xfs_attr_remote.h b/fs/xfs/libxfs/xfs_attr_remote.h
index 85f2593..7ab3770 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.h
+++ b/fs/xfs/libxfs/xfs_attr_remote.h
@@ -12,4 +12,5 @@  int xfs_attr_rmtval_get(struct xfs_da_args *args);
 int xfs_attr_rmtval_set(struct xfs_da_args *args);
 int xfs_attr_rmtval_remove(struct xfs_da_args *args);
 int xfs_attr_rmtval_invalidate(struct xfs_da_args *args);
+int xfs_attr_rmtval_unmap(struct xfs_da_args *args);
 #endif /* __XFS_ATTR_REMOTE_H__ */