LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Dean Nelson <dcn@sgi.com>
Cc: Al Viro <viro@ftp.linux.org.uk>,
	akpm@linux-foundation.org, linux-ia64@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	tony.luck@intel.com, jes@sgi.com
Subject: Re: [PATCH 1/4] export __put_task_struct for XPMEM
Date: Tue, 28 Aug 2007 19:01:44 +0100	[thread overview]
Message-ID: <20070828180144.GA32585@infradead.org> (raw)
In-Reply-To: <20070827181056.GA30176@sgi.com>

On Mon, Aug 27, 2007 at 01:10:56PM -0500, Dean Nelson wrote:
> > Does it?  Well, then open the file in question and start doing close(dup(fd))
> > in a loop.  Won't take long for an oops...
> 
> Actually it won't oops. And that's because when the file is opened,
> xpmem_open() creates a structure for that thread group, and when
> xpmem_flush() is called on the close() it first looks for that structure
> and if it finds it then it does what it needs to do (which includes the
> put_task_struct() call) and then finishes off by destroying the structure.
> So for subsequent closes xpmem_flush() returns without calling
> put_task_struct().

Your refcounting is totally broken.  fds can be passed around in the same
process group (which btw is not something driver should look at because
there are variants of different kinds of process groups depending on clone
flags), and your driver is going boom in most of the case.

We don't export the routines to acquire reference to task structs for
a reason, and this piece of junk called xpmem is not going to change it.

  parent reply	other threads:[~2007-08-28 18:02 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-27 15:56 [PATCH 0/4] SGI Altix cross partition memory (XPMEM) Dean Nelson
2007-08-27 15:59 ` [PATCH 1/4] export __put_task_struct for XPMEM Dean Nelson
2007-08-27 16:13   ` Al Viro
2007-08-27 18:10     ` Dean Nelson
2007-08-27 18:15       ` Al Viro
2007-08-27 19:19         ` Dean Nelson
2007-08-27 19:35           ` Al Viro
2007-08-27 20:24             ` Robin Holt
2007-08-27 20:47               ` Al Viro
2007-08-28 18:01       ` Christoph Hellwig [this message]
2007-08-27 16:01 ` [PATCH 2/4] export zap_page_range " Dean Nelson
2007-08-27 16:02 ` [PATCH 3/4] add new lock ordering rule to mm/filemap.c Dean Nelson
2007-08-27 16:41 ` [PATCH 4/4] add SGI Altix cross partition memory (XPMEM) driver Dean Nelson
2007-08-27 20:00   ` Adrian Bunk
2007-08-28 18:02   ` Christoph Hellwig
2007-08-28 19:00     ` Robin Holt
2007-08-28 19:05       ` Christoph Hellwig

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=20070828180144.GA32585@infradead.org \
    --to=hch@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=dcn@sgi.com \
    --cc=jes@sgi.com \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=tony.luck@intel.com \
    --cc=viro@ftp.linux.org.uk \
    --subject='Re: [PATCH 1/4] export __put_task_struct for XPMEM' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).