diff mbox series

[4/4] ceph: check if LOOKUPNAME request was aborted when filling trace

Message ID 20180928094355.6049-4-zyan@redhat.com (mailing list archive)
State New, archived
Headers show
Series [1/4] Revert "ceph: fix dentry leak in splice_dentry()" | expand

Commit Message

Yan, Zheng Sept. 28, 2018, 9:43 a.m. UTC 
d_lookup()/d_alloc() require parent inode locked. Parent inode is
not locked if request is aborted.

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
---
 fs/ceph/inode.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Jeff Layton Oct. 11, 2018, 3:07 p.m. UTC | #1 
On Fri, 2018-09-28 at 17:43 +0800, Yan, Zheng wrote:
> d_lookup()/d_alloc() require parent inode locked. Parent inode is
> not locked if request is aborted.
> 
> Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
> ---
>  fs/ceph/inode.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index a58e82a64545..b41dc0ed3a35 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -1203,7 +1203,9 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
>  			WARN_ON_ONCE(1);
>  		}
>  
> -		if (dir && req->r_op == CEPH_MDS_OP_LOOKUPNAME) {
> +		if (dir && req->r_op == CEPH_MDS_OP_LOOKUPNAME &&
> +		    test_bit(CEPH_MDS_R_PARENT_LOCKED, &req->r_req_flags) &&
> +		    !test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) {
>  			struct qstr dname;
>  			struct dentry *dn, *parent;
>  

I think the code looks fine, but the description is not accurate.

The parent is still locked in the case of an ABORTED request, but if the
thing was aborted then we don't want to make any changes based on the
reply since it's not meaningful.

Also, we really don't want to make any changes if the parent isn't
locked. It generally always is for a LOOKUPNAME request but checking it
every time is more future proof.

Reviewed-by: Jeff Layton <jlayton@redhat.com>
diff mbox series

Patch

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index a58e82a64545..b41dc0ed3a35 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -1203,7 +1203,9 @@  int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
 			WARN_ON_ONCE(1);
 		}
 
-		if (dir && req->r_op == CEPH_MDS_OP_LOOKUPNAME) {
+		if (dir && req->r_op == CEPH_MDS_OP_LOOKUPNAME &&
+		    test_bit(CEPH_MDS_R_PARENT_LOCKED, &req->r_req_flags) &&
+		    !test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) {
 			struct qstr dname;
 			struct dentry *dn, *parent;