[PATCH] ceph: wait for async create reply before sending any cap messages

[PATCH] ceph: wait for async create reply before sending any cap messages

[Jeff Layton]

If we haven't received a reply to an async create request, then we don't
want to send any cap messages to the MDS for that inode yet.

Just have ceph_check_caps  and __kick_flushing_caps return without doing
anything, and have ceph_write_inode wait for the reply if we were asked
to wait on the inode writeback.

URL: https://tracker.ceph.com/issues/54107
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/caps.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index e668cdb9c99e..f29e2dbcf8df 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -1916,6 +1916,13 @@ void ceph_check_caps(struct ceph_inode_info *ci, int flags,
 		ceph_get_mds_session(session);
 
 	spin_lock(&ci->i_ceph_lock);
+	if (ci->i_ceph_flags & CEPH_I_ASYNC_CREATE) {
+		/* Don't send messages until we get async create reply */
+		spin_unlock(&ci->i_ceph_lock);
+		ceph_put_mds_session(session);
+		return;
+	}
+
 	if (ci->i_ceph_flags & CEPH_I_FLUSH)
 		flags |= CHECK_CAPS_FLUSH;
 retry:
@@ -2410,6 +2417,9 @@ int ceph_write_inode(struct inode *inode, struct writeback_control *wbc)
 	dout("write_inode %p wait=%d\n", inode, wait);
 	ceph_fscache_unpin_writeback(inode, wbc);
 	if (wait) {
+		err = ceph_wait_on_async_create(inode);
+		if (err)
+			return err;
 		dirty = try_flush_caps(inode, &flush_tid);
 		if (dirty)
 			err = wait_event_interruptible(ci->i_cap_wq,
@@ -2440,6 +2450,10 @@ static void __kick_flushing_caps(struct ceph_mds_client *mdsc,
 	u64 first_tid = 0;
 	u64 last_snap_flush = 0;
 
+	/* Don't do anything until create reply comes in */
+	if (ci->i_ceph_flags & CEPH_I_ASYNC_CREATE)
+		return;
+
 	ci->i_ceph_flags &= ~CEPH_I_KICK_FLUSH;
 
 	list_for_each_entry_reverse(cf, &ci->i_cap_flush_list, i_list) {
-- 
2.34.1

Re: [PATCH] ceph: wait for async create reply before sending any cap messages

[Patrick Donnelly]

Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>

-- 
Patrick Donnelly, Ph.D.
He / Him / His
Principal Software Engineer
Red Hat, Inc.
GPG: 19F28A586F808C2402351B93C3301A3E258DD79D

Re: [PATCH] ceph: wait for async create reply before sending any cap messages

[Xiubo Li]

On 2/5/22 11:17 PM, Jeff Layton wrote:
> If we haven't received a reply to an async create request, then we don't
> want to send any cap messages to the MDS for that inode yet.
>
> Just have ceph_check_caps  and __kick_flushing_caps return without doing
> anything, and have ceph_write_inode wait for the reply if we were asked
> to wait on the inode writeback.
>
> URL: https://tracker.ceph.com/issues/54107
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>   fs/ceph/caps.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index e668cdb9c99e..f29e2dbcf8df 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -1916,6 +1916,13 @@ void ceph_check_caps(struct ceph_inode_info *ci, int flags,
>   		ceph_get_mds_session(session);
>   
>   	spin_lock(&ci->i_ceph_lock);
> +	if (ci->i_ceph_flags & CEPH_I_ASYNC_CREATE) {
> +		/* Don't send messages until we get async create reply */
> +		spin_unlock(&ci->i_ceph_lock);
> +		ceph_put_mds_session(session);
> +		return;
> +	}
> +
>   	if (ci->i_ceph_flags & CEPH_I_FLUSH)
>   		flags |= CHECK_CAPS_FLUSH;
>   retry:
> @@ -2410,6 +2417,9 @@ int ceph_write_inode(struct inode *inode, struct writeback_control *wbc)
>   	dout("write_inode %p wait=%d\n", inode, wait);
>   	ceph_fscache_unpin_writeback(inode, wbc);
>   	if (wait) {
> +		err = ceph_wait_on_async_create(inode);
> +		if (err)
> +			return err;
>   		dirty = try_flush_caps(inode, &flush_tid);
>   		if (dirty)
>   			err = wait_event_interruptible(ci->i_cap_wq,
> @@ -2440,6 +2450,10 @@ static void __kick_flushing_caps(struct ceph_mds_client *mdsc,
>   	u64 first_tid = 0;
>   	u64 last_snap_flush = 0;
>   
> +	/* Don't do anything until create reply comes in */
> +	if (ci->i_ceph_flags & CEPH_I_ASYNC_CREATE)
> +		return;
> +
>   	ci->i_ceph_flags &= ~CEPH_I_KICK_FLUSH;
>   
>   	list_for_each_entry_reverse(cf, &ci->i_cap_flush_list, i_list) {

Is it also possible in case that just after the async unlinking request 
is submit and a flush cap request is fired ? Then in MDS side the inode 
could be removed from the cache and then the flush cap request comes.

Re: [PATCH] ceph: wait for async create reply before sending any cap messages

[Jeff Layton]

On Mon, 2022-02-07 at 14:54 +0800, Xiubo Li wrote:
> On 2/5/22 11:17 PM, Jeff Layton wrote:
> > If we haven't received a reply to an async create request, then we don't
> > want to send any cap messages to the MDS for that inode yet.
> > 
> > Just have ceph_check_caps  and __kick_flushing_caps return without doing
> > anything, and have ceph_write_inode wait for the reply if we were asked
> > to wait on the inode writeback.
> > 
> > URL: https://tracker.ceph.com/issues/54107
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >   fs/ceph/caps.c | 14 ++++++++++++++
> >   1 file changed, 14 insertions(+)
> > 
> > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> > index e668cdb9c99e..f29e2dbcf8df 100644
> > --- a/fs/ceph/caps.c
> > +++ b/fs/ceph/caps.c
> > @@ -1916,6 +1916,13 @@ void ceph_check_caps(struct ceph_inode_info *ci, int flags,
> >   		ceph_get_mds_session(session);
> >   
> >   	spin_lock(&ci->i_ceph_lock);
> > +	if (ci->i_ceph_flags & CEPH_I_ASYNC_CREATE) {
> > +		/* Don't send messages until we get async create reply */
> > +		spin_unlock(&ci->i_ceph_lock);
> > +		ceph_put_mds_session(session);
> > +		return;
> > +	}
> > +
> >   	if (ci->i_ceph_flags & CEPH_I_FLUSH)
> >   		flags |= CHECK_CAPS_FLUSH;
> >   retry:
> > @@ -2410,6 +2417,9 @@ int ceph_write_inode(struct inode *inode, struct writeback_control *wbc)
> >   	dout("write_inode %p wait=%d\n", inode, wait);
> >   	ceph_fscache_unpin_writeback(inode, wbc);
> >   	if (wait) {
> > +		err = ceph_wait_on_async_create(inode);
> > +		if (err)
> > +			return err;
> >   		dirty = try_flush_caps(inode, &flush_tid);
> >   		if (dirty)
> >   			err = wait_event_interruptible(ci->i_cap_wq,
> > @@ -2440,6 +2450,10 @@ static void __kick_flushing_caps(struct ceph_mds_client *mdsc,
> >   	u64 first_tid = 0;
> >   	u64 last_snap_flush = 0;
> >   
> > +	/* Don't do anything until create reply comes in */
> > +	if (ci->i_ceph_flags & CEPH_I_ASYNC_CREATE)
> > +		return;
> > +
> >   	ci->i_ceph_flags &= ~CEPH_I_KICK_FLUSH;
> >   
> >   	list_for_each_entry_reverse(cf, &ci->i_cap_flush_list, i_list) {
> 
> Is it also possible in case that just after the async unlinking request 
> is submit and a flush cap request is fired ? Then in MDS side the inode 
> could be removed from the cache and then the flush cap request comes.
> 
> 
> 

Yes. I think that race should be fairly benign though. The MDS might
drop the update onto the floor when it can't find the inode, but since
the inode is already stale, I don't think we really care...

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH] ceph: wait for async create reply before sending any cap messages

[Xiubo Li]

On 2/7/22 9:08 PM, Jeff Layton wrote:
> On Mon, 2022-02-07 at 14:54 +0800, Xiubo Li wrote:
>> On 2/5/22 11:17 PM, Jeff Layton wrote:
>>> If we haven't received a reply to an async create request, then we don't
>>> want to send any cap messages to the MDS for that inode yet.
>>>
>>> Just have ceph_check_caps  and __kick_flushing_caps return without doing
>>> anything, and have ceph_write_inode wait for the reply if we were asked
>>> to wait on the inode writeback.
>>>
>>> URL: https://tracker.ceph.com/issues/54107
>>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>>> ---
>>>    fs/ceph/caps.c | 14 ++++++++++++++
>>>    1 file changed, 14 insertions(+)
>>>
>>> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
>>> index e668cdb9c99e..f29e2dbcf8df 100644
>>> --- a/fs/ceph/caps.c
>>> +++ b/fs/ceph/caps.c
>>> @@ -1916,6 +1916,13 @@ void ceph_check_caps(struct ceph_inode_info *ci, int flags,
>>>    		ceph_get_mds_session(session);
>>>    
>>>    	spin_lock(&ci->i_ceph_lock);
>>> +	if (ci->i_ceph_flags & CEPH_I_ASYNC_CREATE) {
>>> +		/* Don't send messages until we get async create reply */
>>> +		spin_unlock(&ci->i_ceph_lock);
>>> +		ceph_put_mds_session(session);
>>> +		return;
>>> +	}
>>> +
>>>    	if (ci->i_ceph_flags & CEPH_I_FLUSH)
>>>    		flags |= CHECK_CAPS_FLUSH;
>>>    retry:
>>> @@ -2410,6 +2417,9 @@ int ceph_write_inode(struct inode *inode, struct writeback_control *wbc)
>>>    	dout("write_inode %p wait=%d\n", inode, wait);
>>>    	ceph_fscache_unpin_writeback(inode, wbc);
>>>    	if (wait) {
>>> +		err = ceph_wait_on_async_create(inode);
>>> +		if (err)
>>> +			return err;
>>>    		dirty = try_flush_caps(inode, &flush_tid);
>>>    		if (dirty)
>>>    			err = wait_event_interruptible(ci->i_cap_wq,
>>> @@ -2440,6 +2450,10 @@ static void __kick_flushing_caps(struct ceph_mds_client *mdsc,
>>>    	u64 first_tid = 0;
>>>    	u64 last_snap_flush = 0;
>>>    
>>> +	/* Don't do anything until create reply comes in */
>>> +	if (ci->i_ceph_flags & CEPH_I_ASYNC_CREATE)
>>> +		return;
>>> +
>>>    	ci->i_ceph_flags &= ~CEPH_I_KICK_FLUSH;
>>>    
>>>    	list_for_each_entry_reverse(cf, &ci->i_cap_flush_list, i_list) {
>> Is it also possible in case that just after the async unlinking request
>> is submit and a flush cap request is fired ? Then in MDS side the inode
>> could be removed from the cache and then the flush cap request comes.
>>
>>
>>
> Yes. I think that race should be fairly benign though. The MDS might
> drop the update onto the floor when it can't find the inode, but since
> the inode is already stale, I don't think we really care...

Yeah, I also didn't see this will be a problem from the kclient code for 
now.

LGTM.

Reviewed-by: Xiubo Li <xiubli@redhat.com>

>