[1/3] fs/fuse kio: fix loss of FUSE_FSYNC/FUSE_FLUSH reqs

Submitted by Pavel Butsykin on Dec. 3, 2018, 1:02 p.m.

Details

Message ID 20181203130210.32011-2-pbutsykin@virtuozzo.com
State New
Series "Fuse KIO: fix fsync/flush loss"
Headers show

Commit Message

Pavel Butsykin Dec. 3, 2018, 1:02 p.m.
Fast-Path can loss fsync/flush requests, because PCS_REQ_T_SYNC request set up
PCS_MAP_FLUSHING flag, but never cleans it. Although initially PCS_MAP_FLUSHING
flag was created for sync timer, so PCS_REQ_T_SYNC request should ignore
everything related to PCS_REQ_T_SYNC flag.

This patch adds timer_sync flag in prepare_map_flush_ireq() to ignore all sync
timer stuff on PCS_REQ_T_SYNC way.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
---
 fs/fuse/kio/pcs/pcs_map.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

Patch hide | download patch | download mbox

diff --git a/fs/fuse/kio/pcs/pcs_map.c b/fs/fuse/kio/pcs/pcs_map.c
index 459645417462..4bd18c5224ab 100644
--- a/fs/fuse/kio/pcs/pcs_map.c
+++ b/fs/fuse/kio/pcs/pcs_map.c
@@ -2959,20 +2959,24 @@  static void prepare_map_flush_msg(struct pcs_map_entry * m, struct pcs_int_reque
 	msg->done = sync_done;
 }
 
-static bool valid_for_flush(struct pcs_map_entry *m)
+static bool valid_for_flush(struct pcs_map_entry *m, bool timer_sync)
 {
-	if (m->state & PCS_MAP_DEAD)
-		return false;
+	if (timer_sync) {
+		if (m->state & PCS_MAP_DEAD)
+			return false;
 
+		if (m->flags & PCS_MAP_FLUSHING)
+			return false;
+	}
 	if (!(m->flags & PCS_MAP_DIRTY))
 		return false;
-	if (m->flags & PCS_MAP_FLUSHING)
-		return false;
 
 	return true;
 }
 
