xfs
[Top] [All Lists]

Re: [PATCH v2] Use atomic_t and wait_event to track dquot pincount

To: Lachlan McIlroy <lachlan@xxxxxxx>
Subject: Re: [PATCH v2] Use atomic_t and wait_event to track dquot pincount
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Sat, 27 Sep 2008 11:08:50 +1000
Cc: Peter Leckie <pleckie@xxxxxxx>, xfs@xxxxxxxxxxx, xfs-dev@xxxxxxx
In-reply-to: <48DC3A4E.7010602@xxxxxxx>
Mail-followup-to: Lachlan McIlroy <lachlan@xxxxxxx>, Peter Leckie <pleckie@xxxxxxx>, xfs@xxxxxxxxxxx, xfs-dev@xxxxxxx
References: <48D9C1DD.6030607@xxxxxxx> <48D9EB8F.1070104@xxxxxxx> <48D9EF6E.8010505@xxxxxxx> <20080924074604.GK5448@disturbed> <48D9F718.4010905@xxxxxxx> <20080925010318.GB27997@disturbed> <48DB4F3F.8040307@xxxxxxx> <20080926003401.GG27997@disturbed> <48DC3638.3050601@xxxxxxx> <48DC3A4E.7010602@xxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
On Fri, Sep 26, 2008 at 11:26:38AM +1000, Lachlan McIlroy wrote:
> Peter Leckie wrote:
>> Dave Chinner wrote:
>>> but it doesn't fix the underlying problem that was causing the
>>> spurious wakeups, which is the fact that xfs_qm_dqflush() is not
>>> obeying non-blocking flush directions. The patch below should fix
>>> that. Can you please test it before you add your patch?
>>>   
>> Yeah I already had this idea I  just have not posted a patch because  
>> Lachlan though
>> it might introduce a deadlock.
> I suggested some changes a while back to make tail pushing non-blocking
> and Dave thought it might cause a deadlock.
>
> http://oss.sgi.com/archives/xfs/2008-07/msg00472.html

That is a different case - the aborting of writeback due to a locked
inode cluster buffer could be problematic for the AIL code
because it already has special code to handle cluster buffer pushing
in the case of DELWRI flushed inodes.

The case I described is what the xfsaild "watchdog timeout" really
catches - before the aild the filesystem would simply lock up, and
one way to trigger that was to have the AIL traversal restart too
many times without making progress. The AIL cursor patch series I
posted fixes the excessive restart problem, but doesn't prevent
problem from occurring if the async push doesn't actually write the
item back in IOP_PUSH(). Effectively we need to tweak the ail
push-wait-push loops in the log grant code to avoid this problem.

FWIW, IOP_TRYLOCK() will return ITEM_PINNED for any object
that is still pinned, and hence the AIL does not call IOP_PUSH()
for such items. Instead it schedules a non-blocking log force
for the end of the traversal to get things moving for the next
push to flush it out. Hence the AIL pushing should never, ever be
trying to push a pinned inode or dquot to disk, and hence the
proposed change will not affect AIL pushing at all....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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