[PATCH take 2] UBIFS - new flash file system

[PATCH take 2] UBIFS - new flash file system

[Artem Bityutskiy]

Hello,

here is the third round of UBIFS submission. If you are not aware what
UBIFS is about, please refer Jonathan Corbet's article:
http://lwn.net/Articles/276025/

Also, please refer the second and the first UBIFS submissions which
contain short UBIFS description and URLs.
second: http://marc.info/?l=linux-kernel&m=121006384211288&w=2
first: http://marc.info/?l=linux-kernel&m=120662475821992&w=2

We've addressed most of the requests from the second round:

* Requests from Christoph Hellwig's review (may be found here:
  http://marc.info/?l=linux-kernel&m=121093446502796&w=2). Few things
  related to the background thread were not done though. Here are
  explanations why: http://marc.info/?l=linux-kernel&m=121155640026661&w=2
  and http://marc.info/?l=linux-kernel&m=121119680522445&w=2 (see end
  of the mail).
  Also, we have not changed readdir() implementation so far, but just
  put a comment that we cannot support NFS at the moment. However, we
  have an idea how to fix this, but need some comments from the community.
  We'll send a separate mail describing the problem and the possible
  solution shortly. Nevertheless, this should not be a blocker.
* The 'generic_sync_sb_inodes()' was introduce as it was pointed to by
  Andrew Morton.
* Many build fixes spotted by Andrew Morton.
* Get rid of some dead code spotted by Marcin Slusarz.

Other news is that UBIFS has been sitting in -mm tree for a couple of
weeks already. And of course we have got more bug reports and fixed
more bugs since the last submission.

Christoph prefers tarballs - here it is:
http://www.infradead.org/~dedekind/ubifs/ubifs-08-05-26.tar.bz2

Changes between take 1 and take 2:
* Store milliseconds for [mca]time stamps in the inodes, not only seconds
  (requested by Andi Kleen)
* Documentation/filesystems/ubifs.txt has been added (requested by Josh Boyer)
* do_div() is not anymore used with 'unsigned long long' values
  (requested by Andrew Morton)
* The amount of debugging-related config options was lessened and module
  parameters were introduced instead (requested by Pekka Enberg and
  Christoph Hellwig)
* Many assertions were removed (requested by Pekka Enberg)
* Some debugging checks like custom memory leak and memory pressure checks
  were removed (requested by Pekka Enberg)

Thank you,
Adrian Hunter
Artem Bityutskiy

P.S. The patches are against v2.6.26-rc3.

