headers
Arve Hjønnevåg | 1 Mar 2009 01:06
Favicon

Re: [RFD] Automatic suspend

On Sat, Feb 28, 2009 at 2:53 PM, Rafael J. Wysocki <rjw <at> sisk.pl> wrote:
>> >> What you are talking about here is mostly an optimization of the
>> >> wakelock api. You have removed timeout support and made each wakelock
>> >> reference counted.
>> >
>> > I also removed the arbitrary number of wakelocks (I really _hate_ the name,
>> > can we please stop using it from now on?).
>>
>> What do you mean by this? You removed the struct wake_lock?
>
> Basically, yes.

Why is this better? If you move the stats into devices and tasks, this
may take up more space than adding an object to the structures or
tasks that you are protecting.

>> >> I do provide an option to turn off the wakelock stats, which makes
>> >> wake_lock/unlock significantly faster, but we never run with wakelock
>> >> stats off. Also, it pushes timeout handling to the drivers. I know may
>> >> of you don't like timeout support, but ignoring the problem is not a
>> >> solution. If each driver that needs timeouts uses its own timer, then
>> >> you will often wakeup from idle just to unlock a wakelock that will
>> >> not trigger suspend. This wakeup is a thousand times as costly on the
>> >> msm platform as a wakelock/unlock pair (with wakelock stats enabled).
>> >
>> > Well, at least a couple of people told you that the timeouts are hardly
>> > acceptable and they told you why.  Please stop repeating the same arguments you
>> > have given already for a couple of times.  They're not convincing.
>>
>> And you keep ignoring them.
(Continue reading)

Permalink | Reply | 16 comments |
headers
David Miller | 1 Mar 2009 01:06
Favicon

Re: [patch] x86, mm: pass in 'total' to __copy_from_user_*nocache()

From: Linus Torvalds <torvalds <at> linux-foundation.org>
Date: Sat, 28 Feb 2009 09:42:18 -0800 (PST)

> On Sat, 28 Feb 2009, Arjan van de Ven wrote:
> > 
> > it invalidates all caches in the hierarchy
> 
> Yeah, now that I look at the intel pdf's, I see that.
> 
> > afaik this is what Intel cpus do; but I also thought this behavior was
> > quite architectural as well...
> 
> Ok, I really think we should definitely not use non-temporal stores for 
> anything smaller than one full page in that case. In fact, I wonder if 
> even any of the old streaming benchmarks are even true. I thought it would 
> still stay in the L3, but yes, it literally seems to make the access 
> totally noncached and WC.
> 
> That's almost unacceptable in the long run. With a 8MB L3 cache - and a 
> compile sequence, do we really want to go out to memory to write the .S 
> file, and then have the assembler go out to memory to read it back? For a 
> compile, that _probably_ is all fine (the compiler in particular will have 
> enough data structures around that it's not going to fit in the cache 
> anyway), but I'm seeing leaner compilers and other cases where forcing 
> things out all the way on the bus is simply the wrong thing.

I think this is an accurate analysis as well, it's really unfortunate
the non-temporal stuff on x86 doesn't preserve existing cache lines
when present.
(Continue reading)

Permalink | Reply | 16 comments |
headers
Theodore Tso | 1 Mar 2009 01:18
Picon
Picon
Favicon
Gravatar

Re: [PATCH] checkpatch: Warn on empty commit log bodies

On Sun, Mar 01, 2009 at 12:01:38AM +0100, Stefan Richter wrote:
> > As I have previously said, that is not the case in reality.  There
> > appears to be substantial sentiment among people handling patches that
> > not having any text in the body of the e-mail makes it harder to handle
> > patches.
> 
> It is indeed a problem
>   - if the patch title alone insufficiently describes the patch
> or
>   - if a patch reviewer believes that it is OK to ignore patch titles.

Worse yet, if we start getting these sorts of entries being returned
by "git log":

------------
ext4: Fix spelling error: successfull

Fix spelling error: successfull

Signed-off-by: "Trivial Patch Submitter" <spelling <at> nits.org>
------------

Just to shut up checkpatch, I'm going to feel the urge to shake a
checkpatch maintainer warmly by the throat.

