On Tue, Dec 17, 2019 at 9:53 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> Currently, we could have an open-by-handle (or NFS server) call
> into the filesystem and start working with an inode before it's
> properly filled out.
>
> Don't clear I_NEW until we have filled out the inode, and discard it
> properly if that fails. Note that we occasionally take an extra
> reference to the inode to ensure that we don't put the last reference in
> discard_new_inode, but rather leave it for ceph_async_iput.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/ceph/inode.c | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 5bdc1afc2bee..64634c5af403 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -55,11 +55,9 @@ struct inode *ceph_get_inode(struct super_block *sb, struct ceph_vino vino)
> inode = iget5_locked(sb, t, ceph_ino_compare, ceph_set_ino_cb, &vino);
> if (!inode)
> return ERR_PTR(-ENOMEM);
> - if (inode->i_state & I_NEW) {
> + if (inode->i_state & I_NEW)
> dout("get_inode created new inode %p %llx.%llx ino %llx\n",
> inode, ceph_vinop(inode), (u64)inode->i_ino);
> - unlock_new_inode(inode);
> - }
>
> dout("get_inode on %lu=%llx.%llx got %p\n", inode->i_ino, vino.ino,
> vino.snap, inode);
> @@ -88,6 +86,10 @@ struct inode *ceph_get_snapdir(struct inode *parent)
> inode->i_fop = &ceph_snapdir_fops;
> ci->i_snap_caps = CEPH_CAP_PIN; /* so we can open */
> ci->i_rbytes = 0;
> +
> + if (inode->i_state & I_NEW)
> + unlock_new_inode(inode);
> +
> return inode;
> }
>
> @@ -1301,7 +1303,6 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> err = PTR_ERR(in);
> goto done;
> }
> - req->r_target_inode = in;
>
> err = fill_inode(in, req->r_locked_page, &rinfo->targeti, NULL,
> session,
> @@ -1311,8 +1312,13 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> if (err < 0) {
> pr_err("fill_inode badness %p %llx.%llx\n",
> in, ceph_vinop(in));
> + if (in->i_state & I_NEW)
> + discard_new_inode(in);
> goto done;
> }
> + req->r_target_inode = in;
> + if (in->i_state & I_NEW)
> + unlock_new_inode(in);
I wonder if it's better to move these code into fill_inode()?
> }
>
> /*
> @@ -1496,7 +1502,14 @@ static int readdir_prepopulate_inodes_only(struct ceph_mds_request *req,
> if (rc < 0) {
> pr_err("fill_inode badness on %p got %d\n", in, rc);
> err = rc;
> + if (in->i_state & I_NEW) {
> + ihold(in);
> + discard_new_inode(in);
> + }
> + } else if (in->i_state & I_NEW) {
> + unlock_new_inode(in);
> }
> +
> /* avoid calling iput_final() in mds dispatch threads */
> ceph_async_iput(in);
> }
> @@ -1698,12 +1711,18 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req,
> if (d_really_is_negative(dn)) {
> /* avoid calling iput_final() in mds
> * dispatch threads */
> + if (in->i_state & I_NEW) {
> + ihold(in);
> + discard_new_inode(in);
> + }
> ceph_async_iput(in);
> }
> d_drop(dn);
> err = ret;
> goto next_item;
> }
> + if (in->i_state & I_NEW)
> + unlock_new_inode(in);
>
> if (d_really_is_negative(dn)) {
> if (ceph_security_xattr_deadlock(in)) {
> --
> 2.23.0
>
