All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Ming Lei <ming.lei@canonical.com>, Tejun Heo <tj@kernel.org>
Cc: Alex Riesen <raa.lkml@gmail.com>,
	Alan Stern <stern@rowland.harvard.edu>,
	Jens Axboe <axboe@kernel.dk>,
	USB list <linux-usb@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: USB device cannot be reconnected and khubd "blocked for more than 120 seconds"
Date: Tue, 15 Jan 2013 10:18:45 -0800	[thread overview]
Message-ID: <CA+55aFy_7mng3fgKdYM_h3c1=etKhX5JqEvFVE23X8kNYJQkpw@mail.gmail.com> (raw)
In-Reply-To: <CA+55aFyOyTS1ZCm6UWqGLteoPuFXKu0WSYB-aTeRYvJUYPqe5A@mail.gmail.com>

On Tue, Jan 15, 2013 at 9:36 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> This kind of "let's randomly encourage people to write subtly buggy
> code that has magical timing dependencies, so that the developer won't
> likely even see it because he has fast disks etc" code is totally
> unacceptable. And this code was *designed* to be that kind of buggy.

Btw, we could *possibly* do this the other way around. Wait for all
async work by default, but then have a really hacky way to turn that
off for modules that explicitly don't want it, because they know they
can be loaded in async context, and they don't do any async work
themselves. Then we could make the IO schedulers set that flag ("I
know I'm loaded from async space, and I know I'm not myself doing any
async init")

Quite frankly, I'd still much rather prefer the automated approach -
or even better, just avoiding the "load modules in async context"
entirely. But at least the "I can put a huge comment about why I don't
want to be waited on" would be much more acceptable than the "I need
to explicitly tell the world that it needs to wait on me".

So Ming Lei's patch was "easily subtly buggy by mistake" (showing that
by the fact that it was indeed buggy), while the opposite model where
you have to explicitly ask people not to wait for you could still be
very buggy, but at least now it needs to explicitly do extra work in
order to be buggy.

So if an interface is fragile, it should aim to be fragile in the
right way - making the fragility explicit, so that people can grep for
it, and people can add comments to the particular code that marks it
fragile. The default behavior should be the robust one.

And if would be lovely to add a warning to the "people loaded a module
from async context" case, so that we'd *see* this.

Tejun, is there a good way for code to see "I'm running in async
context"? Then we could do something like

    WARN_ON_ONCE(wait && system_state == SYSTEM_RUNNING && in_async_thread());

in kernel/kmod.c (__request_module()). That should at least warn about
this whole issue happening.

                    Linus

  reply	other threads:[~2013-01-15 18:19 UTC|newest]