git-diff --stat --summary -M v2.6.26-rc3
 Documentation/filesystems/ubifs.txt |  163 ++
 fs/Kconfig                          |    3 +
 fs/Makefile                         |    1 +
 fs/fs-writeback.c                   |   22 +-
 fs/ubifs/Kconfig                    |   71 +
 fs/ubifs/Makefile                   |    9 +
 fs/ubifs/budget.c                   |  859 +++++++++
 fs/ubifs/commit.c                   |  718 ++++++++
 fs/ubifs/compress.c                 |  253 +++
 fs/ubifs/debug.c                    | 1486 ++++++++++++++++
 fs/ubifs/debug.h                    |  392 ++++
 fs/ubifs/dir.c                      | 1016 +++++++++++
 fs/ubifs/file.c                     |  960 ++++++++++
 fs/ubifs/find.c                     |  956 ++++++++++
 fs/ubifs/gc.c                       |  761 ++++++++
 fs/ubifs/io.c                       |  921 ++++++++++
 fs/ubifs/ioctl.c                    |  212 +++
 fs/ubifs/journal.c                  | 1275 ++++++++++++++
 fs/ubifs/key.h                      |  532 ++++++
 fs/ubifs/log.c                      |  799 +++++++++
 fs/ubifs/lprops.c                   | 1353 ++++++++++++++
 fs/ubifs/lpt.c                      | 2241 +++++++++++++++++++++++
 fs/ubifs/lpt_commit.c               | 1631 +++++++++++++++++
 fs/ubifs/master.c                   |  387 ++++
 fs/ubifs/misc.h                     |  310 ++++
 fs/ubifs/orphan.c                   |  955 ++++++++++
 fs/ubifs/recovery.c                 | 1509 ++++++++++++++++
 fs/ubifs/replay.c                   | 1009 +++++++++++
 fs/ubifs/sb.c                       |  609 +++++++
 fs/ubifs/scan.c                     |  362 ++++
 fs/ubifs/shrinker.c                 |  322 ++++
 fs/ubifs/super.c                    | 1956 ++++++++++++++++++++
 fs/ubifs/tnc.c                      | 3330 +++++++++++++++++++++++++++++++++++
 fs/ubifs/tnc_commit.c               | 1105 ++++++++++++
 fs/ubifs/tnc_misc.c                 |  259 +++
 fs/ubifs/ubifs-media.h              |  725 ++++++++
 fs/ubifs/ubifs.h                    | 1576 +++++++++++++++++
 fs/ubifs/xattr.c                    |  582 ++++++
 include/linux/fs.h                  |    2 +
 init/do_mounts.c                    |    3 +-
 40 files changed, 31624 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/filesystems/ubifs.txt
 create mode 100644 fs/ubifs/Kconfig
 create mode 100644 fs/ubifs/Makefile
 create mode 100644 fs/ubifs/budget.c
 create mode 100644 fs/ubifs/commit.c
 create mode 100644 fs/ubifs/compress.c
 create mode 100644 fs/ubifs/debug.c
 create mode 100644 fs/ubifs/debug.h
 create mode 100644 fs/ubifs/dir.c
 create mode 100644 fs/ubifs/file.c
 create mode 100644 fs/ubifs/find.c
 create mode 100644 fs/ubifs/gc.c
 create mode 100644 fs/ubifs/io.c
 create mode 100644 fs/ubifs/ioctl.c
 create mode 100644 fs/ubifs/journal.c
 create mode 100644 fs/ubifs/key.h
 create mode 100644 fs/ubifs/log.c
 create mode 100644 fs/ubifs/lprops.c
 create mode 100644 fs/ubifs/lpt.c
 create mode 100644 fs/ubifs/lpt_commit.c
 create mode 100644 fs/ubifs/master.c
 create mode 100644 fs/ubifs/misc.h
 create mode 100644 fs/ubifs/orphan.c
 create mode 100644 fs/ubifs/recovery.c
 create mode 100644 fs/ubifs/replay.c
 create mode 100644 fs/ubifs/sb.c
 create mode 100644 fs/ubifs/scan.c
 create mode 100644 fs/ubifs/shrinker.c
 create mode 100644 fs/ubifs/super.c
 create mode 100644 fs/ubifs/tnc.c
 create mode 100644 fs/ubifs/tnc_commit.c
 create mode 100644 fs/ubifs/tnc_misc.c
 create mode 100644 fs/ubifs/ubifs-media.h
 create mode 100644 fs/ubifs/ubifs.h
 create mode 100644 fs/ubifs/xattr.c

Re: [PATCH take 2] UBIFS - new flash file system

[Artem Bityutskiy]

Artem Bityutskiy wrote:
> Hello,
> 
> here is the third round of UBIFS submission. If you are not aware what
> UBIFS is about, please refer Jonathan Corbet's article:
> http://lwn.net/Articles/276025/

Sorry, ignore this mail. Just sent the patches with correct
subject.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

[PATCH take 2] UBIFS - new flash file system

[Artem Bityutskiy]

Hello,

here is the second round of UBIFS submission. Please, refer the following
URL if you are not familiar what is UBIFS about:

http://kerneltrap.org/mailarchive/linux-kernel/2008/3/27/1273514

