On Monday, December 05, 2016 11:19:45 AM cuilifei wrote:
> Freezing process can abort when a client is waiting uninterruptibly
> for a response. Add new macro wait_fatal_freezable to try to fix it.
> 
> Signed-off-by: cuilifei <cuili...@xiaomi.com>

I'm not a big fan of this to be honest.

Do we really want to suspend the system (or freeze tasks for other reasons)
if that happens?

> ---
>  fs/fuse/dev.c           | 30 ++++++++++++++++++++++++++----
>  include/linux/freezer.h | 26 ++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 70ea57c..e33a081 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -19,6 +19,7 @@
>  #include <linux/pipe_fs_i.h>
>  #include <linux/swap.h>
>  #include <linux/splice.h>
> +#include <linux/freezer.h>
>  
>  MODULE_ALIAS_MISCDEV(FUSE_MINOR);
>  MODULE_ALIAS("devname:fuse");
> @@ -99,6 +100,19 @@ void fuse_request_free(struct fuse_req *req)
>       kmem_cache_free(fuse_req_cachep, req);
>  }
>  
> +static void block_sigs(sigset_t *oldset)
> +{
> +     sigset_t mask;
> +
> +     siginitsetinv(&mask, sigmask(SIGKILL));
> +     sigprocmask(SIG_BLOCK, &mask, oldset);
> +}
> +
> +static void restore_sigs(sigset_t *oldset)
> +{
> +     sigprocmask(SIG_SETMASK, oldset, NULL);
> +}
> +
>  void __fuse_get_request(struct fuse_req *req)
>  {
>       atomic_inc(&req->count);
> @@ -134,13 +148,18 @@ static struct fuse_req *__fuse_get_req(struct fuse_conn 
> *fc, unsigned npages,
>                                      bool for_background)
>  {
>       struct fuse_req *req;
> +     sigset_t oldset;
> +     int intr;
>       int err;
>       atomic_inc(&fc->num_waiting);
>  
>       if (fuse_block_alloc(fc, for_background)) {
>               err = -EINTR;
> -             if (wait_event_killable_exclusive(fc->blocked_waitq,
> -                             !fuse_block_alloc(fc, for_background)))
> +             block_sigs(&oldset);
> +             intr = wait_fatal_freezable(fc->blocked_waitq,
> +                             !fuse_block_alloc(fc, for_background), true);
> +             restore_sigs(&oldset);
> +             if (intr)
>                       goto out;
>       }
>       /* Matches smp_wmb() in fuse_set_initialized() */
> @@ -427,9 +446,12 @@ static void request_wait_answer(struct fuse_conn *fc, 
> struct fuse_req *req)
>       }
>  
>       if (!test_bit(FR_FORCE, &req->flags)) {
> +             sigset_t oldset;
>               /* Only fatal signals may interrupt this */
> -             err = wait_event_killable(req->waitq,
> -                                     test_bit(FR_FINISHED, &req->flags));
> +             block_sigs(&oldset);
> +             err = wait_fatal_freezable(req->waitq,
> +                             test_bit(FR_FINISHED, &req->flags), false);
> +             restore_sigs(&oldset);
>               if (!err)
>                       return;
>  
> diff --git a/include/linux/freezer.h b/include/linux/freezer.h
> index dd03e83..2504cd0 100644
> --- a/include/linux/freezer.h
> +++ b/include/linux/freezer.h
> @@ -256,6 +256,22 @@ static inline int 
> freezable_schedule_hrtimeout_range(ktime_t *expires,
>       __retval;                                                       \
>  })
>  
> +#define wait_fatal_freezable(wq, condition, exclusive)                       
> \
> +({                                                                   \
> +     int __ret = 0;                                                  \
> +     do {                                                            \
> +             if (exclusive)                                          \
> +                     __ret = wait_event_interruptible_exclusive(wq,  \
> +                                                     condition);     \
> +             else                                                    \
> +                     __ret = wait_event_interruptible(wq,            \
> +                                                     condition);     \
> +             if (!__ret || fatal_signal_pending(current))            \
> +                     break;                                          \
> +     } while (try_to_freeze());                                      \
> +     __ret;                                                          \
> +})
> +
>  #else /* !CONFIG_FREEZER */
>  static inline bool frozen(struct task_struct *p) { return false; }
>  static inline bool freezing(struct task_struct *p) { return false; }
> @@ -296,6 +312,16 @@ static inline void set_freezable(void) {}
>  #define wait_event_freezekillable_unsafe(wq, condition)                      
> \
>               wait_event_killable(wq, condition)
>  
> +#define wait_fatal_freezable(wq, condition, exclusive)                       
> \
> +({                                                                   \
> +     int __ret = 0;                                                  \
> +     if (exclusive)                                                  \
> +             __ret = wait_event_killable_exclusive(wq, condition);   \
> +     else                                                            \
> +             __ret = wait_event_killable(wq, condition);             \
> +     __ret;                                                          \
> +})
> +
>  #endif /* !CONFIG_FREEZER */
>  
>  #endif       /* FREEZER_H_INCLUDED */
> 

Thanks,
Rafael

Reply via email to