Re: [PATCH] Fix queueing work if !bdi_cap_writeback_dirty()

[Fengguang Wu <fengguang.wu@intel.com>]

On Fri, Sep 14, 2012 at 10:07:48PM +0900, OGAWA Hirofumi wrote:
> Fengguang Wu <fengguang.wu@intel.com> writes:
> 
> >> The writeback task is always called with sync_mode != WB_SYNC_ALL except
> >> sync_inodes_sb(). But FS has sb->s_op->sync_fs() handler for
> >> sync_inodes_sb() path. So, writeback task just bothers FS to control to
> >> flush.
> >> 
> >> Also it wants to control the reclaimable of inode cache too, because FS
> >> have to control to flush, and wants to use inode in own FS task, and it
> >> knows when inode is cleaned and can be reclaimed.
> >> 
> >> I thought there are 2 options - 1) pin inode with iget(), and iput() on
> >> own FS task, 2) disable writeback task and care about inode reclaim by
> >> dirty flags.
> >> 
> >> (1) was complex (e.g. inode can be the orphan inode), and seems to be
> >> ineffective workaround to survive with writeback task.
> >
> > In principle, the VFS should of course give enough flexibility for the
> > FS. But it's all about the details that matter. As for the
> > BDI_CAP_NO_WRITEBACK approach, I'm afraid you'll not get the expected
> > "FS control" through it. Because the flusher thread may already have a
> > long queue of works which will take long time to finish. It even have
> > its internal background/periodic works that's not controllable this
> > way, see wb_check_background_flush().
> 
> wb_check_background_flush() is called from,
> 
> bdi_forker_thread()
>     bdi_writeback_thread()
>         wb_do_writeback()
>             wb_check_background_flush()
> 
> But, bdi_forker_thread() never start bdi_writeback_thread() if
> !bdi_cap_writeback_dirty(bdi).
> 
> Or I'm seeing something wrong here?

Nothing wrong.

> > And BDI_CAP_NO_WRITEBACK is expected to be a static/constant flag that
> > always evaluate to true/false for a given bdi.  There will be
> > correctness problems if you change the BDI_CAP_NO_WRITEBACK flag
> > dynamically.
> 
> I'm going to use it as static or per-sb by initialized in
> fill_super(). And it uses always BDI_CAP_NO_WRITEBACK if sb is
> available. Because own FS task flush instead.

Ah OK, sorry I didn't quite catch your use case.

But then if you set BDI_CAP_NO_WRITEBACK in the beginning, how come
__bdi_start_writeback() will be called at all?

Thanks,
Fengguang