All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-mm@kvack.org, xfs@oss.sgi.com
Subject: Re: [PATCH 02/17] xfs: skip writeback from reclaim context
Date: Thu, 3 Jun 2010 02:52:52 -0400	[thread overview]
Message-ID: <20100603065252.GA28592@infradead.org> (raw)
In-Reply-To: <20100602230209.GA27325@dastard>

On Thu, Jun 03, 2010 at 09:02:09AM +1000, Dave Chinner wrote:
> Did you skip it unconditionally, or only when a transaction was
> required?

xfs_vm_releasepage is mostly a no-op if no transaction is required.
If we have neither delalloc nor unwritten buffer we do not actually
enter xfs_page_state_convert, and ->releasepage also doesn't touch
unampped buffers at all.

> The scary part is that I've seen stack traces (i.e. most stack used)
> through this reclaim path for delalloc conversion even for
> allocations that are GFP_NOFS and the only thing saving us from
> deadlocks is th PF_FSTRANS check. Even worse is that
> shrinker_page_list() will call try_to_release_pages() without
> checking whether it's allowed to enter the filesystem or not, so we
> can be doing block allocation in places we've specifically told the
> memory allocation subsystem not to....

s/shrinker_page_list/shrink_page_list/ and
s/try_to_release_pages/try_to_release_page/ above.

shrink_page_list takes the gfp_mask for try_to_release_page from
the scan_control structure passed to it from all the top of the long
callchain.  I can't find anobvious bug, but this could cause a lot
more harm.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com, linux-mm@kvack.org
Subject: Re: [PATCH 02/17] xfs: skip writeback from reclaim context
Date: Thu, 3 Jun 2010 02:52:52 -0400	[thread overview]
Message-ID: <20100603065252.GA28592@infradead.org> (raw)
In-Reply-To: <20100602230209.GA27325@dastard>

On Thu, Jun 03, 2010 at 09:02:09AM +1000, Dave Chinner wrote:
> Did you skip it unconditionally, or only when a transaction was
> required?

xfs_vm_releasepage is mostly a no-op if no transaction is required.
If we have neither delalloc nor unwritten buffer we do not actually
enter xfs_page_state_convert, and ->releasepage also doesn't touch
unampped buffers at all.

> The scary part is that I've seen stack traces (i.e. most stack used)
> through this reclaim path for delalloc conversion even for
> allocations that are GFP_NOFS and the only thing saving us from
> deadlocks is th PF_FSTRANS check. Even worse is that
> shrinker_page_list() will call try_to_release_pages() without
> checking whether it's allowed to enter the filesystem or not, so we
> can be doing block allocation in places we've specifically told the
> memory allocation subsystem not to....

s/shrinker_page_list/shrink_page_list/ and
s/try_to_release_pages/try_to_release_page/ above.

shrink_page_list takes the gfp_mask for try_to_release_page from
the scan_control structure passed to it from all the top of the long
callchain.  I can't find anobvious bug, but this could cause a lot
more harm.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2010-06-03  6:50 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-31 16:07 [PATCH 00/17] pending patches Christoph Hellwig
2010-05-31 16:07 ` [PATCH 01/17] xfs: remove done roadmap item from xfs-delayed-logging-design.txt Christoph Hellwig
2010-06-02  4:33   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 02/17] xfs: skip writeback from reclaim context Christoph Hellwig
2010-06-02  4:39   ` Dave Chinner
2010-06-02 10:08     ` Christoph Hellwig
2010-06-02 23:02       ` Dave Chinner
2010-06-03  6:52         ` Christoph Hellwig [this message]
2010-06-03  6:52           ` Christoph Hellwig
2010-05-31 16:07 ` [PATCH 03/17] xfs: improve xfs_isilocked Christoph Hellwig
2010-06-02  4:41   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 04/17] xfs: drop dmapi hooks Christoph Hellwig
2010-06-02  4:45   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 05/17] xfs: remove unneeded #include statements Christoph Hellwig
2010-06-02  4:45   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 06/17] xfs: simplify log item descriptor tracking Christoph Hellwig
2010-06-02  5:11   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 07/17] xfs: merge iop_unpin_remove into iop_unpin Christoph Hellwig
2010-06-02  5:14   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 08/17] xfs: give xfs_item_ops methods the correct prototypes Christoph Hellwig
2010-06-02  5:30   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 09/17] xfs: give li_cb callbacks the correct prototype Christoph Hellwig
2010-06-02  5:45   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 10/17] xfs: simplify buffer pinning Christoph Hellwig
2010-06-02  5:47   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 11/17] xfs: simplify inode to transaction joining Christoph Hellwig
2010-06-02  5:57   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 12/17] xfs: fix the xfs_log_iovec i_addr type Christoph Hellwig
2010-06-02  6:01   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 13/17] xfs: kill the unused xlog_debug variable Christoph Hellwig
2010-06-02  6:02   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 14/17] xfs: remove the unused XFS_LOG_SLEEP and XFS_LOG_NOSLEEP flags Christoph Hellwig
2010-06-02  6:02   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 15/17] xfs: remove the unused XFS_TRANS_NOSLEEP/XFS_TRANS_WAIT flags Christoph Hellwig
2010-06-02  6:03   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 16/17] xfs: remove unused XFS_BMAPI_ flags Christoph Hellwig
2010-06-02  6:04   ` Dave Chinner
2010-05-31 16:07 ` [PATCH 17/17] xfs: remove unused delta tracking code in xfs_bmapi Christoph Hellwig
2010-06-02  6:11   ` Dave Chinner
2010-06-02  6:13 ` [PATCH 00/17] pending patches Dave Chinner
2010-06-03 17:01 ` Alex Elder
2010-06-03 17:08   ` dropping dmapi support, was " Christoph Hellwig
2010-06-03 22:33     ` Dave Chinner

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=20100603065252.GA28592@infradead.org \
    --to=hch@infradead.org \
    --cc=david@fromorbit.com \
    --cc=linux-mm@kvack.org \
    --cc=xfs@oss.sgi.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.