Additionally, there is a excellent introductional LWN article written by
Jonathan Corbet:
http://lwn.net/Articles/276025/
(thanks Jonathan).

We've addressed most of the requests from the first round and the following
is a rough change log:
* Store milliseconds for [mca]time stamps in the inodes, not only seconds
  (requested by Andi Kleen)
* Documentation/filesystems/ubifs.txt has been added (requested by Josh Boyer)
* do_div() is not anymore used with 'unsigned long long' values
  (requested by Andrew Morton)
* The amount of debugging-related config options was lessened and module
  parameters were introduced instead (requested by Pekka Enberg and
  Christoph Hellwig)
* Many assertions were removed (requested by Pekka Enberg)
* Some debugging checks like custom memory leak and memory pressure checks
  were removed (requested by Pekka Enberg)

Besides, several bugs were fixed both in UBIFS and mkfs.ubifs. The on-flash
format has also been changed because of the inode time-stamp format changes
(milliseconds were added).

We've got few users outside of Nokia who seem to be happy with UBIFS and
utilize it. There were some positive feed-backs from the community, e.g.
http://kerneltrap.org/mailarchive/linux-kernel/2008/4/18/1466334
(thanks Thomas).

We kindly ask for more review and feed-back. We'd also like to get into
Andrew's mm tree.

Thank you,
Adrian Hunter
Artem Bityutskiy

git-diff --stat --summary -M v2.6.25
 Documentation/filesystems/ubifs.txt |  163 ++
 fs/Kconfig                          |    3 +
 fs/Makefile                         |    1 +
 fs/fs-writeback.c                   |    8 +
 fs/ubifs/Kconfig                    |   47 +
 fs/ubifs/Kconfig.debug              |   30 +
 fs/ubifs/Makefile                   |    9 +
 fs/ubifs/budget.c                   |  857 +++++++++
 fs/ubifs/build.c                    | 1374 +++++++++++++++
 fs/ubifs/commit.c                   |  709 ++++++++
 fs/ubifs/compress.c                 |  253 +++
 fs/ubifs/debug.c                    | 1463 ++++++++++++++++
 fs/ubifs/debug.h                    |  392 +++++
 fs/ubifs/dir.c                      |  978 +++++++++++
 fs/ubifs/file.c                     |  902 ++++++++++
 fs/ubifs/find.c                     |  957 +++++++++++
 fs/ubifs/gc.c                       |  773 +++++++++
 fs/ubifs/io.c                       |  921 ++++++++++
 fs/ubifs/ioctl.c                    |  205 +++
 fs/ubifs/journal.c                  | 1264 ++++++++++++++
 fs/ubifs/key.h                      |  507 ++++++
 fs/ubifs/log.c                      |  799 +++++++++
 fs/ubifs/lprops.c                   | 1353 +++++++++++++++
 fs/ubifs/lpt.c                      | 2243 ++++++++++++++++++++++++
 fs/ubifs/lpt_commit.c               | 1625 ++++++++++++++++++
 fs/ubifs/master.c                   |  387 +++++
 fs/ubifs/misc.h                     |  310 ++++
 fs/ubifs/orphan.c                   |  955 +++++++++++
 fs/ubifs/recovery.c                 | 1439 ++++++++++++++++
 fs/ubifs/replay.c                   | 1008 +++++++++++
 fs/ubifs/sb.c                       |  612 +++++++
 fs/ubifs/scan.c                     |  362 ++++
 fs/ubifs/shrinker.c                 |  322 ++++
 fs/ubifs/super.c                    |  525 ++++++
 fs/ubifs/tnc.c                      | 3240 +++++++++++++++++++++++++++++++++++
 fs/ubifs/tnc_commit.c               | 1097 ++++++++++++
 fs/ubifs/tnc_misc.c                 |  259 +++
 fs/ubifs/ubifs-media.h              |  719 ++++++++
 fs/ubifs/ubifs.h                    | 1563 +++++++++++++++++
 fs/ubifs/xattr.c                    |  582 +++++++
 include/linux/writeback.h           |    1 +
 init/do_mounts.c                    |    3 +-
 42 files changed, 31219 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/filesystems/ubifs.txt
 create mode 100644 fs/ubifs/Kconfig
 create mode 100644 fs/ubifs/Kconfig.debug
 create mode 100644 fs/ubifs/Makefile
 create mode 100644 fs/ubifs/budget.c
 create mode 100644 fs/ubifs/build.c
 create mode 100644 fs/ubifs/commit.c
 create mode 100644 fs/ubifs/compress.c
 create mode 100644 fs/ubifs/debug.c
 create mode 100644 fs/ubifs/debug.h
 create mode 100644 fs/ubifs/dir.c
 create mode 100644 fs/ubifs/file.c
 create mode 100644 fs/ubifs/find.c
 create mode 100644 fs/ubifs/gc.c
 create mode 100644 fs/ubifs/io.c
 create mode 100644 fs/ubifs/ioctl.c
 create mode 100644 fs/ubifs/journal.c
 create mode 100644 fs/ubifs/key.h
 create mode 100644 fs/ubifs/log.c
 create mode 100644 fs/ubifs/lprops.c
 create mode 100644 fs/ubifs/lpt.c
 create mode 100644 fs/ubifs/lpt_commit.c
 create mode 100644 fs/ubifs/master.c
 create mode 100644 fs/ubifs/misc.h
 create mode 100644 fs/ubifs/orphan.c
 create mode 100644 fs/ubifs/recovery.c
 create mode 100644 fs/ubifs/replay.c
 create mode 100644 fs/ubifs/sb.c
 create mode 100644 fs/ubifs/scan.c
 create mode 100644 fs/ubifs/shrinker.c
 create mode 100644 fs/ubifs/super.c
 create mode 100644 fs/ubifs/tnc.c
 create mode 100644 fs/ubifs/tnc_commit.c
 create mode 100644 fs/ubifs/tnc_misc.c
 create mode 100644 fs/ubifs/ubifs-media.h
 create mode 100644 fs/ubifs/ubifs.h
 create mode 100644 fs/ubifs/xattr.c

