linux-kernel - Re: [PATCH 1/1] Trivial: devtmpfsd: Setting task running/interruptible states

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110921211007.GA13605@suse.de>
Date:	Wed, 21 Sep 2011 14:10:07 -0700
From:	Greg KH <gregkh@...e.de>
To:	"kautuk.c @samsung.com" <consul.kautuk@...il.com>
Cc:	Jiri Kosina <trivial@...nel.org>, jkosina@...e.cz,
	linux-kernel@...r.kernel.org
Subject: Re: [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@...e.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/