From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752580AbcLFWTB (ORCPT ); Tue, 6 Dec 2016 17:19:01 -0500 Received: from cloudserver094114.home.net.pl ([79.96.170.134]:46679 "EHLO cloudserver094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752363AbcLFWS7 (ORCPT ); Tue, 6 Dec 2016 17:18:59 -0500 From: "Rafael J. Wysocki" To: cuilifei Cc: linux-kernel@vger.kernel.org, miklos@szeredi.hu, pavel@ucw.cz, len.brown@intel.com, linux-fsdevel@vger.kernel.org, viro@zeniv.linux.org.uk, piaoyingmin@xiaomi.com, liucai@xiaomi.com, Jiri Kosina Subject: Re: [PATCH] fuse: freezing abort when use wait_event_killable{,_exclusive}(). Date: Tue, 06 Dec 2016 23:15:19 +0100 Message-ID: <2611375.NpU9fySFZX@aspire.rjw.lan> User-Agent: KMail/4.14.10 (Linux/4.9.0-rc5+; KDE/4.14.9; x86_64; ; ) In-Reply-To: <1480907985-1494-1-git-send-email-cuilifei@xiaomi.com> References: <1480907985-1494-1-git-send-email-cuilifei@xiaomi.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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 > #include > #include > +#include > > 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