Re: [PATCH take 2] UBIFS - new flash file system

[Christoph Hellwig]

Do you have the filesystem as a single tarball or patch?  This cancer
of split-up patches that's spreading quickly all over the linux world
makes sensible reviews quite hard unfortunately.

Re: [PATCH take 2] UBIFS - new flash file system

[Artem Bityutskiy]

Christoph Hellwig wrote:
> Do you have the filesystem as a single tarball or patch?  This cancer
> of split-up patches that's spreading quickly all over the linux world
> makes sensible reviews quite hard unfortunately.

Will git tree work?
http://www.linux-mtd.infradead.org/doc/ubifs.html#L_source

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH take 2] UBIFS - new flash file system

[Christoph Hellwig]

On Wed, May 07, 2008 at 11:07:47AM +0300, Artem Bityutskiy wrote:
> Christoph Hellwig wrote:
>> Do you have the filesystem as a single tarball or patch?  This cancer
>> of split-up patches that's spreading quickly all over the linux world
>> makes sensible reviews quite hard unfortunately.
>
> Will git tree work?
> http://www.linux-mtd.infradead.org/doc/ubifs.html#L_source

A git tree of just ubifs would be perfect, but the full tree does it
aswell.  Not really conveniant, but much better than split patches :)

Re: [PATCH take 2] UBIFS - new flash file system

[Artem Bityutskiy]

ext Christoph Hellwig wrote:
> On Wed, May 07, 2008 at 11:07:47AM +0300, Artem Bityutskiy wrote:
>> Christoph Hellwig wrote:
>>> Do you have the filesystem as a single tarball or patch?  This cancer
>>> of split-up patches that's spreading quickly all over the linux world
>>> makes sensible reviews quite hard unfortunately.
>> Will git tree work?
>> http://www.linux-mtd.infradead.org/doc/ubifs.html#L_source
> 
> A git tree of just ubifs would be perfect, but the full tree does it
> aswell.  Not really conveniant, but much better than split patches :)

