Message ID | 20170201114914.20808-3-jlayton@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
> On 1 Feb 2017, at 19:49, Jeff Layton <jlayton@redhat.com> wrote: > > ...and simplify the handling of it in ceph_fill_trace. > > Signed-off-by: Jeff Layton <jlayton@redhat.com> > --- > fs/ceph/inode.c | 17 +++++++++-------- > fs/ceph/mds_client.c | 8 ++++---- > fs/ceph/mds_client.h | 2 +- > 3 files changed, 14 insertions(+), 13 deletions(-) > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c > index 4926265f4223..bd2e94a78057 100644 > --- a/fs/ceph/inode.c > +++ b/fs/ceph/inode.c > @@ -1233,8 +1233,8 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, > > err = fill_inode(in, req->r_locked_page, &rinfo->targeti, NULL, > session, req->r_request_started, > - (!req->r_aborted && rinfo->head->result == 0) ? > - req->r_fmode : -1, > + (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags) && > + rinfo->head->result == 0) ? req->r_fmode : -1, > &req->r_caps_reservation); > if (err < 0) { > pr_err("fill_inode badness %p %llx.%llx\n", > @@ -1243,12 +1243,14 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, > } > } > > + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) > + goto done; > + This seems wrong. If we got the reply, we should update inode’s caps even request is aborted. Otherwise, MDS can hang at revoking the dropped caps. > /* > * ignore null lease/binding on snapdir ENOENT, or else we > * will have trouble splicing in the virtual snapdir later > */ > - if (rinfo->head->is_dentry && !req->r_aborted && > - req->r_locked_dir && > + if (rinfo->head->is_dentry && req->r_locked_dir && > (rinfo->head->is_target || strncmp(req->r_dentry->d_name.name, > fsc->mount_options->snapdir_name, > req->r_dentry->d_name.len))) { > @@ -1351,9 +1353,8 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, > update_dentry_lease(dn, rinfo->dlease, session, > req->r_request_started); > dout(" final dn %p\n", dn); > - } else if (!req->r_aborted && > - (req->r_op == CEPH_MDS_OP_LOOKUPSNAP || > - req->r_op == CEPH_MDS_OP_MKSNAP)) { > + } else if (req->r_op == CEPH_MDS_OP_LOOKUPSNAP || > + req->r_op == CEPH_MDS_OP_MKSNAP) { > struct dentry *dn = req->r_dentry; > struct inode *dir = req->r_locked_dir; > > @@ -1478,7 +1479,7 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req, > u32 fpos_offset; > struct ceph_readdir_cache_control cache_ctl = {}; > > - if (req->r_aborted) > + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) > return readdir_prepopulate_inodes_only(req, session); > > if (rinfo->hash_order && req->r_path2) { > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > index 1f2ef02832d9..a5156b6a0aed 100644 > --- a/fs/ceph/mds_client.c > +++ b/fs/ceph/mds_client.c > @@ -2115,7 +2115,7 @@ static int __do_request(struct ceph_mds_client *mdsc, > int err = 0; > > if (req->r_err || req->r_got_result) { > - if (req->r_aborted) > + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) > __unregister_request(mdsc, req); > goto out; > } > @@ -2331,7 +2331,7 @@ int ceph_mdsc_do_request(struct ceph_mds_client *mdsc, > */ > mutex_lock(&req->r_fill_mutex); > req->r_err = err; > - req->r_aborted = true; > + set_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags); > mutex_unlock(&req->r_fill_mutex); > > if (req->r_locked_dir && > @@ -2538,7 +2538,7 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg) > } > out_err: > mutex_lock(&mdsc->mutex); > - if (!req->r_aborted) { > + if (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) { > if (err) { > req->r_err = err; > } else { > @@ -2587,7 +2587,7 @@ static void handle_forward(struct ceph_mds_client *mdsc, > goto out; /* dup reply? */ > } > > - if (req->r_aborted) { > + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) { > dout("forward tid %llu aborted, unregistering\n", tid); > __unregister_request(mdsc, req); > } else if (fwd_seq <= req->r_num_fwd) { > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h > index a58cacccc986..3da20955d5e6 100644 > --- a/fs/ceph/mds_client.h > +++ b/fs/ceph/mds_client.h > @@ -206,6 +206,7 @@ struct ceph_mds_request { > struct inode *r_target_inode; /* resulting inode */ > > #define CEPH_MDS_R_DIRECT_IS_HASH (1) /* r_direct_hash is valid */ > +#define CEPH_MDS_R_ABORTED (2) /* call was aborted */ > unsigned long r_req_flags; > > struct mutex r_fill_mutex; > @@ -236,7 +237,6 @@ struct ceph_mds_request { > struct ceph_mds_reply_info_parsed r_reply_info; > struct page *r_locked_page; > int r_err; > - bool r_aborted; > > unsigned long r_timeout; /* optional. jiffies, 0 is "wait forever" */ > unsigned long r_started; /* start time to measure timeout against */ > -- > 2.9.3 > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2017-02-02 at 16:06 +0800, Yan, Zheng wrote: > > On 1 Feb 2017, at 19:49, Jeff Layton <jlayton@redhat.com> wrote: > > > > ...and simplify the handling of it in ceph_fill_trace. > > > > Signed-off-by: Jeff Layton <jlayton@redhat.com> > > --- > > fs/ceph/inode.c | 17 +++++++++-------- > > fs/ceph/mds_client.c | 8 ++++---- > > fs/ceph/mds_client.h | 2 +- > > 3 files changed, 14 insertions(+), 13 deletions(-) > > > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c > > index 4926265f4223..bd2e94a78057 100644 > > --- a/fs/ceph/inode.c > > +++ b/fs/ceph/inode.c > > @@ -1233,8 +1233,8 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, > > > > err = fill_inode(in, req->r_locked_page, &rinfo->targeti, NULL, > > session, req->r_request_started, > > - (!req->r_aborted && rinfo->head->result == 0) ? > > - req->r_fmode : -1, > > + (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags) && > > + rinfo->head->result == 0) ? req->r_fmode : -1, > > &req->r_caps_reservation); > > if (err < 0) { > > pr_err("fill_inode badness %p %llx.%llx\n", > > @@ -1243,12 +1243,14 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, > > } > > } > > > > + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) > > + goto done; > > + > > This seems wrong. If we got the reply, we should update inode’s caps even request is aborted. > Otherwise, MDS can hang at revoking the dropped caps. > FWIW, I hit that exact problem in an earlier iteration of this patchset until I figured out what was going on. The caps do get updated with this patch. That gets done in fill_inode, and that's called even when the call was aborted. None of the "move the flag into r_req_flags" patches materially change the behavior of the code. In this patch, we're just moving both of the subsequent !req->r_aborted flag checks into this one spot. > > /* > > * ignore null lease/binding on snapdir ENOENT, or else we > > * will have trouble splicing in the virtual snapdir later > > */ > > - if (rinfo->head->is_dentry && !req->r_aborted && > > - req->r_locked_dir && > > + if (rinfo->head->is_dentry && req->r_locked_dir && > > (rinfo->head->is_target || strncmp(req->r_dentry->d_name.name, > > fsc->mount_options->snapdir_name, > > req->r_dentry->d_name.len))) { > > @@ -1351,9 +1353,8 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, > > update_dentry_lease(dn, rinfo->dlease, session, > > req->r_request_started); > > dout(" final dn %p\n", dn); > > - } else if (!req->r_aborted && > > - (req->r_op == CEPH_MDS_OP_LOOKUPSNAP || > > - req->r_op == CEPH_MDS_OP_MKSNAP)) { > > + } else if (req->r_op == CEPH_MDS_OP_LOOKUPSNAP || > > + req->r_op == CEPH_MDS_OP_MKSNAP) { > > struct dentry *dn = req->r_dentry; > > struct inode *dir = req->r_locked_dir; > > > > @@ -1478,7 +1479,7 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req, > > u32 fpos_offset; > > struct ceph_readdir_cache_control cache_ctl = {}; > > > > - if (req->r_aborted) > > + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) > > return readdir_prepopulate_inodes_only(req, session); > > > > if (rinfo->hash_order && req->r_path2) { > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > > index 1f2ef02832d9..a5156b6a0aed 100644 > > --- a/fs/ceph/mds_client.c > > +++ b/fs/ceph/mds_client.c > > @@ -2115,7 +2115,7 @@ static int __do_request(struct ceph_mds_client *mdsc, > > int err = 0; > > > > if (req->r_err || req->r_got_result) { > > - if (req->r_aborted) > > + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) > > __unregister_request(mdsc, req); > > goto out; > > } > > @@ -2331,7 +2331,7 @@ int ceph_mdsc_do_request(struct ceph_mds_client *mdsc, > > */ > > mutex_lock(&req->r_fill_mutex); > > req->r_err = err; > > - req->r_aborted = true; > > + set_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags); > > mutex_unlock(&req->r_fill_mutex); > > > > if (req->r_locked_dir && > > @@ -2538,7 +2538,7 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg) > > } > > out_err: > > mutex_lock(&mdsc->mutex); > > - if (!req->r_aborted) { > > + if (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) { > > if (err) { > > req->r_err = err; > > } else { > > @@ -2587,7 +2587,7 @@ static void handle_forward(struct ceph_mds_client *mdsc, > > goto out; /* dup reply? */ > > } > > > > - if (req->r_aborted) { > > + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) { > > dout("forward tid %llu aborted, unregistering\n", tid); > > __unregister_request(mdsc, req); > > } else if (fwd_seq <= req->r_num_fwd) { > > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h > > index a58cacccc986..3da20955d5e6 100644 > > --- a/fs/ceph/mds_client.h > > +++ b/fs/ceph/mds_client.h > > @@ -206,6 +206,7 @@ struct ceph_mds_request { > > struct inode *r_target_inode; /* resulting inode */ > > > > #define CEPH_MDS_R_DIRECT_IS_HASH (1) /* r_direct_hash is valid */ > > +#define CEPH_MDS_R_ABORTED (2) /* call was aborted */ > > unsigned long r_req_flags; > > > > struct mutex r_fill_mutex; > > @@ -236,7 +237,6 @@ struct ceph_mds_request { > > struct ceph_mds_reply_info_parsed r_reply_info; > > struct page *r_locked_page; > > int r_err; > > - bool r_aborted; > > > > unsigned long r_timeout; /* optional. jiffies, 0 is "wait forever" */ > > unsigned long r_started; /* start time to measure timeout against */ > > -- > > 2.9.3 > > > >
> On 2 Feb 2017, at 19:26, Jeff Layton <jlayton@redhat.com> wrote: > > On Thu, 2017-02-02 at 16:06 +0800, Yan, Zheng wrote: >>> On 1 Feb 2017, at 19:49, Jeff Layton <jlayton@redhat.com> wrote: >>> >>> ...and simplify the handling of it in ceph_fill_trace. >>> >>> Signed-off-by: Jeff Layton <jlayton@redhat.com> >>> --- >>> fs/ceph/inode.c | 17 +++++++++-------- >>> fs/ceph/mds_client.c | 8 ++++---- >>> fs/ceph/mds_client.h | 2 +- >>> 3 files changed, 14 insertions(+), 13 deletions(-) >>> >>> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c >>> index 4926265f4223..bd2e94a78057 100644 >>> --- a/fs/ceph/inode.c >>> +++ b/fs/ceph/inode.c >>> @@ -1233,8 +1233,8 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, >>> >>> err = fill_inode(in, req->r_locked_page, &rinfo->targeti, NULL, >>> session, req->r_request_started, >>> - (!req->r_aborted && rinfo->head->result == 0) ? >>> - req->r_fmode : -1, >>> + (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags) && >>> + rinfo->head->result == 0) ? req->r_fmode : -1, >>> &req->r_caps_reservation); >>> if (err < 0) { >>> pr_err("fill_inode badness %p %llx.%llx\n", >>> @@ -1243,12 +1243,14 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, >>> } >>> } >>> >>> + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) >>> + goto done; >>> + >> >> This seems wrong. If we got the reply, we should update inode’s caps even request is aborted. >> Otherwise, MDS can hang at revoking the dropped caps. >> > > FWIW, I hit that exact problem in an earlier iteration of this patchset > until I figured out what was going on. The caps do get updated with this > patch. That gets done in fill_inode, and that's called even when the > call was aborted. > > None of the "move the flag into r_req_flags" patches materially change > the behavior of the code. In this patch, we're just moving both of the > subsequent !req->r_aborted flag checks into this one spot. I think you are right. Thank you for explanation Yan, Zheng > >>> /* >>> * ignore null lease/binding on snapdir ENOENT, or else we >>> * will have trouble splicing in the virtual snapdir later >>> */ >>> - if (rinfo->head->is_dentry && !req->r_aborted && >>> - req->r_locked_dir && >>> + if (rinfo->head->is_dentry && req->r_locked_dir && >>> (rinfo->head->is_target || strncmp(req->r_dentry->d_name.name, >>> fsc->mount_options->snapdir_name, >>> req->r_dentry->d_name.len))) { >>> @@ -1351,9 +1353,8 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, >>> update_dentry_lease(dn, rinfo->dlease, session, >>> req->r_request_started); >>> dout(" final dn %p\n", dn); >>> - } else if (!req->r_aborted && >>> - (req->r_op == CEPH_MDS_OP_LOOKUPSNAP || >>> - req->r_op == CEPH_MDS_OP_MKSNAP)) { >>> + } else if (req->r_op == CEPH_MDS_OP_LOOKUPSNAP || >>> + req->r_op == CEPH_MDS_OP_MKSNAP) { >>> struct dentry *dn = req->r_dentry; >>> struct inode *dir = req->r_locked_dir; >>> >>> @@ -1478,7 +1479,7 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req, >>> u32 fpos_offset; >>> struct ceph_readdir_cache_control cache_ctl = {}; >>> >>> - if (req->r_aborted) >>> + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) >>> return readdir_prepopulate_inodes_only(req, session); >>> >>> if (rinfo->hash_order && req->r_path2) { >>> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c >>> index 1f2ef02832d9..a5156b6a0aed 100644 >>> --- a/fs/ceph/mds_client.c >>> +++ b/fs/ceph/mds_client.c >>> @@ -2115,7 +2115,7 @@ static int __do_request(struct ceph_mds_client *mdsc, >>> int err = 0; >>> >>> if (req->r_err || req->r_got_result) { >>> - if (req->r_aborted) >>> + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) >>> __unregister_request(mdsc, req); >>> goto out; >>> } >>> @@ -2331,7 +2331,7 @@ int ceph_mdsc_do_request(struct ceph_mds_client *mdsc, >>> */ >>> mutex_lock(&req->r_fill_mutex); >>> req->r_err = err; >>> - req->r_aborted = true; >>> + set_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags); >>> mutex_unlock(&req->r_fill_mutex); >>> >>> if (req->r_locked_dir && >>> @@ -2538,7 +2538,7 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg) >>> } >>> out_err: >>> mutex_lock(&mdsc->mutex); >>> - if (!req->r_aborted) { >>> + if (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) { >>> if (err) { >>> req->r_err = err; >>> } else { >>> @@ -2587,7 +2587,7 @@ static void handle_forward(struct ceph_mds_client *mdsc, >>> goto out; /* dup reply? */ >>> } >>> >>> - if (req->r_aborted) { >>> + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) { >>> dout("forward tid %llu aborted, unregistering\n", tid); >>> __unregister_request(mdsc, req); >>> } else if (fwd_seq <= req->r_num_fwd) { >>> diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h >>> index a58cacccc986..3da20955d5e6 100644 >>> --- a/fs/ceph/mds_client.h >>> +++ b/fs/ceph/mds_client.h >>> @@ -206,6 +206,7 @@ struct ceph_mds_request { >>> struct inode *r_target_inode; /* resulting inode */ >>> >>> #define CEPH_MDS_R_DIRECT_IS_HASH (1) /* r_direct_hash is valid */ >>> +#define CEPH_MDS_R_ABORTED (2) /* call was aborted */ >>> unsigned long r_req_flags; >>> >>> struct mutex r_fill_mutex; >>> @@ -236,7 +237,6 @@ struct ceph_mds_request { >>> struct ceph_mds_reply_info_parsed r_reply_info; >>> struct page *r_locked_page; >>> int r_err; >>> - bool r_aborted; >>> >>> unsigned long r_timeout; /* optional. jiffies, 0 is "wait forever" */ >>> unsigned long r_started; /* start time to measure timeout against */ >>> -- >>> 2.9.3 >>> >> >> > > -- > Jeff Layton <jlayton@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2017-02-02 at 23:16 +0800, Yan, Zheng wrote: > > On 2 Feb 2017, at 19:26, Jeff Layton <jlayton@redhat.com> wrote: > > > > On Thu, 2017-02-02 at 16:06 +0800, Yan, Zheng wrote: > > > > On 1 Feb 2017, at 19:49, Jeff Layton <jlayton@redhat.com> wrote: > > > > > > > > ...and simplify the handling of it in ceph_fill_trace. > > > > > > > > Signed-off-by: Jeff Layton <jlayton@redhat.com> > > > > --- > > > > fs/ceph/inode.c | 17 +++++++++-------- > > > > fs/ceph/mds_client.c | 8 ++++---- > > > > fs/ceph/mds_client.h | 2 +- > > > > 3 files changed, 14 insertions(+), 13 deletions(-) > > > > > > > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c > > > > index 4926265f4223..bd2e94a78057 100644 > > > > --- a/fs/ceph/inode.c > > > > +++ b/fs/ceph/inode.c > > > > @@ -1233,8 +1233,8 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, > > > > > > > > err = fill_inode(in, req->r_locked_page, &rinfo->targeti, NULL, > > > > session, req->r_request_started, > > > > - (!req->r_aborted && rinfo->head->result == 0) ? > > > > - req->r_fmode : -1, > > > > + (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags) && > > > > + rinfo->head->result == 0) ? req->r_fmode : -1, > > > > &req->r_caps_reservation); > > > > if (err < 0) { > > > > pr_err("fill_inode badness %p %llx.%llx\n", > > > > @@ -1243,12 +1243,14 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, > > > > } > > > > } > > > > > > > > + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) > > > > + goto done; > > > > + > > > > > > This seems wrong. If we got the reply, we should update inode’s caps even request is aborted. > > > Otherwise, MDS can hang at revoking the dropped caps. > > > > > > > FWIW, I hit that exact problem in an earlier iteration of this patchset > > until I figured out what was going on. The caps do get updated with this > > patch. That gets done in fill_inode, and that's called even when the > > call was aborted. > > > > None of the "move the flag into r_req_flags" patches materially change > > the behavior of the code. In this patch, we're just moving both of the > > subsequent !req->r_aborted flag checks into this one spot. > > I think you are right. Thank you for explanation > > Yan, Zheng Well...this patch doesn't break the existing code, but I think we do still want to update the dentry lease, even when the call was cancelled. With this change that won't happen. That'll be fixed in the next posting. > > > > > > /* > > > > * ignore null lease/binding on snapdir ENOENT, or else we > > > > * will have trouble splicing in the virtual snapdir later > > > > */ > > > > - if (rinfo->head->is_dentry && !req->r_aborted && > > > > - req->r_locked_dir && > > > > + if (rinfo->head->is_dentry && req->r_locked_dir && > > > > (rinfo->head->is_target || strncmp(req->r_dentry->d_name.name, > > > > fsc->mount_options->snapdir_name, > > > > req->r_dentry->d_name.len))) { > > > > @@ -1351,9 +1353,8 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, > > > > update_dentry_lease(dn, rinfo->dlease, session, > > > > req->r_request_started); > > > > dout(" final dn %p\n", dn); > > > > - } else if (!req->r_aborted && > > > > - (req->r_op == CEPH_MDS_OP_LOOKUPSNAP || > > > > - req->r_op == CEPH_MDS_OP_MKSNAP)) { > > > > + } else if (req->r_op == CEPH_MDS_OP_LOOKUPSNAP || > > > > + req->r_op == CEPH_MDS_OP_MKSNAP) { > > > > struct dentry *dn = req->r_dentry; > > > > struct inode *dir = req->r_locked_dir; > > > > > > > > @@ -1478,7 +1479,7 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req, > > > > u32 fpos_offset; > > > > struct ceph_readdir_cache_control cache_ctl = {}; > > > > > > > > - if (req->r_aborted) > > > > + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) > > > > return readdir_prepopulate_inodes_only(req, session); > > > > > > > > if (rinfo->hash_order && req->r_path2) { > > > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > > > > index 1f2ef02832d9..a5156b6a0aed 100644 > > > > --- a/fs/ceph/mds_client.c > > > > +++ b/fs/ceph/mds_client.c > > > > @@ -2115,7 +2115,7 @@ static int __do_request(struct ceph_mds_client *mdsc, > > > > int err = 0; > > > > > > > > if (req->r_err || req->r_got_result) { > > > > - if (req->r_aborted) > > > > + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) > > > > __unregister_request(mdsc, req); > > > > goto out; > > > > } > > > > @@ -2331,7 +2331,7 @@ int ceph_mdsc_do_request(struct ceph_mds_client *mdsc, > > > > */ > > > > mutex_lock(&req->r_fill_mutex); > > > > req->r_err = err; > > > > - req->r_aborted = true; > > > > + set_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags); > > > > mutex_unlock(&req->r_fill_mutex); > > > > > > > > if (req->r_locked_dir && > > > > @@ -2538,7 +2538,7 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg) > > > > } > > > > out_err: > > > > mutex_lock(&mdsc->mutex); > > > > - if (!req->r_aborted) { > > > > + if (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) { > > > > if (err) { > > > > req->r_err = err; > > > > } else { > > > > @@ -2587,7 +2587,7 @@ static void handle_forward(struct ceph_mds_client *mdsc, > > > > goto out; /* dup reply? */ > > > > } > > > > > > > > - if (req->r_aborted) { > > > > + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) { > > > > dout("forward tid %llu aborted, unregistering\n", tid); > > > > __unregister_request(mdsc, req); > > > > } else if (fwd_seq <= req->r_num_fwd) { > > > > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h > > > > index a58cacccc986..3da20955d5e6 100644 > > > > --- a/fs/ceph/mds_client.h > > > > +++ b/fs/ceph/mds_client.h > > > > @@ -206,6 +206,7 @@ struct ceph_mds_request { > > > > struct inode *r_target_inode; /* resulting inode */ > > > > > > > > #define CEPH_MDS_R_DIRECT_IS_HASH (1) /* r_direct_hash is valid */ > > > > +#define CEPH_MDS_R_ABORTED (2) /* call was aborted */ > > > > unsigned long r_req_flags; > > > > > > > > struct mutex r_fill_mutex; > > > > @@ -236,7 +237,6 @@ struct ceph_mds_request { > > > > struct ceph_mds_reply_info_parsed r_reply_info; > > > > struct page *r_locked_page; > > > > int r_err; > > > > - bool r_aborted; > > > > > > > > unsigned long r_timeout; /* optional. jiffies, 0 is "wait forever" */ > > > > unsigned long r_started; /* start time to measure timeout against */ > > > > -- > > > > 2.9.3 > > > > > > > > > > > > > > -- > > Jeff Layton <jlayton@redhat.com> > >
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c index 4926265f4223..bd2e94a78057 100644 --- a/fs/ceph/inode.c +++ b/fs/ceph/inode.c @@ -1233,8 +1233,8 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, err = fill_inode(in, req->r_locked_page, &rinfo->targeti, NULL, session, req->r_request_started, - (!req->r_aborted && rinfo->head->result == 0) ? - req->r_fmode : -1, + (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags) && + rinfo->head->result == 0) ? req->r_fmode : -1, &req->r_caps_reservation); if (err < 0) { pr_err("fill_inode badness %p %llx.%llx\n", @@ -1243,12 +1243,14 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, } } + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) + goto done; + /* * ignore null lease/binding on snapdir ENOENT, or else we * will have trouble splicing in the virtual snapdir later */ - if (rinfo->head->is_dentry && !req->r_aborted && - req->r_locked_dir && + if (rinfo->head->is_dentry && req->r_locked_dir && (rinfo->head->is_target || strncmp(req->r_dentry->d_name.name, fsc->mount_options->snapdir_name, req->r_dentry->d_name.len))) { @@ -1351,9 +1353,8 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req, update_dentry_lease(dn, rinfo->dlease, session, req->r_request_started); dout(" final dn %p\n", dn); - } else if (!req->r_aborted && - (req->r_op == CEPH_MDS_OP_LOOKUPSNAP || - req->r_op == CEPH_MDS_OP_MKSNAP)) { + } else if (req->r_op == CEPH_MDS_OP_LOOKUPSNAP || + req->r_op == CEPH_MDS_OP_MKSNAP) { struct dentry *dn = req->r_dentry; struct inode *dir = req->r_locked_dir; @@ -1478,7 +1479,7 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req, u32 fpos_offset; struct ceph_readdir_cache_control cache_ctl = {}; - if (req->r_aborted) + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) return readdir_prepopulate_inodes_only(req, session); if (rinfo->hash_order && req->r_path2) { diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c index 1f2ef02832d9..a5156b6a0aed 100644 --- a/fs/ceph/mds_client.c +++ b/fs/ceph/mds_client.c @@ -2115,7 +2115,7 @@ static int __do_request(struct ceph_mds_client *mdsc, int err = 0; if (req->r_err || req->r_got_result) { - if (req->r_aborted) + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) __unregister_request(mdsc, req); goto out; } @@ -2331,7 +2331,7 @@ int ceph_mdsc_do_request(struct ceph_mds_client *mdsc, */ mutex_lock(&req->r_fill_mutex); req->r_err = err; - req->r_aborted = true; + set_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags); mutex_unlock(&req->r_fill_mutex); if (req->r_locked_dir && @@ -2538,7 +2538,7 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg) } out_err: mutex_lock(&mdsc->mutex); - if (!req->r_aborted) { + if (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) { if (err) { req->r_err = err; } else { @@ -2587,7 +2587,7 @@ static void handle_forward(struct ceph_mds_client *mdsc, goto out; /* dup reply? */ } - if (req->r_aborted) { + if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) { dout("forward tid %llu aborted, unregistering\n", tid); __unregister_request(mdsc, req); } else if (fwd_seq <= req->r_num_fwd) { diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h index a58cacccc986..3da20955d5e6 100644 --- a/fs/ceph/mds_client.h +++ b/fs/ceph/mds_client.h @@ -206,6 +206,7 @@ struct ceph_mds_request { struct inode *r_target_inode; /* resulting inode */ #define CEPH_MDS_R_DIRECT_IS_HASH (1) /* r_direct_hash is valid */ +#define CEPH_MDS_R_ABORTED (2) /* call was aborted */ unsigned long r_req_flags; struct mutex r_fill_mutex; @@ -236,7 +237,6 @@ struct ceph_mds_request { struct ceph_mds_reply_info_parsed r_reply_info; struct page *r_locked_page; int r_err; - bool r_aborted; unsigned long r_timeout; /* optional. jiffies, 0 is "wait forever" */ unsigned long r_started; /* start time to measure timeout against */
...and simplify the handling of it in ceph_fill_trace. Signed-off-by: Jeff Layton <jlayton@redhat.com> --- fs/ceph/inode.c | 17 +++++++++-------- fs/ceph/mds_client.c | 8 ++++---- fs/ceph/mds_client.h | 2 +- 3 files changed, 14 insertions(+), 13 deletions(-)