[Top] [All Lists]

Re: [PATCH v2] xfs: Don't flush stale inodes

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCH v2] xfs: Don't flush stale inodes
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Sat, 9 Jan 2010 11:09:27 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1AB9A794DBDDF54A8A81BE2296F7BDFE012A693A@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
References: <1262399980-19277-1-git-send-email-david@xxxxxxxxxxxxx> <1AB9A794DBDDF54A8A81BE2296F7BDFE012A693A@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
On Fri, Jan 08, 2010 at 05:14:25PM -0600, Alex Elder wrote:
> Dave Chinner wrote:
> > Because inodes remain in cache much longer than inode buffers do
> > under memory pressure, we can get the situation where we have stale,
> > dirty inodes being reclaimed but the backing storage has been freed.
> > Hence we should never, ever flush XFS_ISTALE inodes to disk as
> > there is no guarantee that the backing buffer is in cache and
> > still marked stale when the flush occurs.
> >
> > Signed-off-by: Dave Chinner <david@xxxxxxxxxxxxx>
> Dave, I'm putting this patch in before your perag radix tree
> patch series, so I had to port it.  I am submitting here on
> your behalf, but would like you to review it if possible.
> It builds fine, and I've run it through a quick test.
> I will retain all of your original commentary, etc.  I did
> *not* implement Christoph's recommendation that you change
> the "for" loop in xfs_alloc_search_busy() to a more typical
> form.
> For reference, the original patch is here:
>     http://patchwork.xfs.org/patch/382/
> Signed-off-by: Alex Elder <aelder@xxxxxxx>

Ah, which patch are you talking about? The title says stale inodes,
the patch is searching busy extents.

Assuming you meant the busy extent search fix, you've ported the
wrong version of the busy extent search fix. I posted an updated
version after Christoph reviewed that one (in my response to
Christoph's review).

Because it was a new patch, patchwork doesn't add the email to the
original thread - it starts a new one. It's a real PITA, IMO, but
that is the way patchwork does stuff. The patch that should be
checked in is this one:



Dave Chinner

<Prev in Thread] Current Thread [Next in Thread>