All of lore.kernel.org
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: David Rientjes <rientjes@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Konrad Rzeszutek Wilk <konrad@kernel.org>,
	Dave Jones <davej@redhat.com>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>,
	Suzuki Poulose <suzuki@in.ibm.com>,
	Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
Subject: Re: 3.6rc6 slab corruption.
Date: Thu, 20 Sep 2012 17:14:44 -0400	[thread overview]
Message-ID: <20120920211443.GA27312@konrad-lan.dumpdata.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1209191608160.17867@chino.kir.corp.google.com>

On Wed, Sep 19, 2012 at 04:20:25PM -0700, David Rientjes wrote:
> On Wed, 19 Sep 2012, Linus Torvalds wrote:
> 
> > That does look simpler, and avoiding the lock is a good idea. Since we
> > don't support lseek() (or pread/pwrite) on that thing anyway, there's
> > no way to keep the fd open and just re-use it to read the data over
> > and over, so populating it at open time sounds like a good solution
> > with no real downsides.
> > 
> 
> Yeah, my patch is functionally the same as what we currently have with the 
> only exception being that it isn't racy.  I'm wondering if that's what we 
> really want, though, since the data read from the file will remain 
> persistent as long as it is opened.  That obviously happens in my patch 
> because we allocate and copy the buffer at open(), but also happens 
> implicitly with the old code precisely because it's a non-seekable file 
> and *ppos == 0 only once (when not racy).
> 
> So if the API for these xen files is to remain persistent after open() as 
> it currently does, then my patch solves the issue.  However, if the API 

Nah. It was initially a debug option to see how contended the spinlocks
are. Nobody but developers should look at it - and they can deal with
open/close cycle. Thought we should probably provide a nice little
comment in the file mentioning the reason for stale data.

> wants to allow to only open() once and then read the spinlock_stats data 
> continuously, then we'll need the mutex: allocate the file->private_data 
> buffer once at open() for the maximum allowable size and then copy to the 
> buffer from xen's spinlock_stats under the protection of the mutex to 
> read().
> 
> Konrad?

Your patch is way simpler and it does the job better than mine.

  reply	other threads:[~2012-09-20 21:15 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-18 14:35 3.6rc6 slab corruption Dave Jones
2012-09-18 18:38 ` Linus Torvalds
2012-09-18 18:53   ` Dave Jones
2012-09-19 14:00     ` Raghavendra K T
2012-09-19 17:09       ` Linus Torvalds
2012-09-19 21:27         ` David Rientjes
2012-09-19 21:41           ` Dave Jones
2012-09-18 19:23   ` Konrad Rzeszutek Wilk
2012-09-18 20:24     ` David Rientjes
2012-09-18 20:31       ` Konrad Rzeszutek Wilk
2012-09-18 20:36       ` Linus Torvalds
2012-09-19  0:57       ` Raghavendra K T
2012-09-18 20:35     ` Linus Torvalds
2012-09-18 20:37       ` Konrad Rzeszutek Wilk
2012-09-18 20:49         ` Linus Torvalds
2012-09-19  1:16           ` Raghavendra K T
2012-09-19 19:16           ` Konrad Rzeszutek Wilk
2012-09-19 21:29             ` David Rientjes
2012-09-19 21:49               ` David Rientjes
2012-09-19 23:01                 ` Linus Torvalds
2012-09-19 23:20                   ` David Rientjes
2012-09-20 21:14                     ` Konrad Rzeszutek Wilk [this message]
2012-09-20  2:29                 ` Raghavendra K T
2012-09-20  2:46                   ` David Rientjes
2012-09-20  2:55                     ` Raghavendra K T
2012-09-20 21:18                     ` Konrad Rzeszutek Wilk
2012-09-21  9:16                       ` [patch for-3.6] fs, debugfs: fix race in u32_array_read and allocate array at open David Rientjes
2012-09-21 10:22                         ` Raghavendra K T
2012-09-24 22:26                           ` David Rientjes
2012-09-25  2:54                             ` Raghavendra K T
2012-09-20 12:57                 ` 3.6rc6 slab corruption Raghavendra K T
2012-09-20 21:18                   ` Konrad Rzeszutek Wilk
2012-09-20 12:55             ` Raghavendra K T

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=20120920211443.GA27312@konrad-lan.dumpdata.com \
    --to=konrad.wilk@oracle.com \
    --cc=davej@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=konrad@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=raghavendra.kt@linux.vnet.ibm.com \
    --cc=rientjes@google.com \
    --cc=suzuki@in.ibm.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vatsa@linux.vnet.ibm.com \
    /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.