Wait a little, I'll prepare current linux-2.6.git + ubifs tree.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH take 2] UBIFS - new flash file system

[Artem Bityutskiy]

Christoph Hellwig wrote:
>> Will git tree work?
>> http://www.linux-mtd.infradead.org/doc/ubifs.html#L_source
> 
> A git tree of just ubifs would be perfect, but the full tree does it
> aswell.  Not really conveniant, but much better than split patches :)

So as it is said at the above URL,
git://git.infradead.org/~dedekind/ubifs-2.6.git is the current
development tree. At the moment it is current Linus's tree + UBIFS.

git-diff a15306365a16380f3bafee9e181ba01231d4acd7 would give UBIFS patch.

git://git.infradead.org/~dedekind/ubifs-v2.6.25.git has v2.6.25 + UBIFS.

I guess this should be ok.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH take 2] UBIFS - new flash file system

[Artem Bityutskiy]

Christoph Hellwig wrote:
> A git tree of just ubifs would be perfect, but the full tree does it
> aswell.  Not really conveniant, but much better than split patches :)

Christoph,

Here is a tarball for you:
http://www.infradead.org/~dedekind/ubifs/ubifs-08-05-07.tar.bz2

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH take 2] UBIFS - new flash file system

[Christoph Hellwig]

General comment:

 - not supporting a flash page size different from the system page
   size is a horrible thing for people trying to use the same storage
   on multiple systems.  For a block based filesystem that alone would
   be enough reason not to merge it.  For a flash filesystem I'm not
   entirely sure given that flash isn't moved between systems all that
   often.

VFS/VM interaction comments:

 - splitting most of the mount code out to build.c is rather odd.
   Most filesystems have this in super.c
 - calling convention for mount_ubifs is nasty because it doesn't
   clean up it's own errors.  You might think it's easier to do
   all that in ubifs_umount but beeing in the process of untangling
   that mess for xfs I'd recomment against it.  Unless there's
   a very good reason for it functions should always clean up
   the resources they allocated in the error case.
 - ubifs_get_sb would benefit from splitting out a ubifs_fill_super
   routine that allocates a new sb when it's actually needed.
 - why do you do the commit_on_unmount in your own ->shutdown_super
   instead of the normal ->put_super?  If there's a good reason
   this at least needs a big comment explaining why.
 - ubifs_lookup doesn't really need to use d_splice_alias unless
   you want to support nfs exporting
 - in ubifs_new_inode you inherit the inode flags from the parent.
   This probably wants splitting out in a helper that documents
   explicitly what flags are inherited and what not.  Given that
   you store the general indoe flags settable by chattr in there
   it seems like a bad idea to inherit them by default.
 - the read dir implementation won't ever support nfs exporting
   due to having to keep per open file state.  Nor would it support
   thing like checkpoint and restart.
 - just opencode you mmap routine, there's nothing helpful
   in generic_file_mmap if you set your own vm_ops.
 - ubifs_trunc should never be called on anything but a regular
   file, so the check for it seems superflous.  Having it after
   the S_ISREG is rather odd too even if you want to have
   an assertation.
 - please implement the unlocked_ioctl file operation instead of
   the old ioctl operation that comes with the BKL locked.

Misc comments:

 - ubifs_bg_thread shouldn't set a nice level, especially when it's the
   default one anyway.
 - the mainoop of ubifs_bg_thread looks a bit odd either, when you
   first to an interruotible sleep and then possible one while you
   still have TASK_RUNNING set.  Also the need_bgt flag is not needed
   because the thrad is only woken up to perform it's action.
   In the end the main loop should look something like:

	while (1) {
		if (kthread_should_stop())
			break;
		if (try_to_freeze())
			continue;

		run_bg_commit(c);

		set_current_state(TASK_INTERRUPTIBLE);
		schedule();
		__set_current_state(TASK_RUNNING);
	}

			


