All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Roland McGrath <roland@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86_64 ia32 syscall restart fix
Date: Fri, 29 Feb 2008 17:45:10 +0100	[thread overview]
Message-ID: <20080229164510.GA6850@elte.hu> (raw)
In-Reply-To: <alpine.LFD.1.00.0802290814320.17889@woody.linux-foundation.org>


* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> > > The code to restart syscalls after signals depends on checking for a 
> > > negative orig_ax, and for particular negative -ERESTART* values in ax. 
> > > These fields are 64 bits and for a 32-bit task they get zero-extended. 
> > > The syscall restart behavior is lost, a regression from a native 
> > > 32-bit kernel and from 64-bit tasks' behavior.  This patch fixes the 
> > > problem by doing sign-extension where it matters.  For orig_ax, the 
> > > only time the value should be -1 but winds up as 0x0ffffffff is via a 
> > > 32-bit ptrace call.  So the patch changes ptrace to sign-extend the 
> > > 32-bit orig_eax value when it's stored; it doesn't change the checks 
> > > on orig_ax, though it uses the new current_syscall() inline to better 
> > > document the subtle importance of the used of signedness there.  The 
> > > ax value is stored a lot of ways and it seems hard to get them all 
> > > sign-extended at their origins.  So for that, we use the 
> > > current_syscall_ret() to sign-extend it only for 32-bit tasks at the 
> > > time of the -ERESTART* comparisons.
> > 
> > thanks, applied.
> 
> Btw, can we please try to keep commit log messages readable?

yeah - the minute i added the patch i pinged Roland about that offline.

> Yeah, maybe it's just me, but I like my whitespace. 
> Ihaveareallyhardtime 
> readingtextthatdoesn'thavethepropermarkersforwhereconceptsstartandbegin, 
> andthatreallydoesincludetheverticalwhitespacetoo.

heh :)

> Now, the only reason I mention this is that normally I would probably 
> just have fixed this up myself without even a comment (because it's 
> such a tiny detail that it's not not worth one), but when Ingo merges 
> it I'll now get it through git and it will be fixed.

currently the reality is that i have to fix over 90% of the commit 
messages that go towards you :-/

While i'd like that proportion to be a lot lower, it's really hard for 
people to write good commit messages for fixes: people tend to send 
their fixes the minute they find the problem (being happy about having 
found and fixed a problem!), so the commit message gets little 
attention.

another effect is that kernel generalist people like Roland have a very 
large list of todo items so when they write up the commit message they 
might be thinking about the next unsolved problem already - and the 
commit message becomes a quick, unstructured and semi-automatic 
brain-dump of all details in essence :-/

Also, who am i to complain about the commit message - i'm often the one 
who has put the bug in to begin with! [ So i'm perfectly happy with you 
volunteering to take over that role ;-) ]

But yes, it's easier for me too to sort and prioritize patches if their 
description has good structure, so i regularly try to remind high-volume 
patch submitters about that. ( With little success i have to say - 
commit messages seem to be suffering from the same curse of inattention 
as other types of documentation do :-/ )

	Ingo

  reply	other threads:[~2008-02-29 16:45 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-29  3:57 [PATCH] x86_64 ia32 syscall restart fix Roland McGrath
2008-02-29 15:52 ` Ingo Molnar
2008-02-29 16:26   ` Linus Torvalds
2008-02-29 16:45     ` Ingo Molnar [this message]
2008-02-29 17:03       ` Linus Torvalds
2008-02-29 17:17         ` Ingo Molnar
2008-02-29 17:37           ` Ingo Molnar
2008-02-29 21:02             ` Andrew Morton
2008-02-29 21:20               ` Ingo Molnar
2008-03-01  5:48                 ` [stable] " Greg KH
2008-02-29 22:42     ` Roland McGrath
2008-02-29 23:18       ` Linus Torvalds
2008-03-07 22:56 ` [PATCH] x86_64 ptrace orig_ax on ia32 task Roland McGrath
2008-03-07 23:18   ` Linus Torvalds
2008-03-08  1:37     ` Roland McGrath
2008-03-10 19:19   ` Chuck Ebbert
2008-03-10 19:48     ` Linus Torvalds
2008-03-10 20:01       ` Roland McGrath
2008-03-11  9:32         ` Ingo Molnar

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=20080229164510.GA6850@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=roland@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.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.