On Tue, Jul 10, 2018 at 5:20 AM Luis Henriques <lhenriques@xxxxxxxx> wrote: > > Chengguang Xu <cgxu519@xxxxxxx> writes: > > > When file num exceeds quota limit or fails from ceph_per_init_acls() > > should call d_drop to drop dentry from cache as well. > > > > Signed-off-by: Chengguang Xu <cgxu519@xxxxxxx> > > --- > > Hello, > > > > Sorry, I haven't got resource to fully test this patch, so please review. > > I think this patch isn't correct. In fact, function ceph_mknod seems to > be already buggy as it should only call d_drop if dget is also executed > (which isn't true if ceph_mdsc_create_request or kstrdup fails). > why? I think caller (vfs) of ceph_mknod already holds a reference. > Patch #2 has the same issue, but ceph_symlink does look correct > regarding dget/d_drop. > > Cheers, > -- > Luis > > > > > > fs/ceph/dir.c | 18 +++++++++++------- > > 1 file changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c > > index 036ac0f3a393..813c01e8ad05 100644 > > --- a/fs/ceph/dir.c > > +++ b/fs/ceph/dir.c > > @@ -827,19 +827,21 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry, > > if (ceph_snap(dir) != CEPH_NOSNAP) > > return -EROFS; > > > > - if (ceph_quota_is_max_files_exceeded(dir)) > > - return -EDQUOT; > > + if (ceph_quota_is_max_files_exceeded(dir)) { > > + err = -EDQUOT; > > + goto out; > > + } > > > > err = ceph_pre_init_acls(dir, &mode, &acls); > > if (err < 0) > > - return err; > > + goto out; > > > > dout("mknod in dir %p dentry %p mode 0%ho rdev %d\n", > > dir, dentry, mode, rdev); > > req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_MKNOD, USE_AUTH_MDS); > > if (IS_ERR(req)) { > > err = PTR_ERR(req); > > - goto out; > > + goto out2; > > } > > req->r_dentry = dget(dentry); > > req->r_num_caps = 2; > > @@ -857,12 +859,14 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry, > > if (!err && !req->r_reply_info.head->is_dentry) > > err = ceph_handle_notrace_create(dir, dentry); > > ceph_mdsc_put_request(req); > > -out: > > + > > if (!err) > > ceph_init_inode_acls(d_inode(dentry), &acls); > > - else > > - d_drop(dentry); > > +out2: > > ceph_release_acls_info(&acls); > > +out: > > + if (err) > > + d_drop(dentry); > > return err; > > } > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html