Re: [PATCH] Limit number of concurrent hotplug processes

[Hannes Reinecke]

Andrew Morton wrote:
> Hannes Reinecke <hare@xxxxxxx> 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@xxxxxxx
SuSE Linux AG				S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/