From: Hannes Reinecke <hare@suse.de>
To: Andrew Morton <akpm@osdl.org>
Cc: linux-hotplug-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Limit number of concurrent hotplug processes
Date: Tue, 27 Jul 2004 09:05:04 +0000 [thread overview]
Message-ID: <41061AC0.8000607@suse.de> (raw)
In-Reply-To: <20040727013427.52d3e5f5.akpm@osdl.org>
Andrew Morton wrote:
> Hannes Reinecke <hare@suse.de> wrote:
>
>> Patch (for the semaphore version) is attached.
>
>
> err, what on earth is this patch trying to do? It adds tons more
> complexity then I expected to see. Are the async (wait=0) semantics for
> call_usermodehelper() preserved?
>
Problem with your patch is that call_usermodehelper might block on
down() regardless whether it is called async or sync.
So any write to sysfs which triggers a hotplug event might block until
enough resources are available.
Most complexity is in fact due to the possibility to change khelper_max
on the fly. If we disallow that everything else will be far cleaner.
> Why is the code now doing
>
> if (stored_info.wait > 0) {
> and
> if (stored_info.wait >= 0) {
>
> ? `wait' is a boolean. Or did its semantics get secretly changed somewhere?
>
> Why is a new kernel thread needed to up and down a semaphore?
>
As I said; down() might block. Unless we accept that the caller will
only return after all down()s have been executed successfully we need
something like that.
> Sorry, but I've completely lost the plot on what you're trying to do here!
>
Sorry for this. I've probably pushed too hard for this.
I'll wrap up a patch which only allows for a static setting (via kernel
command line parameters) and leave the on-the-fly setting for later :-).
>
> I'd have though that something like the below (untested, slightly hacky)
> patch would suit.
>
Indeed, but only if we accept that any call to call_usermodehelper might
block if not enough resources are available.
THX for the patch, btw.
Cheers,
Hannes
--
Dr. Hannes Reinecke hare@suse.de
SuSE Linux AG S390 & zSeries
Maxfeldstraße 5 +49 911 74053 688
90409 Nürnberg http://www.suse.de
-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_idG21&alloc_id\x10040&opÌk
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
WARNING: multiple messages have this Message-ID (diff)
From: Hannes Reinecke <hare@suse.de>
To: Andrew Morton <akpm@osdl.org>
Cc: linux-hotplug-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Limit number of concurrent hotplug processes
Date: Tue, 27 Jul 2004 11:05:04 +0200 [thread overview]
Message-ID: <41061AC0.8000607@suse.de> (raw)
In-Reply-To: <20040727013427.52d3e5f5.akpm@osdl.org>
Andrew Morton wrote:
> Hannes Reinecke <hare@suse.de> wrote:
>
>> Patch (for the semaphore version) is attached.
>
>
> err, what on earth is this patch trying to do? It adds tons more
> complexity then I expected to see. Are the async (wait=0) semantics for
> call_usermodehelper() preserved?
>
Problem with your patch is that call_usermodehelper might block on
down() regardless whether it is called async or sync.
So any write to sysfs which triggers a hotplug event might block until
enough resources are available.
Most complexity is in fact due to the possibility to change khelper_max
on the fly. If we disallow that everything else will be far cleaner.
> Why is the code now doing
>
> if (stored_info.wait > 0) {
> and
> if (stored_info.wait >= 0) {
>
> ? `wait' is a boolean. Or did its semantics get secretly changed somewhere?
>
> Why is a new kernel thread needed to up and down a semaphore?
>
As I said; down() might block. Unless we accept that the caller will
only return after all down()s have been executed successfully we need
something like that.
> Sorry, but I've completely lost the plot on what you're trying to do here!
>
Sorry for this. I've probably pushed too hard for this.
I'll wrap up a patch which only allows for a static setting (via kernel
command line parameters) and leave the on-the-fly setting for later :-).
>
> I'd have though that something like the below (untested, slightly hacky)
> patch would suit.
>
Indeed, but only if we accept that any call to call_usermodehelper might
block if not enough resources are available.
THX for the patch, btw.
Cheers,
Hannes
--
Dr. Hannes Reinecke hare@suse.de
SuSE Linux AG S390 & zSeries
Maxfeldstraße 5 +49 911 74053 688
90409 Nürnberg http://www.suse.de
next prev parent reply other threads:[~2004-07-27 9:05 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-07-20 13:52 [PATCH] Limit number of concurrent hotplug processes Hannes Reinecke
2004-07-20 13:52 ` Hannes Reinecke
2004-07-26 1:20 ` Andrew Morton
2004-07-26 1:20 ` Andrew Morton
2004-07-26 6:19 ` Christian Borntraeger
2004-07-26 6:19 ` Christian Borntraeger
2004-07-26 10:59 ` Hannes Reinecke
2004-07-26 10:59 ` Hannes Reinecke
2004-07-26 20:18 ` Andrew Morton
2004-07-26 20:18 ` Andrew Morton
2004-07-27 7:04 ` Hannes Reinecke
2004-07-27 7:04 ` Hannes Reinecke
2004-07-27 7:24 ` Andrew Morton
2004-07-27 7:24 ` Andrew Morton
2004-07-27 7:59 ` Hannes Reinecke
2004-07-27 7:59 ` Hannes Reinecke
2004-07-27 8:34 ` Andrew Morton
2004-07-27 8:34 ` Andrew Morton
2004-07-27 9:05 ` Hannes Reinecke [this message]
2004-07-27 9:05 ` Hannes Reinecke
2004-07-27 9:28 ` Andrew Morton
2004-07-27 9:28 ` Andrew Morton
2004-07-27 12:22 ` Hannes Reinecke
2004-07-27 12:22 ` Hannes Reinecke
2004-07-28 7:12 ` Paul Jackson
2004-07-28 7:12 ` Paul Jackson
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=41061AC0.8000607@suse.de \
--to=hare@suse.de \
--cc=akpm@osdl.org \
--cc=linux-hotplug-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.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.