From: Boaz Harrosh <bharrosh@panasas.com>
To: Oleg Nesterov <oleg@redhat.com>,
Andrew Morton <akpm@linux-foundation.org>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
<linux-security-module@vger.kernel.org>,
Ingo Molnar <mingo@elte.hu>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Paul Turner <pjt@google.com>,
Thomas Gleixner <tglx@linutronix.de>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
NFS list <linux-nfs@vger.kernel.org>,
Trond Myklebust <Trond.Myklebust@netapp.com>,
"Bhamare, Sachin" <sbhamare@panasas.com>,
David Howells <dhowells@redhat.com>,
Eric Paris <eparis@redhat.com>,
"Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>,
Kay Sievers <kay.sievers@vrfy.org>,
James Morris <jmorris@namei.org>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Greg Kroah-Hartman <gregkh@suse.de>,
"Rafael J. Wysocki" <rjw@sisk.pl>,
"keyrings@linux-nfs.org" <keyrings@linux-nfs.org>
Subject: Re: [PATCHSET 0/6 version 2] kmod: Optional timeout on the wait in call_usermodehelper_exec
Date: Wed, 28 Mar 2012 14:42:04 -0700 [thread overview]
Message-ID: <4F7385AC.1070605@panasas.com> (raw)
In-Reply-To: <20120328201904.GA4517@redhat.com>
> On 03/27, Andrew Morton wrote:
>>
>> IOW, please explain at some length why you need this. Do you think
>> that there are existing call sites which can usefully use this feature?
>> Do you expect that new callers are likely to need this? etcetera.
>
Andrew Hi.
Yes all these explanations were on the other thread which started
this one. sorry.
On my next attempt I will also add a user for this API as a last
Patch. So you can see exactly how it makes sense, and why I do
need it. When I started this the user was not in the Kernel
yet, but now it's in for the 3.4-rc1, and it can make things clearer.
But in (very) short. The call is made in some special cases at
the read/write path of NFS. I have a very clean way out if
the call fails for any reason, not only timeout, it can fail
and properly handled in many other cases as well.
So early failure does not scare me, what does scare me a lot is
if it will get stuck (deadlocked) forever. This will eventually do
very bad things to the NFS client.
So the very easy thing to do is just add the proper timeout parameter
to existing wait_for_completion calls. If it means some cleanups and code
reorganization met on the way I don't mind.
And yes, I expect that a lot of users that now use UMH_WAIT_PROC
which need to sync with the app, will enjoy the timeout, just as I do.
And all users should be revisited and perhaps enhanced with the extra
robustness this gives.
BTW: Currently the script ran - "osd_login", as submitted
to the nfs-utils package maintainer, has a user-mode "watchdog"
sub-process that will kill the parent after a timeout. But I would not
like to rely on user-mode for this. There are plenty of other things
that can go wrong. I'd like the Kernel to be independent. Also in light
that the script may slightly change from distro to distro and is not
totally in my control.
On 03/28/2012 01:19 PM, Oleg Nesterov wrote:
> Cough. Can't resist... Could you also explain why
> http://marc.info/?l=linux-nfs&m=133252084301205 can't work?
>
Because it's another thread and another wait object and all bunch of
other allocations, where I already have 3 forks now. I don't do that
because the Politics to get new code in the Kernel is hard.
I'm utilizing all the current resources and am just exposing currently
hard coded constants to upper API's but in effect I'm not degrading
the hot path *at all*. I'll never do what you suggest it's a total waist
when the right way is just doing what is done today, only cleaned and
exposed.
> To clarify, I am just curious, I am not arguing. I am asking because
> if UMH_WAIT_PROC(timeout) fails with -ETIMEDOUT, then perhaps it makes
> sense to not "leak" the user-space process servicing the kernel request
> we were waiting for.
>
Hopefully it is not leaked, right? It will eventually return and de-allocate.
Do you mean that I should kill it? that's an additional mess in kmod.c.
But it's a good point I'll make it killable, and an admin can kill it if it's
really deadlocked forever.
> Oleg.
>
Thanks
Boaz
prev parent reply other threads:[~2012-03-28 21:43 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-20 23:18 [PATCHSET 0/4] kmod: Optional timeout on the wait in call_usermodehelper_exec Boaz Harrosh
2012-03-20 23:23 ` [PATCH 1/4] kmod: Un-export call_usermodehelper_freeinfo() Boaz Harrosh
2012-03-20 23:26 ` [PATCH 2/4] kmod: Convert two call sites to call_usermodehelper_fns() Boaz Harrosh
2012-03-22 3:00 ` James Morris
2012-03-20 23:28 ` [PATCH 3/4] kmod: Move call_usermodehelper_fns() to .c file and unexport it's helpers Boaz Harrosh
2012-03-20 23:32 ` [RFC 4/4] {RFC} kmod.c: Add new call_usermodehelper_timeout() API Boaz Harrosh
2012-03-22 2:44 ` Boaz Harrosh
2012-03-22 2:48 ` Boaz Harrosh
2012-03-22 2:52 ` Boaz Harrosh
2012-03-22 11:48 ` [RFC 4/4] {RFC} kmod.c: Add new call_usermodehelper_timeout()API Tetsuo Handa
2012-03-22 14:27 ` [RFC 4/4] {RFC} kmod.c: Add new call_usermodehelper_timeout() API Oleg Nesterov
2012-03-22 14:42 ` Oleg Nesterov
2012-03-22 19:08 ` Boaz Harrosh
2012-03-22 22:16 ` [RFC 4/4] {RFC} kmod.c: Add new call_usermodehelper_timeout()API Tetsuo Handa
2012-03-23 4:48 ` Boaz Harrosh
2012-03-23 5:23 ` Tetsuo Handa
2012-03-23 16:30 ` Oleg Nesterov
2012-03-23 13:34 ` [RFC 4/4] {RFC} kmod.c: Add new call_usermodehelper_timeout() API Oleg Nesterov
2012-03-21 15:35 ` [PATCHSET 0/4] kmod: Optional timeout on the wait in call_usermodehelper_exec Greg KH
2012-03-22 0:18 ` Boaz Harrosh
2012-03-22 0:31 ` Myklebust, Trond
2012-03-22 1:18 ` Boaz Harrosh
2012-03-27 1:57 ` [PATCHSET 0/6 version 2] " Boaz Harrosh
2012-03-27 2:00 ` [PATCH 1/6] kmod: Unexport call_usermodehelper_freeinfo() Boaz Harrosh
2012-03-27 2:02 ` [PATCH 2/6] kmod: Convert two call sites to call_usermodehelper_fns() Boaz Harrosh
2012-03-27 2:04 ` [PATCH 3/6] kmod: Move call_usermodehelper_fns() to .c file and unexport all it's helpers Boaz Harrosh
2012-03-27 2:06 ` [PATCH 4/6 OPTION-A] completion: Add new wait_for_completion_timeout_state Boaz Harrosh
2012-03-27 2:33 ` [PATCH 4/6 OPTION-A version 3] " Boaz Harrosh
2012-03-27 8:11 ` Peter Zijlstra
2012-03-28 18:19 ` Boaz Harrosh
2012-03-28 18:25 ` Peter Zijlstra
2012-03-28 17:38 ` Oleg Nesterov
2012-03-27 2:09 ` [PATCH 4/6 option-B] kmod: add new wait_for_completion_timeout_state() helper Boaz Harrosh
2012-03-27 2:13 ` [PATCH 5/6] kmod: Add new call_usermodehelper_timeout() API Boaz Harrosh
2012-03-27 15:43 ` Oleg Nesterov
2012-03-28 17:04 ` Oleg Nesterov
2012-03-27 2:15 ` [PATCH 6/6] kmod: optional: Convert the use of xchg to a kref Boaz Harrosh
2012-03-28 16:35 ` Oleg Nesterov
2012-03-27 21:07 ` [PATCHSET 0/6 version 2] kmod: Optional timeout on the wait in call_usermodehelper_exec Andrew Morton
2012-03-28 20:19 ` Oleg Nesterov
2012-03-28 21:42 ` Boaz Harrosh [this message]
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=4F7385AC.1070605@panasas.com \
--to=bharrosh@panasas.com \
--cc=Trond.Myklebust@netapp.com \
--cc=a.p.zijlstra@chello.nl \
--cc=akpm@linux-foundation.org \
--cc=dhowells@redhat.com \
--cc=ebiederm@xmission.com \
--cc=eparis@redhat.com \
--cc=gregkh@suse.de \
--cc=jmorris@namei.org \
--cc=kay.sievers@vrfy.org \
--cc=keyrings@linux-nfs.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=oleg@redhat.com \
--cc=penguin-kernel@I-love.SAKURA.ne.jp \
--cc=pjt@google.com \
--cc=rjw@sisk.pl \
--cc=sbhamare@panasas.com \
--cc=srivatsa.bhat@linux.vnet.ibm.com \
--cc=tglx@linutronix.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).