Dave Chinner wrote:
> 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:
>> 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.
Oops. I blame my Left-handed mouse clicking skills, which are
improving but are still unreliable. I'm working mostly without
my Right hand this week because of an injury...
> 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).
Yes, I see that now. I had marked your older one "Reviewed" and
didn't look further. I will re-do the port and will re-submit.
BTW I also plan to port the perag radix tree patch(es) over this
for you after I get this one published.
> 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:
Yes, I agree. I find it helpful when updates are flagged with
something like "[PATCH v2]" because it somehow catches my eye
better *and* makes it explicit that the message contains a patch
that obsoletes a previous one.