Same comments on naming in the code:

 - bgt is not very descriptive for your kernel thread.  These are per
   defintion in the background, so just call them thread or
   <somethinginformative>_thread.  In this case it would probably
   be commit_thread.
 - any chance you could spell out journal instead of jrn?  jrn always
   sounds like joern with the wovel eaten by a mailer.. :)

Re: [PATCH take 2] UBIFS - new flash file system

[Adrian Hunter]

Thanks for the review.

Christoph Hellwig wrote:
> General comment:
> 
>  - not supporting a flash page size different from the system page
>    size is a horrible thing for people trying to use the same storage
>    on multiple systems.  For a block based filesystem that alone would
>    be enough reason not to merge it.  For a flash filesystem I'm not
>    entirely sure given that flash isn't moved between systems all that
>    often.

We are working on that.  It will be added next week probably.

> VFS/VM interaction comments:
> 
>  - splitting most of the mount code out to build.c is rather odd.
>    Most filesystems have this in super.c

We will get rid of build.c and move everything to super.c.

>  - calling convention for mount_ubifs is nasty because it doesn't
>    clean up it's own errors.  You might think it's easier to do
>    all that in ubifs_umount but beeing in the process of untangling
>    that mess for xfs I'd recomment against it.  Unless there's
>    a very good reason for it functions should always clean up
>    the resources they allocated in the error case.

Ok

>  - ubifs_get_sb would benefit from splitting out a ubifs_fill_super
>    routine that allocates a new sb when it's actually needed.

Ok

>  - why do you do the commit_on_unmount in your own ->shutdown_super
>    instead of the normal ->put_super?  If there's a good reason
>    this at least needs a big comment explaining why.

It is in ->shutdown_super to be outside BKL.  Will add comment.

>  - ubifs_lookup doesn't really need to use d_splice_alias unless
>    you want to support nfs exporting

Well we would like to support nfs one day, so maybe we could just
leave it?

>  - in ubifs_new_inode you inherit the inode flags from the parent.
>    This probably wants splitting out in a helper that documents
>    explicitly what flags are inherited and what not.  Given that
>    you store the general indoe flags settable by chattr in there
>    it seems like a bad idea to inherit them by default.

Ok

>  - the read dir implementation won't ever support nfs exporting
>    due to having to keep per open file state.  Nor would it support
>    thing like checkpoint and restart.

We could get rid of the per-open-file state if we could come up
with a scheme that would make fpos unique.  At the moment it is
not unique because it is the name hash, but we could perhaps
allocate a unique number to each colliding entry.  We will
think some more about this.

>  - just opencode you mmap routine, there's nothing helpful
>    in generic_file_mmap if you set your own vm_ops.

Ok

>  - ubifs_trunc should never be called on anything but a regular
>    file, so the check for it seems superflous.  Having it after
>    the S_ISREG is rather odd too even if you want to have
>    an assertation.

Ok

>  - please implement the unlocked_ioctl file operation instead of
>    the old ioctl operation that comes with the BKL locked.

Ok

> 
> Misc comments:
> 
>  - ubifs_bg_thread shouldn't set a nice level, especially when it's the
>    default one anyway.

Ok

>  - the mainoop of ubifs_bg_thread looks a bit odd either, when you
>    first to an interruotible sleep and then possible one while you
>    still have TASK_RUNNING set.  Also the need_bgt flag is not needed
>    because the thrad is only woken up to perform it's action.
>    In the end the main loop should look something like:
> 
> 	while (1) {
> 		if (kthread_should_stop())
> 			break;
> 		if (try_to_freeze())
> 			continue;
> 
> 		run_bg_commit(c);
> 
> 		set_current_state(TASK_INTERRUPTIBLE);
> 		schedule();
> 		__set_current_state(TASK_RUNNING);
> 	}

