xfs
[Top] [All Lists]

Re: [PATCH block/for-linus] writeback: fix syncing of I_DIRTY_TIME inode

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH block/for-linus] writeback: fix syncing of I_DIRTY_TIME inodes
From: Tejun Heo <tj@xxxxxxxxxx>
Date: Mon, 24 Aug 2015 14:10:38 -0400
Cc: Eryu Guan <eguan@xxxxxxxxxx>, Jens Axboe <axboe@xxxxxxxxx>, Jan Kara <jack@xxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, axboe@xxxxxx, Jan Kara <jack@xxxxxxxx>, linux-fsdevel@xxxxxxxxxxxxxxx, kernel-team@xxxxxx
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=p/fgajdvmUaoB7Hl5Vebo/ZH3nbu7AkMmI4+OCiQVeY=; b=GWirfu2jmi48q6jOdZ6MPgvMYW8TaWgIVg823O2GRwnLDMmOlYVj2s1KcvYEwGlzMP vKxjKQy/tDzVzU7WvfPlzPG2awO15lCGgWklVunk1tFgUFFXZRhIj4WRnpgV8CZy3STt Sk50epOtPh3+HmSS7hspX6AeUBPHCSg6gMBxSli5gEC24ppSElS2d0xCIhPbkMgeFfSs 2954rfunV5RSK526XEqgyGaumFixYAIe/AvWNmUFG9YGAh0CMJr4o4V07XLV0eYuB/g1 UHrSRIeBu+h4i976s5olnaj5o6YfDhqG9QOiB3ejsYvzqx+2ogTcLahMgNhpTthn04HO kRaQ==
In-reply-to: <20150820230451.GT714@dastard>
References: <20150817200254.GG21075@xxxxxxxxxxxxxxx> <20150818091603.GA12317@xxxxxxxxxxxxx> <20150818174718.GA15739@xxxxxxxxxxxxxxx> <20150818195439.GB15739@xxxxxxxxxxxxxxx> <20150818215611.GD3902@dastard> <20150820061224.GG17933@xxxxxxxxxxxxxxxxxxxxxxxxxx> <20150820143626.GI17933@xxxxxxxxxxxxxxxxxxxxxxxxxx> <20150820143735.GJ17933@xxxxxxxxxxxxxxxxxxxxxxxxxx> <20150820165537.GA2044@xxxxxxxxxxxxxxx> <20150820230451.GT714@dastard>
Sender: Tejun Heo <htejun@xxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
Hello, Dave.

On Fri, Aug 21, 2015 at 09:04:51AM +1000, Dave Chinner wrote:
> > Maybe I'm misunderstanding the code but all xfs_writepage() calls are
> > from unbound workqueues - the writeback workers - while
> > xfs_setfilesize() are from bound workqueues, so I wondered why that
> > was and looked at the code and the setsize functions are run off of a
> > separate work item which is queued from the end_bio callback and I
> > can't tell who would be waiting for them.  Dave, what am I missing?
> 
> xfs_setfilesize runs transactions, so it can't be run from IO
> completion context as it needs to block (i.e. on log space or inode
> locks). It also can't block log IO completion, nor metadata Io
> completion, as only log IO completion can free log space, and the
> inode lock might be waiting on metadata buffer IO completion (e.g.
> during delayed allocation). Hence we have multiple IO completion
> workqueues to keep these things separated and deadlock free. i.e.
> they all get punted to a workqueue where they are then processed in
> a context that can block safely.

I'm still a bit confused.  What prevents the following from happening?

1. io completion of last dirty page of an inode and work item for
   xfs_setfilesize() is queued.

2. inode removed from dirty list.

3. __sync_filesystem() invokes sync_inodes_sb().  There are no dirty
   pages, so it finishes.

4. xfs_fs_sync_fs() is called which calls _xfs_log_force() but the
   work item from #1 hasn't run yet, so the size update isn't written
   out.

5. Crash.

Is it that _xfs_log_force() waits for the setfilesize transaction
created during writepage?

Thanks.

-- 
tejun

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