Sometimes, all that is needed is:

------------
ext4: Fix spelling error: successfull
(Continue reading)

Permalink | Reply | 13 comments |
headers
Andi Kleen | 1 Mar 2009 01:40

Re: [patch] x86, mm: pass in 'total' to __copy_from_user_*nocache()

> I think this is an accurate analysis as well, it's really unfortunate
> the non-temporal stuff on x86 doesn't preserve existing cache lines
> when present.
> 
> I thought that was the whole point.  Don't pollute the caches, but
> if cache lines are already loaded there, use them and don't purge!

x86 actually supports that, it's just not done through movnt.

You can do that on x86 by using PREFETCHNTA (or T0/T1/T2 for specific
cache levels). Typically this is implemented by forcing the cache line 
to only a single way of the cache (so only using max 1/8 or so of your last 
level cache) 

I'm not sure how it interacts with REP MOVS* though, this internally
tends to do additional magic for larger copies.

-Andi

--

-- 
ak <at> linux.intel.com -- Speaking for myself only.
Permalink | Reply | 15 comments |
headers
Felipe Balbi | 1 Mar 2009 01:30

Re: [PATCH 1/2] input: misc: add twl4030-pwrbutton driver

On Sat, Feb 28, 2009 at 02:23:03PM -0800, Dmitry Torokhov wrote:
> Hi Felipe,
> 
> On Fri, Feb 27, 2009 at 09:28:02PM +0200, Felipe Balbi wrote:
> > From: Felipe Balbi <felipe.balbi <at> nokia.com>
> > 
> > This is part of the twl4030 multifunction device driver.
> > 
> > With this driver we add support for reporting KEY_POWER
> > events via the input layer.
> 
> ...

thanks for reviewing, how about the version below:

========== cut here ==========

From 60c4980bc13b08c73aa5b647cda6ef540ac94939 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <felipe.balbi <at> nokia.com>
Date: Fri, 27 Feb 2009 21:18:15 +0200
Subject: [PATCH] input: misc: add twl4030-pwrbutton driver

This is part of the twl4030 multifunction device driver.

With this driver we add support for reporting KEY_POWER
events via the input layer.

Cc: David Brownell <dbrownell <at> users.sourceforge.net>
Cc: Dmitry Torokhov <dmitry.torokhov <at> gmail.com>
Cc: Samuel Ortiz <sameo <at> openedhand.com>
(Continue reading)

Permalink | Reply | 14 comments |
headers
H. Peter Anvin | 1 Mar 2009 01:28
Favicon

Re: [patch] x86, mm: pass in 'total' to __copy_from_user_*nocache()

Andi Kleen wrote:
>> I think this is an accurate analysis as well, it's really unfortunate
>> the non-temporal stuff on x86 doesn't preserve existing cache lines
>> when present.
>>
>> I thought that was the whole point.  Don't pollute the caches, but
>> if cache lines are already loaded there, use them and don't purge!
> 
> x86 actually supports that, it's just not done through movnt.
> 
> You can do that on x86 by using PREFETCHNTA (or T0/T1/T2 for specific
> cache levels). Typically this is implemented by forcing the cache line 
> to only a single way of the cache (so only using max 1/8 or so of your last 
> level cache) 
> 
> I'm not sure how it interacts with REP MOVS* though, this internally
> tends to do additional magic for larger copies.