Thread overview: 93+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-11 21:04 USB device cannot be reconnected and khubd "blocked for more than 120 seconds" Alex Riesen
2013-01-12  7:48 ` Alex Riesen
2013-01-12  9:18   ` Lan Tianyu
2013-01-12 17:37   ` Alan Stern
2013-01-12 19:39     ` Alex Riesen
2013-01-12 20:33       ` Alex Riesen
2013-01-12 22:52         ` Alan Stern
2013-01-13 12:09           ` Alex Riesen
2013-01-13 16:56             ` Alan Stern
2013-01-13 17:42               ` Alex Riesen
2013-01-13 19:16                 ` Oliver Neukum
2013-01-14  2:39                   ` Alan Stern
2013-01-14 16:43                     ` Alex Riesen
2013-01-14  3:47                 ` Ming Lei
2013-01-14  7:15                   ` Ming Lei
2013-01-14 17:30                     ` Linus Torvalds
2013-01-14 18:04                       ` Alan Stern
2013-01-14 18:34                         ` Linus Torvalds
2013-01-15  1:53                       ` Ming Lei
2013-01-15  6:23                         ` Ming Lei
2013-01-15 17:36                           ` Linus Torvalds
2013-01-15 18:18                             ` Linus Torvalds [this message]
2013-01-15 23:17                               ` Tejun Heo
2013-01-15 18:20                             ` Alan Stern
2013-01-15 18:39                               ` Tejun Heo
2013-01-15 18:32                             ` Tejun Heo
2013-01-15 20:18                               ` Linus Torvalds
2013-01-15 23:50                                 ` Tejun Heo
2013-01-16  0:25                                   ` Arjan van de Ven
2013-01-16  0:35                                     ` Tejun Heo
2013-01-16  4:01                                       ` Alan Stern
2013-01-16 16:12                                         ` Tejun Heo
2013-01-16 17:01                                           ` Alan Stern
2013-01-16 17:37                                             ` Tejun Heo
2013-01-16 17:51                                               ` Alan Stern
2013-01-16  0:36                                   ` Linus Torvalds
2013-01-16  0:40                                     ` Linus Torvalds
2013-01-16  2:52                                       ` [PATCH] module, async: async_synchronize_full() on module init iff async is used Tejun Heo
2013-01-16  3:00                                         ` Linus Torvalds
2013-01-16  3:25                                           ` Tejun Heo
2013-01-16  3:37                                             ` Linus Torvalds
2013-01-16 16:22                                               ` Arjan van de Ven
2013-01-16 16:48                                               ` Tejun Heo
2013-01-16 17:03                                                 ` Arjan van de Ven
2013-01-16 17:06                                                   ` Linus Torvalds
2013-01-16 21:30                                                     ` [PATCH 1/2] init, block: try to load default elevator module early during boot Tejun Heo
2013-01-17 18:05                                                       ` Linus Torvalds
2013-01-17 18:38                                                         ` Tejun Heo
2013-01-17 18:46                                                           ` Linus Torvalds
2013-01-17 18:59                                                             ` Tejun Heo
2013-01-17 19:00                                                               ` Linus Torvalds
2013-01-18  1:24                                                         ` [PATCH 1/3] workqueue: set PF_WQ_WORKER on rescuers Tejun Heo
2013-01-18  1:25                                                         ` [PATCH 2/3] workqueue, async: implement work/async_current_func() Tejun Heo
2013-01-18  2:47                                                           ` Linus Torvalds
2013-01-18  2:59                                                             ` Tejun Heo
2013-01-18  3:04                                                               ` Tejun Heo
2013-01-18  3:18                                                                 ` Linus Torvalds
2013-01-18  3:47                                                                   ` Tejun Heo
2013-01-18 22:08                                                                   ` [PATCH 1/5] workqueue: set PF_WQ_WORKER on rescuers Tejun Heo
2013-01-18 22:10                                                                   ` [PATCH 2/5] workqueue: rename kernel/workqueue_sched.h to kernel/workqueue_internal.h Tejun Heo
2013-01-18 22:11                                                                   ` [PATCH 3/5] workqueue: move struct worker definition to workqueue_internal.h Tejun Heo
2013-01-18 22:11                                                                   ` [PATCH 4/5] workqueue: implement current_is_async() Tejun Heo
2013-01-18 22:12                                                                   ` [PATCH 5/5] async, kmod: warn on synchronous request_module() from async workers Tejun Heo
2022-06-23  5:25                                                                     ` Saravana Kannan
2013-01-18  1:27                                                         ` [PATCH 3/3] " Tejun Heo
2013-01-23  0:53                                                       ` [PATCH v2 1/2] init, block: try to load default elevator module early during boot Tejun Heo
2013-01-16 21:31                                                     ` [PATCH 2/2] block: don't request module during elevator init Tejun Heo
2013-01-23  0:51                                                       ` [PATCH v2 " Tejun Heo
2013-01-16  3:30                                         ` [PATCH] module, async: async_synchronize_full() on module init iff async is used Ming Lei
2013-01-16  4:24                                         ` Rusty Russell
2013-01-16 11:36                                         ` Alex Riesen
2013-08-12  7:04                                         ` [3.8-rc3 -> 3.8-rc4 regression] " Jonathan Nieder
2013-08-12 15:09                                           ` Tejun Heo
2013-11-26 21:29                                             ` Josh Hunt
2013-11-26 21:53                                               ` Linus Torvalds
2013-11-26 22:12                                                 ` Josh Hunt
2013-11-26 22:29                                                   ` Tejun Heo
2013-12-03 14:28                                                     ` Josh Hunt
2013-12-03 15:19                                                       ` Tejun Heo
2013-12-04 23:01                                                         ` Josh Hunt
2013-12-04 23:12                                                           ` Tejun Heo
2013-11-26 22:30                                                   ` Linus Torvalds
2013-01-16  0:44                                     ` USB device cannot be reconnected and khubd "blocked for more than 120 seconds" Tejun Heo
2013-01-16 17:19                               ` [PATCH] async: fix __lowest_in_progress() Tejun Heo
2013-01-17 18:16                                 ` Linus Torvalds
2013-01-17 18:50                                   ` Tejun Heo
2013-01-23  0:15                                 ` [PATCH v2] " Tejun Heo
2013-01-23  0:22                                   ` Linus Torvalds
2013-01-16  3:05                             ` USB device cannot be reconnected and khubd "blocked for more than 120 seconds" Ming Lei
2013-01-16  4:14                               ` Linus Torvalds
2013-01-14  8:22                   ` Oliver Neukum
2013-01-14  8:40                     ` Ming Lei
2013-01-12 19:56     ` Alex Riesen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+55aFy_7mng3fgKdYM_h3c1=etKhX5JqEvFVE23X8kNYJQkpw@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=ming.lei@canonical.com \
    --cc=raa.lkml@gmail.com \
    --cc=stern@rowland.harvard.edu \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.