-static int prepare_map_flush_ireq(struct pcs_map_entry *m, struct pcs_int_request **sreqp)
+static int prepare_map_flush_ireq(struct pcs_map_entry *m,
+				  struct pcs_int_request **sreqp,
+				  bool timer_sync)
 {
 	struct pcs_dentry_info *de;
 	struct pcs_cs_list *cslist;
@@ -2980,7 +2984,7 @@  static int prepare_map_flush_ireq(struct pcs_map_entry *m, struct pcs_int_reques
 	struct pcs_msg * msg;
 
 	spin_lock(&m->lock);
-	if (!valid_for_flush(m)) {
+	if (!valid_for_flush(m, timer_sync)) {
 		spin_unlock(&m->lock);
 		return 0;
 	}
@@ -3010,7 +3014,7 @@  static int prepare_map_flush_ireq(struct pcs_map_entry *m, struct pcs_int_reques
 	/* All resources allocated, we need to recheck maps state again */
 	spin_lock(&m->lock);
 	cslist_put(cslist);
-	if (!valid_for_flush(m) || m->cs_list != cslist) {
+	if (!valid_for_flush(m, timer_sync) || m->cs_list != cslist) {
 		spin_unlock(&m->lock);
 		return 0;
 	}
@@ -3025,7 +3029,8 @@  static int prepare_map_flush_ireq(struct pcs_map_entry *m, struct pcs_int_reques
 	sreq->complete_cb = pcs_flushreq_complete;
 	sreq->flushreq.msg = msg;
 	FUSE_KTRACE(sreq->cc->fc, "timed FLUSH " MAP_FMT, MAP_ARGS(m));
-	m->flags |= PCS_MAP_FLUSHING;
+	if (timer_sync)
+		m->flags |= PCS_MAP_FLUSHING;
 	__pcs_map_get(m);
 	spin_unlock(&m->lock);
 	*sreqp	= sreq;
@@ -3047,7 +3052,7 @@  static void sync_timer_work(struct work_struct *w)
 	struct pcs_int_request * sreq = NULL;
 	int err;
 
-	err = prepare_map_flush_ireq(m, &sreq);
+	err = prepare_map_flush_ireq(m, &sreq, true);
 	if (err) {
 		spin_lock(&m->lock);
 		if (!(m->state & PCS_MAP_DEAD))
@@ -3125,7 +3130,7 @@  void map_inject_flush_req(struct pcs_int_request *ireq)
 				break;
 			if (!maps[i])
 				continue;
-			err = prepare_map_flush_ireq(maps[i], &sreq);
+			err = prepare_map_flush_ireq(maps[i], &sreq, false);
 			pcs_map_put(maps[i]);
 			if (err) {
 				pcs_set_local_error(&ireq->error, PCS_ERR_NOMEM);

Comments

Pavel Butsykin Dec. 3, 2018, 1:04 p.m.
pls ignore this.

On 03.12.2018 16:02, Pavel Butsykin wrote:
> Fast-Path can loss fsync/flush requests, because PCS_REQ_T_SYNC request set up
> PCS_MAP_FLUSHING flag, but never cleans it. Although initially PCS_MAP_FLUSHING
> flag was created for sync timer, so PCS_REQ_T_SYNC request should ignore
> everything related to PCS_REQ_T_SYNC flag.
> 
> This patch adds timer_sync flag in prepare_map_flush_ireq() to ignore all sync
> timer stuff on PCS_REQ_T_SYNC way.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> ---
>   fs/fuse/kio/pcs/pcs_map.c | 27 ++++++++++++++++-----------
>   1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/fuse/kio/pcs/pcs_map.c b/fs/fuse/kio/pcs/pcs_map.c
> index 459645417462..4bd18c5224ab 100644
> --- a/fs/fuse/kio/pcs/pcs_map.c
> +++ b/fs/fuse/kio/pcs/pcs_map.c
> @@ -2959,20 +2959,24 @@ static void prepare_map_flush_msg(struct pcs_map_entry * m, struct pcs_int_reque
>   	msg->done = sync_done;
>   }
>   
> -static bool valid_for_flush(struct pcs_map_entry *m)
> +static bool valid_for_flush(struct pcs_map_entry *m, bool timer_sync)
>   {
> -	if (m->state & PCS_MAP_DEAD)
> -		return false;
> +	if (timer_sync) {
> +		if (m->state & PCS_MAP_DEAD)
> +			return false;
>   
> +		if (m->flags & PCS_MAP_FLUSHING)
> +			return false;
> +	}
>   	if (!(m->flags & PCS_MAP_DIRTY))
>   		return false;
> -	if (m->flags & PCS_MAP_FLUSHING)
> -		return false;
>   
>   	return true;
>   }
>   
> -static int prepare_map_flush_ireq(struct pcs_map_entry *m, struct pcs_int_request **sreqp)
> +static int prepare_map_flush_ireq(struct pcs_map_entry *m,
> +				  struct pcs_int_request **sreqp,
> +				  bool timer_sync)
>   {
>   	struct pcs_dentry_info *de;
>   	struct pcs_cs_list *cslist;
> @@ -2980,7 +2984,7 @@ static int prepare_map_flush_ireq(struct pcs_map_entry *m, struct pcs_int_reques
>   	struct pcs_msg * msg;
>   
>   	spin_lock(&m->lock);
> -	if (!valid_for_flush(m)) {
> +	if (!valid_for_flush(m, timer_sync)) {
>   		spin_unlock(&m->lock);
>   		return 0;
>   	}
> @@ -3010,7 +3014,7 @@ static int prepare_map_flush_ireq(struct pcs_map_entry *m, struct pcs_int_reques
>   	/* All resources allocated, we need to recheck maps state again */
>   	spin_lock(&m->lock);
>   	cslist_put(cslist);
> -	if (!valid_for_flush(m) || m->cs_list != cslist) {
> +	if (!valid_for_flush(m, timer_sync) || m->cs_list != cslist) {
>   		spin_unlock(&m->lock);
>   		return 0;
>   	}
> @@ -3025,7 +3029,8 @@ static int prepare_map_flush_ireq(struct pcs_map_entry *m, struct pcs_int_reques
>   	sreq->complete_cb = pcs_flushreq_complete;
>   	sreq->flushreq.msg = msg;
>   	FUSE_KTRACE(sreq->cc->fc, "timed FLUSH " MAP_FMT, MAP_ARGS(m));
> -	m->flags |= PCS_MAP_FLUSHING;
> +	if (timer_sync)
> +		m->flags |= PCS_MAP_FLUSHING;
>   	__pcs_map_get(m);
>   	spin_unlock(&m->lock);
>   	*sreqp	= sreq;
> @@ -3047,7 +3052,7 @@ static void sync_timer_work(struct work_struct *w)
>   	struct pcs_int_request * sreq = NULL;
>   	int err;
>   
> -	err = prepare_map_flush_ireq(m, &sreq);
> +	err = prepare_map_flush_ireq(m, &sreq, true);
>   	if (err) {
>   		spin_lock(&m->lock);
>   		if (!(m->state & PCS_MAP_DEAD))
> @@ -3125,7 +3130,7 @@ void map_inject_flush_req(struct pcs_int_request *ireq)
>   				break;
>   			if (!maps[i])
>   				continue;
> -			err = prepare_map_flush_ireq(maps[i], &sreq);
> +			err = prepare_map_flush_ireq(maps[i], &sreq, false);
>   			pcs_map_put(maps[i]);
>   			if (err) {
>   				pcs_set_local_error(&ireq->error, PCS_ERR_NOMEM);
>
Pavel Butsykin Dec. 3, 2018, 1:07 p.m.
sorry, Not there XD

pls, don't ignore this.

On 03.12.2018 16:04, Pavel Butsykin wrote:
> pls ignore this.
> 
> On 03.12.2018 16:02, Pavel Butsykin wrote:
>> Fast-Path can loss fsync/flush requests, because PCS_REQ_T_SYNC 
>> request set up
>> PCS_MAP_FLUSHING flag, but never cleans it. Although initially 
>> PCS_MAP_FLUSHING
>> flag was created for sync timer, so PCS_REQ_T_SYNC request should ignore
>> everything related to PCS_REQ_T_SYNC flag.
>>
>> This patch adds timer_sync flag in prepare_map_flush_ireq() to ignore 
>> all sync
>> timer stuff on PCS_REQ_T_SYNC way.
>>
>> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
>> ---
>>   fs/fuse/kio/pcs/pcs_map.c | 27 ++++++++++++++++-----------
>>   1 file changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/fuse/kio/pcs/pcs_map.c b/fs/fuse/kio/pcs/pcs_map.c
>> index 459645417462..4bd18c5224ab 100644
>> --- a/fs/fuse/kio/pcs/pcs_map.c
>> +++ b/fs/fuse/kio/pcs/pcs_map.c
>> @@ -2959,20 +2959,24 @@ static void prepare_map_flush_msg(struct 
>> pcs_map_entry * m, struct pcs_int_reque
>>       msg->done = sync_done;
>>   }
>> -static bool valid_for_flush(struct pcs_map_entry *m)
>> +static bool valid_for_flush(struct pcs_map_entry *m, bool timer_sync)
>>   {
>> -    if (m->state & PCS_MAP_DEAD)
>> -        return false;
>> +    if (timer_sync) {
>> +        if (m->state & PCS_MAP_DEAD)
>> +            return false;
>> +        if (m->flags & PCS_MAP_FLUSHING)
>> +            return false;
>> +    }
>>       if (!(m->flags & PCS_MAP_DIRTY))
>>           return false;
>> -    if (m->flags & PCS_MAP_FLUSHING)
>> -        return false;
>>       return true;
>>   }
>> -static int prepare_map_flush_ireq(struct pcs_map_entry *m, struct 
>> pcs_int_request **sreqp)
>> +static int prepare_map_flush_ireq(struct pcs_map_entry *m,
>> +                  struct pcs_int_request **sreqp,
>> +                  bool timer_sync)
>>   {
>>       struct pcs_dentry_info *de;
>>       struct pcs_cs_list *cslist;
>> @@ -2980,7 +2984,7 @@ static int prepare_map_flush_ireq(struct 
>> pcs_map_entry *m, struct pcs_int_reques
>>       struct pcs_msg * msg;
>>       spin_lock(&m->lock);
>> -    if (!valid_for_flush(m)) {
>> +    if (!valid_for_flush(m, timer_sync)) {
>>           spin_unlock(&m->lock);
>>           return 0;
>>       }
>> @@ -3010,7 +3014,7 @@ static int prepare_map_flush_ireq(struct 
>> pcs_map_entry *m, struct pcs_int_reques
>>       /* All resources allocated, we need to recheck maps state again */
>>       spin_lock(&m->lock);
>>       cslist_put(cslist);
>> -    if (!valid_for_flush(m) || m->cs_list != cslist) {
>> +    if (!valid_for_flush(m, timer_sync) || m->cs_list != cslist) {
>>           spin_unlock(&m->lock);
>>           return 0;
>>       }
>> @@ -3025,7 +3029,8 @@ static int prepare_map_flush_ireq(struct 
>> pcs_map_entry *m, struct pcs_int_reques
>>       sreq->complete_cb = pcs_flushreq_complete;
>>       sreq->flushreq.msg = msg;
>>       FUSE_KTRACE(sreq->cc->fc, "timed FLUSH " MAP_FMT, MAP_ARGS(m));
>> -    m->flags |= PCS_MAP_FLUSHING;
>> +    if (timer_sync)
>> +        m->flags |= PCS_MAP_FLUSHING;
>>       __pcs_map_get(m);
>>       spin_unlock(&m->lock);
>>       *sreqp    = sreq;
>> @@ -3047,7 +3052,7 @@ static void sync_timer_work(struct work_struct *w)
>>       struct pcs_int_request * sreq = NULL;
>>       int err;
>> -    err = prepare_map_flush_ireq(m, &sreq);
>> +    err = prepare_map_flush_ireq(m, &sreq, true);
>>       if (err) {
>>           spin_lock(&m->lock);
>>           if (!(m->state & PCS_MAP_DEAD))
>> @@ -3125,7 +3130,7 @@ void map_inject_flush_req(struct pcs_int_request 
>> *ireq)
>>                   break;
>>               if (!maps[i])
>>                   continue;
>> -            err = prepare_map_flush_ireq(maps[i], &sreq);
>> +            err = prepare_map_flush_ireq(maps[i], &sreq, false);
>>               pcs_map_put(maps[i]);
>>               if (err) {
>>                   pcs_set_local_error(&ireq->error, PCS_ERR_NOMEM);
>>