[PATCH] ceph: fix setting of xattrs on async created inodes

[PATCH] ceph: fix setting of xattrs on async created inodes

[Jeff Layton]

Currently when we create a file, we spin up an xattr buffer to send
along with the create request. If we end up doing an async create
however, then we currently pass down a zero-length xattr buffer.

Fix the code to send down the xattr buffer in req->r_pagelist. If the
xattrs span more than a page, however give up and don't try to do an
async create.

Fixes: 9a8d03ca2e2c ("ceph: attempt to do async create when possible")
URL: https://bugzilla.redhat.com/show_bug.cgi?id=2063929
Reported-by: John Fortin <fortinj66@gmail.com>
Reported-by: Sri Ramanujam <sri@ramanujam.io>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/file.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 6c9e837aa1d3..8c8226c0feac 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -629,9 +629,15 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry,
 	iinfo.change_attr = 1;
 	ceph_encode_timespec64(&iinfo.btime, &now);
 
-	iinfo.xattr_len = ARRAY_SIZE(xattr_buf);
-	iinfo.xattr_data = xattr_buf;
-	memset(iinfo.xattr_data, 0, iinfo.xattr_len);
+	if (req->r_pagelist) {
+		iinfo.xattr_len = req->r_pagelist->length;
+		iinfo.xattr_data = req->r_pagelist->mapped_tail;
+	} else {
+		/* fake it */
+		iinfo.xattr_len = ARRAY_SIZE(xattr_buf);
+		iinfo.xattr_data = xattr_buf;
+		memset(iinfo.xattr_data, 0, iinfo.xattr_len);
+	}
 
 	in.ino = cpu_to_le64(vino.ino);
 	in.snapid = cpu_to_le64(CEPH_NOSNAP);
@@ -743,6 +749,10 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 		err = ceph_security_init_secctx(dentry, mode, &as_ctx);
 		if (err < 0)
 			goto out_ctx;
+		/* Async create can't handle more than a page of xattrs */
+		if (as_ctx.pagelist &&
+		    !list_is_singular(&as_ctx.pagelist->head))
+			try_async = false;
 	} else if (!d_in_lookup(dentry)) {
 		/* If it's not being looked up, it's negative */
 		return -ENOENT;
-- 
2.35.1

Re: [PATCH] ceph: fix setting of xattrs on async created inodes

[Jeff Layton]

On Mon, 2022-04-25 at 15:54 -0400, Jeff Layton wrote:
> Currently when we create a file, we spin up an xattr buffer to send
> along with the create request. If we end up doing an async create
> however, then we currently pass down a zero-length xattr buffer.
> 
> Fix the code to send down the xattr buffer in req->r_pagelist. If the
> xattrs span more than a page, however give up and don't try to do an
> async create.
> 
> Fixes: 9a8d03ca2e2c ("ceph: attempt to do async create when possible")
> URL: https://bugzilla.redhat.com/show_bug.cgi?id=2063929
> Reported-by: John Fortin <fortinj66@gmail.com>
> Reported-by: Sri Ramanujam <sri@ramanujam.io>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/file.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 6c9e837aa1d3..8c8226c0feac 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -629,9 +629,15 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry,
>  	iinfo.change_attr = 1;
>  	ceph_encode_timespec64(&iinfo.btime, &now);
>  
> -	iinfo.xattr_len = ARRAY_SIZE(xattr_buf);
> -	iinfo.xattr_data = xattr_buf;
> -	memset(iinfo.xattr_data, 0, iinfo.xattr_len);
> +	if (req->r_pagelist) {
> +		iinfo.xattr_len = req->r_pagelist->length;
> +		iinfo.xattr_data = req->r_pagelist->mapped_tail;
> +	} else {
> +		/* fake it */
> +		iinfo.xattr_len = ARRAY_SIZE(xattr_buf);
> +		iinfo.xattr_data = xattr_buf;
> +		memset(iinfo.xattr_data, 0, iinfo.xattr_len);
> +	}
>  
>  	in.ino = cpu_to_le64(vino.ino);
>  	in.snapid = cpu_to_le64(CEPH_NOSNAP);
> @@ -743,6 +749,10 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>  		err = ceph_security_init_secctx(dentry, mode, &as_ctx);
>  		if (err < 0)
>  			goto out_ctx;
> +		/* Async create can't handle more than a page of xattrs */
> +		if (as_ctx.pagelist &&
> +		    !list_is_singular(&as_ctx.pagelist->head))
> +			try_async = false;
>  	} else if (!d_in_lookup(dentry)) {
>  		/* If it's not being looked up, it's negative */
>  		return -ENOENT;

Oh, I meant to mark this for stable as well. Xiubo, can you do that when
you merge it?

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH] ceph: fix setting of xattrs on async created inodes

