All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kent Overstreet <koverstreet@google.com>
To: Andrew Morton <akpm@linux-foundation.org>, jack@suse.cz, tytso@mit.edu
Cc: Valdis.Kletnieks@vt.edu, Hillf Danton <dhillf@gmail.com>,
	Benjamin LaHaise <bcrl@kvack.org>,
	linux-kernel@vger.kernel.org, linux-aio@kvack.org, zab@zabbo.net,
	linux-fsdevel@vger.kernel.org
Subject: Re: next-20130117 - kernel BUG with aio
Date: Thu, 24 Jan 2013 14:13:52 -0800	[thread overview]
Message-ID: <20130124221352.GK26407@google.com> (raw)
In-Reply-To: <20130124132759.c892fb4c.akpm@linux-foundation.org>

On Thu, Jan 24, 2013 at 01:27:59PM -0800, Andrew Morton wrote:
> Please also take a look at Jan's recent
> http://www.spinics.net/lists/linux-fsdevel/msg61738.html and have a
> think about how this plays with your patchset.

I can't think of any possible interactions - none of my aio stuff messes
with the way the fput() happens; the aio code does call fput() when the
kiocb is freed and my patches do touch _that_ code but none of the
behaviour there changes.

Might be worth documenting this though, I can't think of any reason it'd
be obvious looking at the aio code that the fput() has to happen after
aio_complete(). As with the bugs I just sent you patches for it's not
terribly clear who owns what in the kiocb when.

Reading those patches though - the main change is to call
inode_dio_done() before calling aio_complete(). All inode_dio_done()
does though is issue a wakeup - to whatever called inode_dio_wait().

That means whatever called inode_dio_wait() needs its own ref on the
inode, and from a cursory glance at the code it is _not_ at all clear to
me that's the case - if inode_dio_wait() is merely finishing things for
that specific IO that need to be done in process context, I can easily
imagine it not being the case.

Assuming whatever does call inode_dio_wait() does have its own ref,
there was only a real use after free when nothing was waiting on the
inode.

Similarly for the ext4 code to write unwritten extents - and having seen
and helped chase a bug in that code before, that code _definitely_ needs
auditing.

  parent reply	other threads:[~2013-01-24 22:13 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-21 13:24 next-20130117 - kernel BUG with aio Valdis Kletnieks
2013-01-22 13:43 ` Hillf Danton
2013-01-22 21:28   ` Valdis.Kletnieks
2013-01-23 12:10     ` Hillf Danton
2013-01-24 17:22       ` Valdis.Kletnieks
2013-01-24 21:18         ` Kent Overstreet
2013-01-24 21:27           ` Andrew Morton
2013-01-24 21:27             ` Andrew Morton
2013-01-24 21:39             ` Kent Overstreet
2013-01-24 21:39               ` Kent Overstreet
2013-01-24 22:25               ` Zach Brown
2013-01-24 22:25                 ` Zach Brown
2013-01-24 22:47                 ` Jeff Moyer
2013-01-24 22:47                   ` Jeff Moyer
2013-01-24 23:03                 ` Kent Overstreet
2013-01-24 22:13             ` Kent Overstreet [this message]
2013-01-29 13:41               ` Jan Kara
2013-01-29 13:41                 ` Jan Kara
2013-01-24 21:43           ` [PATCH 1/3] aio: Fix a null pointer deref in batch_complete_aio Kent Overstreet
2013-01-24 21:43             ` Kent Overstreet
2013-01-25 13:15             ` Hillf Danton
2013-01-25 13:15               ` Hillf Danton
2013-01-24 21:43           ` [PATCH 2/3] aio-kill-ki_retry-fix-fix Kent Overstreet
2013-01-24 21:43             ` Kent Overstreet
2013-01-24 21:43           ` [PATCH 3/3] aio-use-cancellation-list-lazily-fix Kent Overstreet
2013-01-24 21:43             ` Kent Overstreet
2013-01-25 13:30             ` Hillf Danton
2013-01-25 13:30               ` Hillf Danton
2013-01-25 23:12               ` Andrew Morton
2013-01-25 23:12                 ` Andrew Morton
2013-01-28 17:37                 ` Kent Overstreet
2013-01-28 17:37                   ` Kent Overstreet
2013-01-31 21:59     ` next-20130117 - kernel BUG with aio Andrew Morton
2013-02-01  0:37       ` Kent Overstreet
2013-02-05 15:53         ` Valdis.Kletnieks
2013-02-05 17:20           ` Kent Overstreet
2013-02-05 17:48             ` Valdis.Kletnieks
2013-02-06 17:15             ` Valdis.Kletnieks

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=20130124221352.GK26407@google.com \
    --to=koverstreet@google.com \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=akpm@linux-foundation.org \
    --cc=bcrl@kvack.org \
    --cc=dhillf@gmail.com \
    --cc=jack@suse.cz \
    --cc=linux-aio@kvack.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    --cc=zab@zabbo.net \
    /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.