The PREFETCHNTA stuff is really for reads rather than writes, however.
Yes, you can prefetch the cache line you're about to overwrite, but I
suspect (I haven't verified) that you lose out on whole-line
optimizations that way.

	-hpa

--

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
(Continue reading)

Permalink | Reply | 14 comments |
headers
Arjan van de Ven | 1 Mar 2009 01:38
Favicon

Re: [patch] x86, mm: pass in 'total' to __copy_from_user_*nocache()

On Sat, 28 Feb 2009 16:28:58 -0800
"H. Peter Anvin" <hpa <at> zytor.com> wrote:

> Andi Kleen wrote:
> >> I think this is an accurate analysis as well, it's really
> >> unfortunate the non-temporal stuff on x86 doesn't preserve
> >> existing cache lines when present.
> >>
> >> I thought that was the whole point.  Don't pollute the caches, but
> >> if cache lines are already loaded there, use them and don't purge!
> > 
> > x86 actually supports that, it's just not done through movnt.
> > 
> > You can do that on x86 by using PREFETCHNTA (or T0/T1/T2 for
> > specific cache levels). Typically this is implemented by forcing
> > the cache line to only a single way of the cache (so only using max
> > 1/8 or so of your last level cache) 
> > 
> > I'm not sure how it interacts with REP MOVS* though, this internally
> > tends to do additional magic for larger copies.
> 
> The PREFETCHNTA stuff is really for reads rather than writes, however.
> Yes, you can prefetch the cache line you're about to overwrite, but I
> suspect (I haven't verified) that you lose out on whole-line
> optimizations that way.

the entire point of using movntq and friends was to save half the
memory bandwidth to not pull it into the cache before writing...
.... so bad idea to do prefetch<anything>
(Continue reading)

Permalink | Reply | 13 comments |
headers
Dmitry Torokhov | 1 Mar 2009 01:42
Picon

Re: [PATCH] Input notifier support

On Sun, Mar 01, 2009 at 12:34:38AM +0100, Andi Kleen wrote:
> Kyungmin Park <kmpark <at> infradead.org> writes:
> 
> > Some hardware doesn't connected with key button and led. In this case key should be connected with led by
software. Of course each application can control it however it's too big burden to application programmer.
> >
> > So add input notifier and then use it at other frameworks such as led.
> > Of course, other input device can use this one.
> >
> > Any commnets are welcome.
> 
> It looks like the perfect interface for a password stealing root kit.
> 
> Yes there are probably other ways to do this, but still this seems to
> make it very easy.
> 

We already have good interface for that. That's why you want to limit
access to /dev/input/eventX ;) and not make it world-readable.

--

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 8 comments |
headers
Mark Brown | 1 Mar 2009 01:46
Picon
Gravatar

Re: [PATCH] checkpatch: Warn on empty commit log bodies

On Sat, Feb 28, 2009 at 07:18:29PM -0500, Theodore Tso wrote:

> Just to shut up checkpatch, I'm going to feel the urge to shake a
> checkpatch maintainer warmly by the throat.

> Sometimes, all that is needed is:

> ------------
> ext4: Fix spelling error: successfull
> 
> Signed-off-by: "Trivial Patch Submitter" <spelling <at> nits.org>
> ------------

As I've said already I pretty much agree with this.

The reason I sent the patch was that sending changelogs like that for
trivial changes is getting me negative feedback and I'm seeing other
comments about "unchangeloged patches" on the lists so I'm pretty sure
it's not just something I'm doing.  I'm not saying I'm always blameless
here but when people are using terms like like "unchangeloged" it really
does suggest that one line changelogs are just considered not to have
changelogs.

Some consistency would be good here.
Permalink | Reply | 11 comments |
headers
Dmitry Torokhov | 1 Mar 2009 01:58
Picon

Re: [PATCH 1/2] input: misc: add twl4030-pwrbutton driver

On Sun, Mar 01, 2009 at 02:30:18AM +0200, Felipe Balbi wrote:
> On Sat, Feb 28, 2009 at 02:23:03PM -0800, Dmitry Torokhov wrote:
> > Hi Felipe,
> > 
> > On Fri, Feb 27, 2009 at 09:28:02PM +0200, Felipe Balbi wrote:
> > > From: Felipe Balbi <felipe.balbi <at> nokia.com>
> > > 
> > > This is part of the twl4030 multifunction device driver.
> > > 
> > > With this driver we add support for reporting KEY_POWER
> > > events via the input layer.
> > 
> > ...
> 
> thanks for reviewing, how about the version below:
> 

Looks good, couple more items...

>  
> +config INPUT_TWL4030_PWRBUTTON
> +	tristate "TWL4030 Power button Driver"
> +	depends on TWL4030_CORE
> +

Help should be added, at least document module name as we do with
other drivers.

> +
> +static struct input_dev *powerbutton_dev;
(Continue reading)

Permalink | Reply | 12 comments |

Gmane