[Xiubo Li]

On 4/26/22 3:57 AM, Jeff Layton wrote:
> On Mon, 2022-04-25 at 15:54 -0400, Jeff Layton wrote:
>> Currently when we create a file, we spin up an xattr buffer to send
>> along with the create request. If we end up doing an async create
>> however, then we currently pass down a zero-length xattr buffer.
>>
>> Fix the code to send down the xattr buffer in req->r_pagelist. If the
>> xattrs span more than a page, however give up and don't try to do an
>> async create.
>>
>> Fixes: 9a8d03ca2e2c ("ceph: attempt to do async create when possible")
>> URL: https://bugzilla.redhat.com/show_bug.cgi?id=2063929
>> Reported-by: John Fortin <fortinj66@gmail.com>
>> Reported-by: Sri Ramanujam <sri@ramanujam.io>
>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>> ---
>>   fs/ceph/file.c | 16 +++++++++++++---
>>   1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> index 6c9e837aa1d3..8c8226c0feac 100644
>> --- a/fs/ceph/file.c
>> +++ b/fs/ceph/file.c
>> @@ -629,9 +629,15 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry,
>>   	iinfo.change_attr = 1;
>>   	ceph_encode_timespec64(&iinfo.btime, &now);
>>   
>> -	iinfo.xattr_len = ARRAY_SIZE(xattr_buf);
>> -	iinfo.xattr_data = xattr_buf;
>> -	memset(iinfo.xattr_data, 0, iinfo.xattr_len);
>> +	if (req->r_pagelist) {
>> +		iinfo.xattr_len = req->r_pagelist->length;
>> +		iinfo.xattr_data = req->r_pagelist->mapped_tail;
>> +	} else {
>> +		/* fake it */
>> +		iinfo.xattr_len = ARRAY_SIZE(xattr_buf);
>> +		iinfo.xattr_data = xattr_buf;
>> +		memset(iinfo.xattr_data, 0, iinfo.xattr_len);
>> +	}
>>   
>>   	in.ino = cpu_to_le64(vino.ino);
>>   	in.snapid = cpu_to_le64(CEPH_NOSNAP);
>> @@ -743,6 +749,10 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>>   		err = ceph_security_init_secctx(dentry, mode, &as_ctx);
>>   		if (err < 0)
>>   			goto out_ctx;
>> +		/* Async create can't handle more than a page of xattrs */
>> +		if (as_ctx.pagelist &&
>> +		    !list_is_singular(&as_ctx.pagelist->head))
>> +			try_async = false;
>>   	} else if (!d_in_lookup(dentry)) {
>>   		/* If it's not being looked up, it's negative */
>>   		return -ENOENT;
> Oh, I meant to mark this for stable as well. Xiubo, can you do that when
> you merge it?

Sure, Jeff.

Looks nice. Merged into the testing branch.

Thanks

-- Xiubo


Looks good. I fixed up the minor spelling error in the comment that
Venky noticed too. Merged into testing branch.

Thanks,



> Thanks,