Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752163AbbEBMGb (ORCPT ); Sat, 2 May 2015 08:06:31 -0400 Received: from forward20m.cmail.yandex.net ([5.255.216.151]:35450 "EHLO forward20m.cmail.yandex.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751159AbbEBMG2 (ORCPT ); Sat, 2 May 2015 08:06:28 -0400 X-Greylist: delayed 578 seconds by postgrey-1.27 at vger.kernel.org; Sat, 02 May 2015 08:06:27 EDT From: Evgeniy Polyakov Envelope-From: drustafa@yandex.ru To: Ingo Molnar , Linus Torvalds , Neil Brown , Stephen Smalley Cc: Alex Williamson , Oleg Nesterov , linux-kernel , kvm In-Reply-To: <20150501193813.GA2812@gmail.com> References: <1430502057.4472.255.camel@redhat.com> <20150501193813.GA2812@gmail.com> Subject: Re: [PATCH] signals: Generate warning when flush_signals() is called from non-kthread context MIME-Version: 1.0 Message-Id: <8198211430567803@web26m.yandex.ru> X-Mailer: Yamail [ http://yandex.ru ] 5.0 Date: Sat, 02 May 2015 14:56:43 +0300 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=koi8-r Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2584 Lines: 58 Hi Ingo 01.05.2015, 22:38, "Ingo Molnar" : > * Linus Torvalds wrote: >> ?On Fri, May 1, 2015 at 10:40 AM, Alex Williamson >> ? wrote: >>> ?- Flush signals on interrupted wait to retain polling interval (Alex Williamson) >> ?This cannot *possibly* be right. If I read this patch right, you're >> ?randomly just getting rid of signals. No way in hell is that correct. >> >> ?"flush_signals()" is only for kernel threads, where it's a hacky >> ?alternative to actually handling them (since kernel threads never >> ?rreturn to user space and cannot really "handle" a signal). But you're >> ?doing it in the ->remove handler for the device, which can be called >> ?by arbitrary system processes. This is not a kernel thread thing, as >> ?far as I can see. >> >> ?If you cannot handle signals, you damn well shouldn't be using >> ?"wait_event_interruptible_timeout()" to begin with. Get rid of the >> ?"interruptible", since it apparently *isn't* interruptible. >> >> ?So I'm not pulling this. >> >> ?Now I'm worried that other drivers do insane things like this. I >> ?wonder if we should add some sanity test to flush_signals() to make >> ?sure that it can only ever get called from a kernel thread. > Looks unsafe: called from various module exit handlers in: > > ??drivers/w1/slaves/w1_bq27000.c > ??drivers/w1/slaves/w1_ds2406.c > ??drivers/w1/slaves/w1_ds2408.c > ??drivers/w1/slaves/w1_ds2413.c > ??drivers/w1/slaves/w1_ds2423.c > ??drivers/w1/slaves/w1_ds2431.c > ??drivers/w1/slaves/w1_ds2433.c > ??drivers/w1/slaves/w1_ds2760.c > ??drivers/w1/slaves/w1_ds2780.c > ??drivers/w1/slaves/w1_ds2781.c > ??drivers/w1/slaves/w1_ds28e04.c > ??drivers/w1/slaves/w1_smem.c > ??drivers/w1/slaves/w1_therm.c > > which would be executed in rmmod context, losing signals. > Cc:-ed Evgeniy. w1 uses a little bit strange refcnt logic, basically every w1 module waits for all its users to release their w1 resources and doesn't exit until its safe. To wait for resources to be freed at module exit time it checks its refcnt to drop to zero periodically and sleeps between the checks for a second. w1 flushes signals between these checks for no particular reason, it can be safely removed from w1_unregister_family() and interruptible sleep replaced with the normal one. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/