public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <ak@suse.de>
To: Ingo Molnar <mingo@elte.hu>
Cc: Ulrich Drepper <drepper@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arjan van de Ven <arjan@infradead.org>,
	David Singleton <dsingleton@mvista.com>,
	Andrew Morton <akpm@osdl.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [patch 0/5] lightweight robust futexes: -V1
Date: 15 Feb 2006 18:35:13 +0100	[thread overview]
Message-ID: <p73lkwc5xv2.fsf@verdi.suse.de> (raw)
In-Reply-To: <20060215151711.GA31569@elte.hu>

Ingo Molnar <mingo@elte.hu> writes:


> In practice, when e.g. yum is kill -9-ed (or segfaults), a system reboot 
> is needed to release that futex based lock. This is one of the leading 
> bugreports against yum.

Reboot?  That object is stored somewhere in user space, isn't it? 
And wherever it is that object can be removed, can't it?

e.g. if you have it in a shared memory object you could just
add some code to always kill everybody who has the shared memory
mapped.

I wrote code like this some time ago when I was experimenting
with a new machine check handler. It looked for all processes
mapping some memory when the CPU reported it corrupted and killed them.

e.g. you could add a new VMA flag that says "when one user
of this dies unexpectedly by a signal kill all" 

That would solve the problem too, wouldn't it? 

It might not be highly available, but people who want that
can just use the plain old sysv in kernel locks. In theory
you could make it in fact highly available by catching the signal
in the other process and then doing the lock cleanup from there.

Of course it won't help if the lock is stored on disk,
but that's not in any way different from any other existing disk
based lock file.
 
> Ulrich Drepper has implemented the necessary glibc support for this new 
> mechanism, which fully enables robust mutexes. (Ulrich plans to commit 
> these changes to glibc-HEAD later today.)

And what happens if the patch is rejected? I don't really think you
can force patches in this way ("do it or I break glibc")

I think it really needs proper discussion first before it's merged
anywhere. And glibc should be the slave of the kernel on this,
not the other way round.


> The patch adds two new syscalls: one to register the userspace list, and 
> one to query the registered list pointer:
> 
>  asmlinkage long
>  sys_set_robust_list(struct robust_list_head __user *head,
>                      size_t len);
> 
>  asmlinkage long
>  sys_get_robust_list(int pid, struct robust_list_head __user **head_ptr,
>                      size_t __user *len_ptr);

What happens when the list gets corrupted? Does the kernel go
into an endless loop? Kernel going through arbitary user structures
like this seems very risky to me. There are ways to do
list walking with cycle detection, but they still have quite
bad worst case detection times.

Or when parts of these mappings are remote on NFS and the server is down
etc - then  the kernel could potentially block a long time in exit.

-Andi


  parent reply	other threads:[~2006-02-15 17:35 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-02-15 15:17 [patch 0/5] lightweight robust futexes: -V1 Ingo Molnar
2006-02-15 15:22 ` Ingo Molnar
2006-02-15 17:35 ` Andi Kleen [this message]
2006-02-15 17:50   ` Ulrich Drepper
2006-02-15 18:42     ` Andi Kleen
2006-02-15 19:49       ` Christopher Friesen
2006-02-15 20:02         ` Andi Kleen
2006-02-15 20:13           ` Antonio Vargas
2006-02-15 20:25             ` Andi Kleen
2006-02-15 20:59           ` Ingo Molnar
2006-02-15 20:43       ` Ingo Molnar
2006-02-15 19:05 ` Daniel Walker
2006-02-15 19:11   ` Arjan van de Ven
2006-02-15 19:13     ` Daniel Walker
2006-02-15 21:31   ` Ingo Molnar
2006-02-16 15:43     ` Daniel Walker
2006-02-15 21:45 ` Andrew Morton
2006-02-15 22:14   ` Ingo Molnar
2006-02-17 21:59     ` Daniel Jacobowitz
2006-02-16  3:57 ` Darren Hart
2006-02-16 14:58 ` Johannes Stezenbach
2006-02-16 17:20   ` Ingo Molnar
2006-02-16 19:04     ` Daniel Walker
2006-02-17  9:09       ` Avi Kivity
2006-02-17 19:55     ` Johannes Stezenbach
2006-02-17 20:02       ` Arjan van de Ven

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=p73lkwc5xv2.fsf@verdi.suse.de \
    --to=ak@suse.de \
    --cc=akpm@osdl.org \
    --cc=arjan@infradead.org \
    --cc=drepper@redhat.com \
    --cc=dsingleton@mvista.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --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