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
next prev parent 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.