Message-ID: <1334299108.2544.68.camel@sauron.fi.intel.com>
Date:	Fri, 13 Apr 2012 09:38:28 +0300
From:	Artem Bityutskiy <dedekind1@...il.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>,
	Linux Kernel Maling List <linux-kernel@...r.kernel.org>,
	Linux FS Maling List <linux-fsdevel@...r.kernel.org>,
	Jens Axboe <axboe@...nel.dk>, Al Viro <viro@...IV.linux.org.uk>
Subject: Re: [PATCH v1 0/3] do not use s_dirt in FAT FS

Hi Andrew, thanks for feed-back! CCing Al.

On Thu, 2012-04-12 at 17:12 -0700, Andrew Morton wrote:
> hm, I hadn't actually noticed the arrival of the sync_supers kernel
> thread.

It arrived when Jens added per-block device BDI threads.

> I don't think that the old scheme of calling sync_supers() via the
> writeback (kudate) code was really appropriate.

Sure.

> > TODO: affs, exofs, hfs, hfsplus, jffs2, reiserfs, sysv, udf, ufs
> 
> That's a lot of work.

Well, may be. But notice:

1. These are baroque file-systems. Modern ones do not use this: xfs,
   btrfs, jfs. Jan Kara has patches which make ext2 stop using
   '->write_super()'. We also sent patches to Ted which make ext4 stop
   using it when it is in non-journal mode (in journal mode it does not
   use it already).

2. Some FSes in the list may not even need '->write_super()'.

Indeed, many file-systems use '->write_super()' to write-out
non-essential optional information, which can be done opportunistically
on unmount/remount/sync_fs instead.

In fact, I think FAT FS is also among those. We do not really need any
timer and it is enough to write out the FSINFO block opportunistically.
Or we may do this in 'fat_write_inode()' when we are writing out the
'fat_inode' which represent the FAT table - seems to make sense because
the FSINFO contains only 2 useful pieces of information about the FAT
table: free clusters and the next free cluster.

I will try this approach.

> I don't really like the implementation.  It implies that we'll be
> copying and pasting quite a lot of boilerplate delayed-work code into
> many different filesystems.  Surely there is a way of hoisting the
> common code up into the vfs library layer.

So basically, my point is:

s/many different filesystems/several baroque file-systems/

> That implies that we retain ->write_super, probably in a modified form.
> Modified to permit the VFS to determine whether the superblock needs
> treatment, if ->s_dirt doesn't suffice.

I tried this approach and it was vetoed by Al Viro. Although it is
simpler to me to resurrect my old patches, I agree with Al that killing
'->write_super()' is a better approach. We do not want to serve a whole
kernel thread in the generic code for few baroque citizens.

Please, see this thread for the reference:
http://lkml.org/lkml/2010/7/22/96

> The code as you've proposed it will cause more wakeups than needed - if
> multiple filesystems are mounted and active, their timers will get out
> of sync.  Which rather defeats the intent of the whole work!  This
> problem should be addressable via some new centralised way of managing
> things.

I do not think this is an issue. If we have many file-systems, and all
of them are actively used so that the super block becomes dirty, which
most probably means there is also write-back - so be it, it is ok to arm
many timers. And if we make them deferrable for most of the FSes (which
we can not do for the generic timer, because we do not know FSes needs)
- then this is not an issue at all.

Also, if you look at this from the angle that only few old FSes will
have this, it becomes not that bad. I assume I will change this
patch-set and won't use delayed works here.

-- 
Best Regards,
Artem Bityutskiy

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)

Powered by blists - more mailing lists