Ok

> 
> 			
> 
> 
> Same comments on naming in the code:
> 
>  - bgt is not very descriptive for your kernel thread.  These are per
>    defintion in the background, so just call them thread or
>    <somethinginformative>_thread.  In this case it would probably
>    be commit_thread.

Ok

>  - any chance you could spell out journal instead of jrn?  jrn always
>    sounds like joern with the wovel eaten by a mailer.. :)

It is named after Jœrn not journal ;-)

How about jnl after Janelle? It uses the same number of characters.

Re: [PATCH take 2] UBIFS - new flash file system

[Artem Bityutskiy]

Christoph,

thanks for the review.

Christoph Hellwig wrote:
>  - the read dir implementation won't ever support nfs exporting
>    due to having to keep per open file state.  Nor would it support
>    thing like checkpoint and restart.

We keep full name of the last readdir()'ed direntry in file->private_data
and in file->f_pos we store the direntry hash value (32 bits). The only reason
we store full name in file->private_data is name hash collisions.

We are able to restart readdir()s, but only if there are no hash collisions.
Otherwise UBIFS will restart, but probably from the wrong directory entry
(the first one in the colliding sequence).

In other words, you may do readdir, telldir, close the directory, open it
again, seekdir, and continue readdir-ing. This will mostly work as expected
unless there was hash collision, in which case you will restart from the
first direntry in the colliding sequence. I.e., telldir/seekdir mostly
work, but not always.

We thought seekdir/telldir was considered as horrible interface anyway so
we assumed it is not that big deal if it occasionally does not work as
expected.

AFAIK there are other FSes which have similar issues. Do all of them solve
this in one way or another? We have an idea of storing collision
number in directory entries and return this number in the high 32 bits of
file->f_pos (the low contain hash value) to make readdir truly restartable.
But this requires some non-trivial changes and we'd want to realize if it
is really needed.

AFAIK, even ext3 may shrink directories and fail to support readdir
restarts occasionally, but I am not 100% sure.

Could you please elaborate some more why UBIFS won't ever support nfs export?
Does NFS export need correct readdir restarts?

>  - ubifs_bg_thread shouldn't set a nice level, especially when it's the
>    default one anyway.

Well, it is also doing write-buffer flushes, not only commits. And we were
planning to add background garbage collection there as well.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH take 2] UBIFS - new flash file system

[Andi Kleen]

Artem Bityutskiy <Artem.Bityutskiy@nokia.com> writes:
>
> Could you please elaborate some more why UBIFS won't ever support nfs export?
> Does NFS export need correct readdir restarts?

Yes, because NFS is stateless. The seek offset has to be valid even over 
a reboot.

If you don't handle it you should probably better forbid nfs export
completely to avoid subtle problems.

-Andi

Re: [PATCH take 2] UBIFS - new flash file system

[Artem Bityutskiy]

Christoph Hellwig wrote:
>  - the mainoop of ubifs_bg_thread looks a bit odd either, when you
>    first to an interruotible sleep and then possible one while you
>    still have TASK_RUNNING set.  Also the need_bgt flag is not needed
>    because the thrad is only woken up to perform it's action.
>    In the end the main loop should look something like:
> 
> 	while (1) {
> 		if (kthread_should_stop())
> 			break;
> 		if (try_to_freeze())
> 			continue;
> 
> 		run_bg_commit(c);
> 
> 		set_current_state(TASK_INTERRUPTIBLE);
> 		schedule();
> 		__set_current_state(TASK_RUNNING);
> 	}

Err, how are we guaranteed that we do not miss wake-up events then,
if we were woken up say, between run_bg_commit(c) and
set_current_state(TASK_INTERRUPTIBLE) ? This was  the purpose
of the 'need_bgt' flag.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)