lkml.org 
[lkml]   [2011]   [Sep]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/1] Trivial: devtmpfsd: Setting task running/interruptible states
On Wed, Sep 21, 2011 at 09:54:01PM +0530, kautuk.c @samsung.com wrote:
> Hi Greg,
>
> On Wed, Sep 21, 2011 at 9:24 PM, Greg KH <gregkh@suse.de> wrote:
> > On Wed, Sep 21, 2011 at 09:09:33PM +0530, Kautuk Consul wrote:
> >> This trivial patch makes the following changes in devtmpfsd() :
> >
> > This is not the definition of "trivial" in that you are changing the
> > logic of the code, not just doing spelling changes.
>
> Well, I didn't really change the performance/functionality so I called
> it trivial.

You changed the code logic, which is not trivial at all in this area.

And actually unneeded from what I can tell, right?

> >
> >> - Set the state to TASK_INTERRUPTIBLE using __set_current_state
> >>   instead of set_current_state as the spin_unlock is an implicit
> >>   memory barrier.
> >
> > Why?  What is this hurting with the original code?
>
> Nothing really hurting, that's why I called this patch trivial.
> There is an extra memory barrier we have to go through by way of
> set_current_state, which is mb().
> That would lead to more overhead on the parallel pipelines of the processor
> as they will have to cease being parallel for instructions before and after
> the memory barrier despite the fact that the spin_unlock already covers this.
> We can do without this because as per the Documentation/memory-barriers.txt,
> atomic operations and unlocks give reliable ordering to instructions.

But the current code is correct, and not hurting anything, and it's not
on a "fast path" at all, so I'd prefer to keep it as-is and not change
it for the sake of changing it, so I'm not going to accept this patch,
sorry.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2011-09-21 23:15    [W:0.047 / U:3.468 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site