All of lore.kernel.org
 help / color / mirror / Atom feed
From: Darren Hart <dvhltc@us.ibm.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Sripathi Kodi <sripathik@in.ibm.com>,
	Fr??d??ric Weisbecker <fweisbec@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC] [PATCH 0/2] Futex fault injection
Date: Tue, 01 Dec 2009 08:16:10 -0800	[thread overview]
Message-ID: <4B15414A.9040405@us.ibm.com> (raw)
In-Reply-To: <20091201125537.GA23382@elte.hu>

Ingo Molnar wrote:
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
>> On Tue, 2009-12-01 at 11:33 +0100, Ingo Molnar wrote:
>>> * Sripathi Kodi <sripathik@in.ibm.com> wrote:
>>>
>>>> Hi,
>>>>
>>>> This patch set adds fault injection for futex subsystem. It adds 
>>>> faults at places where reading/writing from user space can return 
>>>> EFAULT. This will be useful in testing any significant change to futex 
>>>> subsystem.
>>> Instead of this unacceptably ugly and special-purpose debugfs 
>>> interface, please extend perf events to allow event injection. Some 
>>> other places in the kernel (which deal with rare events) want/need 
>>> this capability too.
>> Thing is, he's using the 'normal' fault injection code to do this, I 
>> see no objection to doing that.
> 
> Yes - but its impact to the futex code is butt-ugly. That some facility 
> is in the kernel does not mean it gets a free pass to be applied 
> everywhere and anywhere.

I don't think the "butt-ugly" argument is enough to reject the patch. 
It's a fairly subjective metric and I don't think the proposed solution 
results in "pretty" code either. In fact the super long function names 
and multi-line conditionals are arguably "ugly" (maybe not "butt-ugly" 
though). :-)

However, the arguments are solid and I understand wanting to introduce a 
new feature in a particular way. Has there been any work done on perf 
event injection up to this point or would this be a completely new perf 
feature?

--
Darren

> 
> An example of that would be tracepoints - there's no free pass to add 
> tracepoints in new places and some maintainers elect to use different 
> facilities. (or reject all current facilities)
> 
>> If you want to redo the fault injection subsystem, then that's another 
>> story, but then we need to convert all of its users over.
> 
> What i want to see is sane code in futex.c. If we add hooks/callbacks 
> i'd like it to be a complete solution helping a lot of usecases not some 
> limited approach helping testability only.
> 
> Thanks,
> 
> 	Ingo


-- 
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team

  reply	other threads:[~2009-12-01 16:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-01  8:46 [RFC] [PATCH 0/2] Futex fault injection Sripathi Kodi
2009-12-01  8:49 ` [RFC] [PATCH 1/2] Futex fault injection: Add fault points Sripathi Kodi
2009-12-01  8:51 ` [RFC] [PATCH 2/2] Futex fault injection: Config option Sripathi Kodi
2009-12-01 10:33 ` [RFC] [PATCH 0/2] Futex fault injection Ingo Molnar
2009-12-01 10:54   ` Peter Zijlstra
2009-12-01 12:55     ` Ingo Molnar
2009-12-01 16:16       ` Darren Hart [this message]
2009-12-01 16:23         ` Ingo Molnar
2009-12-02  5:58           ` Sripathi Kodi
2009-12-02  9:19             ` 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=4B15414A.9040405@us.ibm.com \
    --to=dvhltc@us.ibm.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=sripathik@in.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 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.