[PATCH take 2][RFC] fat: Save FAT root directory timestamps to volume label

[PATCH take 2][RFC] fat: Save FAT root directory timestamps to volume label

[Jorg Schummer]

Standard FAT implementations cannot store any of the FAT root directory's
timestamps. This commit adds the mount option 'rootts', which allows saving
the FAT root directory timestamps as the timestamps of the FAT volume label
directory entry. At least Mac OS X is known to support the same mechanism
and interoperate with this commit.

When mounting, the following values can be specified for the 'rootts' mount
option:

  "rootts=ignore"   ignores root directory timestamps. All timestamps are
                    reset to 0 (1/1/1970). This is the default.

  "rootts=load"     only tries to load the root directory's timestamps from
                    a volume label entry when mounting.

  "rootts=preserve" tries to load and save the root directory's timestamps
                    if a volume label entry exists.

  "rootts=save"     tries to load and save the root directory's timestamps.
                    If the root directory was accessed but no volume label
                    entry exists, the label "NO NAME" is created.

Signed-off-by: Jorg Schummer <ext-jorg.2.schummer@nokia.com>
---
 Documentation/filesystems/vfat.txt |   10 +++
 fs/fat/dir.c                       |   44 +++++++++++
 fs/fat/fat.h                       |   27 +++++++
 fs/fat/inode.c                     |  141 +++++++++++++++++++++++++-----------
 fs/fat/misc.c                      |   27 +++++++
 5 files changed, 207 insertions(+), 42 deletions(-)

diff --git a/Documentation/filesystems/vfat.txt b/Documentation/filesystems/vfat.txt
index b58b84b..aa048e5 100644
--- a/Documentation/filesystems/vfat.txt
+++ b/Documentation/filesystems/vfat.txt
@@ -137,6 +137,16 @@ errors=panic|continue|remount-ro
 		 without doing anything or remount the partition in
 		 read-only mode (default behavior).
 
+rootts=ignore|load|preserve|save
+	      -- Specify whether to load/store the root dir timestamps as the
+		 timestamp of the volume label entry.
+		 ignore:   do not load or store root dir ts (default).
+		 load:     only try to load the root dir ts.
+		 preserve: load the root dir ts, store them only if a volume
+			   label already exists.
+		 save:     load and store the root dir ts, create a volume label
+			   if the ts have changed but no label is present.
+
 <bool>: 0,1,yes,no,true,false
 
 TODO
diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 530b4ca..1a9b1e7 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -1362,3 +1362,47 @@ error_remove:
 }
 
 EXPORT_SYMBOL_GPL(fat_add_entries);
+
+int fat_get_label_entry(struct inode *root_inode, struct buffer_head **bh,
+			struct msdos_dir_entry **de)
+{
+	loff_t pos;
+
+	BUG_ON(root_inode->i_ino != MSDOS_ROOT_INO);
+
+	pos = 0;
+	*bh = NULL;
+	while (fat_get_entry(root_inode, &pos, bh, de) >= 0) {
+		/* volume label: note that it is not enough to check only
+		   whether the ATTR_VOLUME bit is set, since this would yield
+		   true on any vfat extended entry */
+		if ((*de)->attr != ATTR_EXT && ((*de)->attr & ATTR_VOLUME))
+			return 0;
+	}
+	return -ENOENT;
+}
+EXPORT_SYMBOL_GPL(fat_get_label_entry);
+
+int fat_create_label_entry(struct inode *root_inode,
+			   const unsigned char *name)
+{
+	struct msdos_dir_entry de;
+	struct fat_slot_info sinfo;
+	int err;
+
+	BUG_ON(root_inode->i_ino != MSDOS_ROOT_INO);
+
+	memcpy(de.name,
+	       (name) ? name : (const unsigned char *) FAT_LABEL_NONAME,
+	       MSDOS_NAME);
+	de.attr = ATTR_VOLUME;
+	de.lcase = 0;
+	de.start = de.starthi = 0;
+	de.size = 0;
+	fat_time_inode2de(MSDOS_SB(root_inode->i_sb), root_inode, &de);
+
+	err = fat_add_entries(root_inode, &de, 1, &sinfo);
+	brelse(sinfo.bh);
+	return err;
+}
+EXPORT_SYMBOL_GPL(fat_create_label_entry);
diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index adb0e72..69a91f9 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -21,6 +21,19 @@
 #define FAT_ERRORS_PANIC	2      /* panic on error */
 #define FAT_ERRORS_RO		3      /* remount r/o on error */
 
+/*
+ * fat root directory timestamp backup
+ */
+#define FAT_ROOT_TS_READ        0x01 /* read root dir ts from volume label ts */
+#define FAT_ROOT_TS_WRITE       0x02 /* write root dir ts to volume label ts */
+#define FAT_ROOT_TS_CREATE      0x04 /* create volume label if there is none */
+
+#define FAT_ROOT_TS_IGNORE      0
+#define FAT_ROOT_TS_LOAD        FAT_ROOT_TS_READ
+#define FAT_ROOT_TS_PRESERVE    (FAT_ROOT_TS_READ | FAT_ROOT_TS_WRITE)
+#define FAT_ROOT_TS_SAVE        (FAT_ROOT_TS_READ | FAT_ROOT_TS_WRITE | \
+				 FAT_ROOT_TS_CREATE)
+
 struct fat_mount_options {
 	uid_t fs_uid;
 	gid_t fs_gid;
@@ -32,6 +45,8 @@ struct fat_mount_options {
 	unsigned char name_check; /* r = relaxed, n = normal, s = strict */
 	unsigned char errors;	  /* On error: continue, panic, remount-ro */
 	unsigned short allow_utime;/* permission for setting the [am]time */
+	unsigned short root_ts;   /* root dir timestamps:
+				       ignore, load, preserve, save */
 	unsigned quiet:1,         /* set = fake successful chmods and chowns */
 		 showexec:1,      /* set = only set x bit for com/exe/bat */
 		 sys_immutable:1, /* set = system files are immutable */
@@ -50,6 +65,8 @@ struct fat_mount_options {
 #define FAT_HASH_BITS	8
 #define FAT_HASH_SIZE	(1UL << FAT_HASH_BITS)
 
+#define FAT_LABEL_NONAME "NO NAME    "
+
 /*
  * MS-DOS file system in-core superblock data
  */
@@ -247,6 +264,12 @@ extern int fat_add_entries(struct inode *dir, void *slots, int nr_slots,
 			   struct fat_slot_info *sinfo);
 extern int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo);
 
+extern int fat_get_label_entry(struct inode *root_inode,
+			       struct buffer_head **bh,
+			       struct msdos_dir_entry **de);
+extern int fat_create_label_entry(struct inode *root_inode,
+				  const unsigned char *name);
+
 /* fat/fatent.c */
 struct fat_entry {
 	int entry;
@@ -329,6 +352,10 @@ extern void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec *ts,
 			      __le16 __time, __le16 __date, u8 time_cs);
 extern void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec *ts,
 			      __le16 *time, __le16 *date, u8 *time_cs);
+extern void fat_time_de2inode(struct msdos_sb_info *sbi, struct inode *inode,
+			      struct msdos_dir_entry *de);
+extern void fat_time_inode2de(struct msdos_sb_info *sbi, struct inode *inode,
+			      struct msdos_dir_entry *de);
 extern int fat_sync_bhs(struct buffer_head **bhs, int nr_bhs);
 
 int fat_cache_init(void);
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 8970d8c..818ba3e 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -384,13 +384,7 @@ static int fat_fill_inode(struct inode *inode, struct msdos_dir_entry *de)
 	inode->i_blocks = ((inode->i_size + (sbi->cluster_size - 1))
 			   & ~((loff_t)sbi->cluster_size - 1)) >> 9;
 
-	fat_time_fat2unix(sbi, &inode->i_mtime, de->time, de->date, 0);
-	if (sbi->options.isvfat) {
-		fat_time_fat2unix(sbi, &inode->i_ctime, de->ctime,
-				  de->cdate, de->ctime_cs);
-		fat_time_fat2unix(sbi, &inode->i_atime, 0, de->adate, 0);
-	} else
-		inode->i_ctime = inode->i_atime = inode->i_mtime;
+	fat_time_de2inode(sbi, inode, de);
 
 	return 0;
 }
@@ -590,45 +584,67 @@ static int fat_write_inode(struct inode *inode, int wait)
 	loff_t i_pos;
 	int err;
 
-	if (inode->i_ino == MSDOS_ROOT_INO)
-		return 0;
+	if (inode->i_ino == MSDOS_ROOT_INO) {
+		if (!(sbi->options.root_ts & FAT_ROOT_TS_WRITE))
+			return 0;
+
+		/* Write the timestamps of a FAT file system's root directory
+		 * as the timestamps of the file system's label dir entry. */
+
+		spin_lock(&sbi->inode_hash_lock);
+		err = fat_get_label_entry(inode, &bh, &raw_entry);
+		if (err) {
+			if (err == -ENOENT) {
+				if (sbi->options.root_ts & FAT_ROOT_TS_CREATE) {
+
+					printk(KERN_INFO "FAT: creating volume"
+					       " label on %s to save root dir"
+					       " timestamps\n", sb->s_id);
+					err = fat_create_label_entry(inode,
+								     NULL);
+				} else {
+					/* No label present, but CREATE flag is
+					   not set. Thus not complaining. */
+					err = 0;
+				}
+			}
+			spin_unlock(&sbi->inode_hash_lock);
+			return err;
+		}
+
+	} else { /* inodes other than root directory */
 
 retry:
-	i_pos = fat_i_pos_read(sbi, inode);
-	if (!i_pos)
-		return 0;
+		i_pos = fat_i_pos_read(sbi, inode);
+		if (!i_pos)
+			return 0;
 
-	bh = sb_bread(sb, i_pos >> sbi->dir_per_block_bits);
-	if (!bh) {
-		printk(KERN_ERR "FAT: unable to read inode block "
-		       "for updating (i_pos %lld)\n", i_pos);
-		return -EIO;
-	}
-	spin_lock(&sbi->inode_hash_lock);
-	if (i_pos != MSDOS_I(inode)->i_pos) {
-		spin_unlock(&sbi->inode_hash_lock);
-		brelse(bh);
-		goto retry;
-	}
+		bh = sb_bread(sb, i_pos >> sbi->dir_per_block_bits);
+		if (!bh) {
+			printk(KERN_ERR "FAT: unable to read inode block "
+			       "for updating (i_pos %lld)\n", i_pos);
+			return -EIO;
+		}
+		spin_lock(&sbi->inode_hash_lock);
+		if (i_pos != MSDOS_I(inode)->i_pos) {
+			spin_unlock(&sbi->inode_hash_lock);
+			brelse(bh);
+			goto retry;
+		}
 
-	raw_entry = &((struct msdos_dir_entry *) (bh->b_data))
-	    [i_pos & (sbi->dir_per_block - 1)];
-	if (S_ISDIR(inode->i_mode))
-		raw_entry->size = 0;
-	else
-		raw_entry->size = cpu_to_le32(inode->i_size);
-	raw_entry->attr = fat_make_attrs(inode);
-	raw_entry->start = cpu_to_le16(MSDOS_I(inode)->i_logstart);
-	raw_entry->starthi = cpu_to_le16(MSDOS_I(inode)->i_logstart >> 16);
-	fat_time_unix2fat(sbi, &inode->i_mtime, &raw_entry->time,
-			  &raw_entry->date, NULL);
-	if (sbi->options.isvfat) {
-		__le16 atime;
-		fat_time_unix2fat(sbi, &inode->i_ctime, &raw_entry->ctime,
-				  &raw_entry->cdate, &raw_entry->ctime_cs);
-		fat_time_unix2fat(sbi, &inode->i_atime, &atime,
-				  &raw_entry->adate, NULL);
+		raw_entry = &((struct msdos_dir_entry *) (bh->b_data))
+			[i_pos & (sbi->dir_per_block - 1)];
+		if (S_ISDIR(inode->i_mode))
+			raw_entry->size = 0;
+		else
+			raw_entry->size = cpu_to_le32(inode->i_size);
+		raw_entry->attr = fat_make_attrs(inode);
+		raw_entry->start = cpu_to_le16(MSDOS_I(inode)->i_logstart);
+		raw_entry->starthi =
+			cpu_to_le16(MSDOS_I(inode)->i_logstart >> 16);
 	}
+
+	fat_time_inode2de(sbi, inode, raw_entry);
 	spin_unlock(&sbi->inode_hash_lock);
 	mark_buffer_dirty(bh);
 	err = 0;
@@ -863,6 +879,17 @@ static int fat_show_options(struct seq_file *m, struct vfsmount *mnt)
 	else
 		seq_puts(m, ",errors=remount-ro");
 
+	switch (opts->root_ts) {
+	case FAT_ROOT_TS_LOAD:
+		seq_puts(m, ",rootts=load");
+		break;
+	case FAT_ROOT_TS_PRESERVE:
+		seq_puts(m, ",rootts=preserve");
+		break;
+	case FAT_ROOT_TS_SAVE:
+		seq_puts(m, ",rootts=save");
+		break;
+	}
 	return 0;
 }
 
@@ -875,7 +902,8 @@ enum {
 	Opt_shortname_winnt, Opt_shortname_mixed, Opt_utf8_no, Opt_utf8_yes,
 	Opt_uni_xl_no, Opt_uni_xl_yes, Opt_nonumtail_no, Opt_nonumtail_yes,
 	Opt_obsolate, Opt_flush, Opt_tz_utc, Opt_rodir, Opt_err_cont,
-	Opt_err_panic, Opt_err_ro, Opt_err,
+	Opt_err_panic, Opt_err_ro, Opt_rootts_load, Opt_rootts_preserve,
+	Opt_rootts_save, Opt_rootts_ignore, Opt_err,
 };
 
 static const match_table_t fat_tokens = {
@@ -903,6 +931,10 @@ static const match_table_t fat_tokens = {
 	{Opt_err_cont, "errors=continue"},
 	{Opt_err_panic, "errors=panic"},
 	{Opt_err_ro, "errors=remount-ro"},
+	{Opt_rootts_load, "rootts=load"},
+	{Opt_rootts_preserve, "rootts=preserve"},
+	{Opt_rootts_save, "rootts=save"},
+	{Opt_rootts_ignore, "rootts=ignore"},
 	{Opt_obsolate, "conv=binary"},
 	{Opt_obsolate, "conv=text"},
 	{Opt_obsolate, "conv=auto"},
@@ -984,6 +1016,7 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,
 	opts->usefree = opts->nocase = 0;
 	opts->tz_utc = 0;
 	opts->errors = FAT_ERRORS_RO;
+	opts->root_ts = FAT_ROOT_TS_IGNORE;
 	*debug = 0;
 
 	if (!options)
@@ -1085,6 +1118,18 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,
 		case Opt_err_ro:
 			opts->errors = FAT_ERRORS_RO;
 			break;
+		case Opt_rootts_ignore:
+			opts->root_ts = FAT_ROOT_TS_IGNORE;
+			break;
+		case Opt_rootts_load:
+			opts->root_ts = FAT_ROOT_TS_LOAD;
+			break;
+		case Opt_rootts_preserve:
+			opts->root_ts = FAT_ROOT_TS_PRESERVE;
+			break;
+		case Opt_rootts_save:
+			opts->root_ts = FAT_ROOT_TS_SAVE;
+			break;
 
 		/* msdos specific */
 		case Opt_dots:
@@ -1207,6 +1252,18 @@ static int fat_read_root(struct inode *inode)
 	inode->i_mtime.tv_nsec = inode->i_atime.tv_nsec = inode->i_ctime.tv_nsec = 0;
 	inode->i_nlink = fat_subdirs(inode)+2;
 
+	/* Try to restore the root dir's timestamps from the FAT volume label
+	   entry */
+	if (sbi->options.root_ts & FAT_ROOT_TS_READ) {
+		struct buffer_head *bh;
+		struct msdos_dir_entry *de;
+
+		if (!fat_get_label_entry(inode, &bh, &de)) {
+			fat_time_de2inode(sbi, inode, de);
+			brelse(bh);
+		}
+	}
+
 	return 0;
 }
 
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index a6c2047..3652096 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -268,6 +268,33 @@ void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec *ts,
 }
 EXPORT_SYMBOL_GPL(fat_time_unix2fat);
 
+void fat_time_de2inode(struct msdos_sb_info *sbi, struct inode *inode,
+		       struct msdos_dir_entry *de)
+{
+	fat_time_fat2unix(sbi, &inode->i_mtime, de->time, de->date, 0);
+	if (sbi->options.isvfat) {
+		fat_time_fat2unix(sbi, &inode->i_ctime, de->ctime,
+				  de->cdate, de->ctime_cs);
+		fat_time_fat2unix(sbi, &inode->i_atime, 0, de->adate, 0);
+	} else
+		inode->i_ctime = inode->i_atime = inode->i_mtime;
+}
+EXPORT_SYMBOL_GPL(fat_time_de2inode);
+
+void fat_time_inode2de(struct msdos_sb_info *sbi, struct inode *inode,
+		       struct msdos_dir_entry *de)
+{
+	fat_time_unix2fat(sbi, &inode->i_mtime, &de->time, &de->date, NULL);
+	if (sbi->options.isvfat) {
+		__le16 atime;
+		fat_time_unix2fat(sbi, &inode->i_ctime, &de->ctime,
+				  &de->cdate, &de->ctime_cs);
+		fat_time_unix2fat(sbi, &inode->i_atime, &atime,
+				  &de->adate, NULL);
+	}
+}
+EXPORT_SYMBOL_GPL(fat_time_inode2de);
+
 int fat_sync_bhs(struct buffer_head **bhs, int nr_bhs)
 {
 	int i, err = 0;
-- 
1.5.4.3

Re: [PATCH take 2][RFC] fat: Save FAT root directory timestamps to volume label

[OGAWA Hirofumi]

Jorg Schummer <ext-jorg.2.schummer@nokia.com> writes:

> Standard FAT implementations cannot store any of the FAT root directory's
> timestamps. This commit adds the mount option 'rootts', which allows saving
> the FAT root directory timestamps as the timestamps of the FAT volume label
> directory entry. At least Mac OS X is known to support the same mechanism
> and interoperate with this commit.
>
> When mounting, the following values can be specified for the 'rootts' mount
> option:
>
>   "rootts=ignore"   ignores root directory timestamps. All timestamps are
>                     reset to 0 (1/1/1970). This is the default.
>
>   "rootts=load"     only tries to load the root directory's timestamps from
>                     a volume label entry when mounting.
>
>   "rootts=preserve" tries to load and save the root directory's timestamps
>                     if a volume label entry exists.
>
>   "rootts=save"     tries to load and save the root directory's timestamps.
>                     If the root directory was accessed but no volume label
>                     entry exists, the label "NO NAME" is created.

Looks like interesting hack. However, personally, I don't think I want
this, but I guess it can be only me.

Well, so, what is this for? If rootdir doesn't have timestamp, some app
is not working, or something?

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH take 2][RFC] fat: Save FAT root directory timestamps to volume label

[Jörg Schummer]

On Sat, 2009-07-25 at 07:48 +0200, ext OGAWA Hirofumi wrote:
> Jorg Schummer <ext-jorg.2.schummer@nokia.com> writes:
> 
> > Standard FAT implementations cannot store any of the FAT root directory's
> > timestamps. This commit adds the mount option 'rootts', which allows saving
> > the FAT root directory timestamps as the timestamps of the FAT volume label
> > directory entry. At least Mac OS X is known to support the same mechanism
> > and interoperate with this commit.

Hi,

I guess I should have explained a bit more:

> Looks like interesting hack. However, personally, I don't think I want
> this, but I guess it can be only me.

If you don't want this functionality to be used, as pointed out before,
"rootts=ignore" is the default.

If you don't want this patch.. There's probably good reasons not to take
it, especially the amount-of-problem-solving-by-maintenance-cost ratio
might be rather low.

> Well, so, what is this for? If rootdir doesn't have timestamp, some app
> is not working, or something?

To be honest, the only app which I can think of here is some
backup-script which I wrote many years ago and which has not been in use
anymore for a long time. (Maybe rsync or similar could also fail under
some circumstances? Not sure.)

So some backup / synchronisation apps might not work properly. But who's
going to use FAT for sensitive, i.e. backup-worthy data anyway? The
answer is: People who use the same volume in many different machines,
some of which might be able to speak only FAT. Drawback: Just for these
people this patch might not be useful since none of those FAT machines
(except for the apple and possibly the penguin) support root dir
timestamps anyway. So whatever timestamps are found at backup-time, they
cannot be trusted, because the volume might have been in use by standard
FAT implementations.

In summary: If you're not keen on integrating new features which would
make Linux look good in the face of shiny Mac OS, there might not be
much reason to take it in. Unless we get zillions of people replying now
with what wonderful things it would enable them to do.. ;-)

Regards,
Jörg


--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH take 2][RFC] fat: Save FAT root directory timestamps to volume label

[OGAWA Hirofumi]

Jörg Schummer <ext-jorg.2.schummer@nokia.com> writes:

> Hi,

Hi,

> If you don't want this patch.. There's probably good reasons not to take
> it, especially the amount-of-problem-solving-by-maintenance-cost ratio
> might be rather low.

Also, the option is easy to add, but really hard to delete by backward
compatible reason. This is why I'm so careful to it.

BTW, the patch has several bugs. fat_get_label_entry() doesn't check
IS_FREE(), is it right? fat_create_label_entry() doesn't initialize all
timestamp in the case of msdos. spin_lock() usage is wrong. and more...

>> Well, so, what is this for? If rootdir doesn't have timestamp, some app
>> is not working, or something?
>
> To be honest, the only app which I can think of here is some
> backup-script which I wrote many years ago and which has not been in use
> anymore for a long time. (Maybe rsync or similar could also fail under
> some circumstances? Not sure.)
>
> So some backup / synchronisation apps might not work properly. But who's
> going to use FAT for sensitive, i.e. backup-worthy data anyway? The
> answer is: People who use the same volume in many different machines,
> some of which might be able to speak only FAT. Drawback: Just for these
> people this patch might not be useful since none of those FAT machines
> (except for the apple and possibly the penguin) support root dir
> timestamps anyway. So whatever timestamps are found at backup-time, they
> cannot be trusted, because the volume might have been in use by standard
> FAT implementations.
>
> In summary: If you're not keen on integrating new features which would
> make Linux look good in the face of shiny Mac OS, there might not be
> much reason to take it in. Unless we get zillions of people replying now
> with what wonderful things it would enable them to do.. ;-)

Yes, we would really want to know whether people want to use this. Is
there any info from Mac OS people?

BTW, my thinking is, the option is why default is "rootts=ignore" if
it's really good? Or 4 rootts=* options is really needed, and what is
reason?  Or if it's not good by some reasons, why is it added? ...

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH take 2][RFC] fat: Save FAT root directory timestamps to volume label

[Jörg Schummer]

On Mon, 2009-07-27 at 13:47 +0200, ext OGAWA Hirofumi wrote:
> Jörg Schummer <ext-jorg.2.schummer@nokia.com> writes:

> > If you don't want this patch.. There's probably good reasons not to take
> > it, especially the amount-of-problem-solving-by-maintenance-cost ratio
> > might be rather low.

> BTW, the patch has several bugs. fat_get_label_entry() doesn't check
> IS_FREE(), is it right? fat_create_label_entry() doesn't initialize all
> timestamp in the case of msdos. spin_lock() usage is wrong. and more...

You got good points there. I'll fix these on the chance that somebody
decides to use it.

> > In summary: If you're not keen on integrating new features which would
> > make Linux look good in the face of shiny Mac OS, there might not be
> > much reason to take it in. Unless we get zillions of people replying now
> > with what wonderful things it would enable them to do.. ;-)
> 
> Yes, we would really want to know whether people want to use this. Is
> there any info from Mac OS people?
> 
> BTW, my thinking is, the option is why default is "rootts=ignore" if
> it's really good?

That's because this feature does not seem to be part of most generally
accepted FAT implementations. So I thought that the default should try
to behave as people would expect FAT to behave: Forget about root dir
timestamps.
On the other hand, except for 'save', none of the options makes 'real'
fs changes, so maybe 'preserve' could be the default. (People who are
used to the 'standard' behaviour should not be harmed by that - once the
bugs are gone, that is..)

> Or 4 rootts=* options is really needed, and what is
> reason? 

I thought 'ignore' should be there in order to provide the
'standard'-FAT behaviour. And 'save' possibly creates a volume label,
which might be not be wanted. But 'load' is not so necessary, indeed.
The behaviour of Mac OS seems to be like 'preserve', btw.

>  Or if it's not good by some reasons, why is it added? ...

The patch was created merely because one guy figured out that FAT can't
save root dir timestamps by default and another guy knew that Mac OS X's
FAT implementation can. So I thought, what the heck, let's enable linux
to do the same thing.

Like mentioned before, this was not created because it would solve many
people's problems. So I seriously do understand if you decide not to
take it in. But still I would appreciate if you could take another look
at my next - hopefully less buggy - version.

Thanks for your comments,
Jörg


--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH take 2][RFC] fat: Save FAT root directory timestamps to volume label

[Jörg Schummer]

Hello again,

just in case you (or perhaps somebody else) have a minute to assist me..

On Mon, 2009-07-27 at 13:47 +0200, ext OGAWA Hirofumi wrote:

> BTW, the patch has several bugs. fat_get_label_entry() doesn't check
> IS_FREE(), is it right?

Not 100% sure if it's needed in a consistent fs, but I guess it
shouldn't do any harm, so I added it.

> fat_create_label_entry() doesn't initialize all
> timestamp in the case of msdos.

Corrected.

> spin_lock() usage is wrong.

>From the code, I'm not totally sure then what inode_hash_lock is
supposed to protect in fat_write_inode. Probably it should prevent that
the dir entry of an inode is being moved around to another i_pos while
it's being changed?

But in the pre-patch version of fat_write_inode, shouldn't then
mark_buffer_dirty(bh) be called when the lock is still held?

Sorry for the newbie-questions, thanks for any help.

Jörg


--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH take 2][RFC] fat: Save FAT root directory timestamps to volume label

[OGAWA Hirofumi]

Jörg Schummer <ext-jorg.2.schummer@nokia.com> writes:

> Hello again,
>
> just in case you (or perhaps somebody else) have a minute to assist me..
>
> On Mon, 2009-07-27 at 13:47 +0200, ext OGAWA Hirofumi wrote:
>
>> BTW, the patch has several bugs. fat_get_label_entry() doesn't check
>> IS_FREE(), is it right?
>
> Not 100% sure if it's needed in a consistent fs, but I guess it
> shouldn't do any harm, so I added it.

If label was removed, what happen? I was assumed utility does

    label_de->name[0] = DELETED_FLAG;


>> spin_lock() usage is wrong.
>
>>From the code, I'm not totally sure then what inode_hash_lock is
> supposed to protect in fat_write_inode. Probably it should prevent that
> the dir entry of an inode is being moved around to another i_pos while
> it's being changed?
>
> But in the pre-patch version of fat_write_inode, shouldn't then
> mark_buffer_dirty(bh) be called when the lock is still held?

I meant, read/write of storage might sleep, so it shouldn't hold
spin_lock when does it.

If you set debug CONFIG_* of locking, I guess it will warn.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH take 2][RFC] fat: Save FAT root directory timestamps to volume label

[Jörg Schummer]

On Mon, 2009-07-27 at 13:47 +0200, ext OGAWA Hirofumi wrote:
> Jörg Schummer <ext-jorg.2.schummer@nokia.com> writes:

> BTW, the patch has several bugs. fat_get_label_entry() doesn't check
> IS_FREE(), is it right? fat_create_label_entry() doesn't initialize all
> timestamp in the case of msdos. spin_lock() usage is wrong. and more...

After reviewing the patch again, I did notice some additional problems
(I suppose that was the "and more..."). If and when I have time again, I
might take a closer look at it again.

Thanks for your help until now!

Jörg

Re: [PATCH take 2][RFC] fat: Save FAT root directory timestamps to volume label

[OGAWA Hirofumi]

Jörg Schummer <ext-jorg.2.schummer@nokia.com> writes:

> On Mon, 2009-07-27 at 13:47 +0200, ext OGAWA Hirofumi wrote:
>> Jörg Schummer <ext-jorg.2.schummer@nokia.com> writes:
>
>> BTW, the patch has several bugs. fat_get_label_entry() doesn't check
>> IS_FREE(), is it right? fat_create_label_entry() doesn't initialize all
>> timestamp in the case of msdos. spin_lock() usage is wrong. and more...
>
> After reviewing the patch again, I did notice some additional problems
> (I suppose that was the "and more..."). If and when I have time again, I
> might take a closer look at it again.
>
> Thanks for your help until now!

No problem at all. Thanks for your work.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH take 3][RFC] fat: Save FAT root directory timestamps to volume label

[Jörg Schummer]

On Tue, 2009-08-04 at 12:32 +0200, ext OGAWA Hirofumi wrote: 
> Jörg Schummer <ext-jorg.2.schummer@nokia.com> writes:
> 
> > On Mon, 2009-07-27 at 13:47 +0200, ext OGAWA Hirofumi wrote:
> >> Jörg Schummer <ext-jorg.2.schummer@nokia.com> writes:
> >
> >> BTW, the patch has several bugs. fat_get_label_entry() doesn't check
> >> IS_FREE(), is it right? fat_create_label_entry() doesn't initialize all
> >> timestamp in the case of msdos. spin_lock() usage is wrong. and more...
> >
> > After reviewing the patch again, I did notice some additional problems
> > (I suppose that was the "and more..."). If and when I have time again, I
> > might take a closer look at it again.
> >
> > Thanks for your help until now!

Hello,

on the off-chance that somebody would like to use this mechanism, I did
a 3rd take of the patch. I tried to fix the problems of the previous
patch and additionally added the functionality to correct the root dir
timestamp's mtime and atime somewhat based on contained directory
entries' ctime.

Any comments are welcome, but especially I'd like to ask if it is safe /
good / doable to call 

  remove_inode_hash(inode);
  fat_attach(inode, i_pos);
  insert_inode_hash(inode);

inside fat_write_inode .

Thanks for any feedback!
Jörg


--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH take 3][RFC] fat: Save FAT root directory timestamps to volume label

[Jorg Schummer]

Standard FAT implementations cannot store any of the FAT root directory's
timestamps. This commit adds the mount option 'rootts', which allows saving
the FAT root directory timestamps as the timestamps of the FAT volume label
directory entry. At least Mac OS X is known to support the same mechanism
and interoperate with this commit.

When mounting, the following values can be specified for the 'rootts' mount
option:

  "rootts=ignore"   ignores root directory timestamps. All timestamps are
                    reset to 0 (1/1/1970). This has been the FAT behaviour
                    prior to this patch.

  "rootts=preserve" tries to load and save the root directory's timestamps
                    if a volume label entry exists. The mtime and atime are
                    corrected based on root directory entries' ctime. This
                    is the default.

  "rootts=save"     tries to load and save the root directory's timestamps.
                    The mtime and atime are corrected based on root
                    directory entries' ctime. If the root directory was
                    accessed but no volume label entry exists, the label
                    "NO NAME" is created.

Signed-off-by: Jorg Schummer <ext-jorg.2.schummer@nokia.com>
---
 Documentation/filesystems/vfat.txt |    9 ++
 fs/fat/dir.c                       |   75 ++++++++++++++
 fs/fat/fat.h                       |   19 ++++
 fs/fat/inode.c                     |  192 +++++++++++++++++++++++++-----------
 4 files changed, 239 insertions(+), 56 deletions(-)

diff --git a/Documentation/filesystems/vfat.txt b/Documentation/filesystems/vfat.txt
index b58b84b..64f6509 100644
--- a/Documentation/filesystems/vfat.txt
+++ b/Documentation/filesystems/vfat.txt
@@ -137,6 +137,15 @@ errors=panic|continue|remount-ro
 		 without doing anything or remount the partition in
 		 read-only mode (default behavior).
 
+rootts=ignore|preserve|save
+	      -- Specify whether to load/store the root dir timestamps as the
+		 timestamp of the volume label entry.
+		 ignore:   do not load or store root dir ts.
+		 preserve: load the root dir ts, store them only if a volume
+			   label already exists. This is the default.
+		 save:     load and store the root dir ts, create a volume label
+			   if the ts have changed but no label is present.
+
 <bool>: 0,1,yes,no,true,false
 
 TODO
diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 530b4ca..114e748 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -1362,3 +1362,78 @@ error_remove:
 }
 
 EXPORT_SYMBOL_GPL(fat_add_entries);
+
+int fat_get_label_entry(struct inode *root_inode, struct buffer_head **bh,
+			struct msdos_dir_entry **de, loff_t *i_pos)
+{
+	loff_t offset;
+
+	BUG_ON(root_inode->i_ino != MSDOS_ROOT_INO);
+
+	offset = 0;
+	*bh = NULL;
+	while (fat_get_entry(root_inode, &offset, bh, de) >= 0) {
+		/* not free and volume label */
+		if (!IS_FREE((*de)->name) && (*de)->attr != ATTR_EXT &&
+		    ((*de)->attr & ATTR_VOLUME)) {
+			*i_pos = fat_make_i_pos(root_inode->i_sb, *bh, *de);
+			return 0;
+		}
+	}
+	return -ENOENT;
+}
+EXPORT_SYMBOL_GPL(fat_get_label_entry);
+
+int fat_add_label_entry(struct inode *root_inode, const unsigned char *name)
+{
+	struct msdos_dir_entry de;
+	struct fat_slot_info sinfo;
+	int err;
+
+	BUG_ON(root_inode->i_ino != MSDOS_ROOT_INO);
+
+	memcpy(de.name,
+	       (name) ? name : (const unsigned char *) FAT_LABEL_NONAME,
+	       MSDOS_NAME);
+	de.attr = ATTR_VOLUME;
+	de.lcase = 0;
+	de.start = de.starthi = 0;
+	de.size = 0;
+	de.time = de.ctime = 0;
+	de.ctime_cs = 0;
+	de.date = de.adate = de.cdate = 0;
+
+	err = fat_add_entries(root_inode, &de, 1, &sinfo);
+	brelse(sinfo.bh);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(fat_add_label_entry);
+
+void fat_correct_root_ts(struct inode *root_inode)
+{
+	struct msdos_sb_info *sbi = MSDOS_SB(root_inode->i_sb);
+	struct buffer_head *bh;
+	struct msdos_dir_entry *de;
+	struct timespec ts;
+	loff_t offset;
+
+	BUG_ON(root_inode->i_ino != MSDOS_ROOT_INO);
+
+	offset = 0;
+	bh = NULL;
+	while (fat_get_entry(root_inode, &offset, &bh, &de) >= 0) {
+		/* not free and short entry, not a volume label */
+		if (!IS_FREE(de->name) &&
+		    de->attr != ATTR_EXT && !(de->attr & ATTR_VOLUME)) {
+			fat_time_fat2unix(sbi, &ts, de->ctime,
+					  de->cdate, de->ctime_cs);
+			if (timespec_compare(&root_inode->i_mtime, &ts) < 0)
+				root_inode->i_mtime = ts;
+		}
+	}
+
+	if (timespec_compare(&root_inode->i_mtime, &root_inode->i_atime) > 0)
+		root_inode->i_atime = root_inode->i_mtime;
+}
+EXPORT_SYMBOL_GPL(fat_correct_root_ts);
diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index adb0e72..27bc46d 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -21,6 +21,14 @@
 #define FAT_ERRORS_PANIC	2      /* panic on error */
 #define FAT_ERRORS_RO		3      /* remount r/o on error */
 
+/*
+ * fat root directory timestamp backup
+ */
+#define FAT_ROOT_TS_IGNORE      1 /* do not read or write root dir ts */
+#define FAT_ROOT_TS_PRESERVE    2 /* read and write root dir ts as volume label
+				     timestamp */
+#define FAT_ROOT_TS_SAVE        3 /* create volume label if there is none */
+
 struct fat_mount_options {
 	uid_t fs_uid;
 	gid_t fs_gid;
@@ -32,6 +40,8 @@ struct fat_mount_options {
 	unsigned char name_check; /* r = relaxed, n = normal, s = strict */
 	unsigned char errors;	  /* On error: continue, panic, remount-ro */
 	unsigned short allow_utime;/* permission for setting the [am]time */
+	unsigned short root_ts;   /* root dir timestamps:
+				       ignore, preserve, save */
 	unsigned quiet:1,         /* set = fake successful chmods and chowns */
 		 showexec:1,      /* set = only set x bit for com/exe/bat */
 		 sys_immutable:1, /* set = system files are immutable */
@@ -50,6 +60,8 @@ struct fat_mount_options {
 #define FAT_HASH_BITS	8
 #define FAT_HASH_SIZE	(1UL << FAT_HASH_BITS)
 
+#define FAT_LABEL_NONAME "NO NAME    "
+
 /*
  * MS-DOS file system in-core superblock data
  */
@@ -247,6 +259,13 @@ extern int fat_add_entries(struct inode *dir, void *slots, int nr_slots,
 			   struct fat_slot_info *sinfo);
 extern int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo);
 
+extern int fat_get_label_entry(struct inode *root_inode,
+			       struct buffer_head **bh,
+			       struct msdos_dir_entry **de, loff_t *i_pos);
+extern int fat_add_label_entry(struct inode *root_inode,
+			       const unsigned char *name);
+extern void fat_correct_root_ts(struct inode *root_inode);
+
 /* fat/fatent.c */
 struct fat_entry {
 	int entry;
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 8970d8c..0ba0e0c 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -330,35 +330,62 @@ static int fat_calc_dir_size(struct inode *inode)
 	return 0;
 }
 
-/* doesn't deal with root inode */
+/* nowadays, this deals also with the root inode */
 static int fat_fill_inode(struct inode *inode, struct msdos_dir_entry *de)
 {
 	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
 	int error;
+	__u8 attr;
+	int is_root = (inode->i_ino == MSDOS_ROOT_INO);
+
+	if (is_root)
+		attr = ATTR_DIR;
+	else
+		attr = de->attr;
 
 	MSDOS_I(inode)->i_pos = 0;
 	inode->i_uid = sbi->options.fs_uid;
 	inode->i_gid = sbi->options.fs_gid;
 	inode->i_version++;
-	inode->i_generation = get_seconds();
+	if (is_root)
+		inode->i_generation = 0;
+	else
+		inode->i_generation = get_seconds();
 
-	if ((de->attr & ATTR_DIR) && !IS_FREE(de->name)) {
-		inode->i_generation &= ~1;
-		inode->i_mode = fat_make_mode(sbi, de->attr, S_IRWXUGO);
+	if ((attr & ATTR_DIR) && (is_root || !IS_FREE(de->name))) {
+		if (!is_root)
+			inode->i_generation &= ~1;
+		inode->i_mode = fat_make_mode(sbi, attr, S_IRWXUGO);
 		inode->i_op = sbi->dir_ops;
 		inode->i_fop = &fat_dir_operations;
 
-		MSDOS_I(inode)->i_start = le16_to_cpu(de->start);
-		if (sbi->fat_bits == 32)
-			MSDOS_I(inode)->i_start |= (le16_to_cpu(de->starthi) << 16);
-
-		MSDOS_I(inode)->i_logstart = MSDOS_I(inode)->i_start;
-		error = fat_calc_dir_size(inode);
+		if (is_root) {
+			if (sbi->fat_bits == 32) {
+				MSDOS_I(inode)->i_start = sbi->root_cluster;
+				error = fat_calc_dir_size(inode);
+			} else {
+				MSDOS_I(inode)->i_start = 0;
+				inode->i_size = sbi->dir_entries *
+					sizeof(struct msdos_dir_entry);
+				error = 0;
+			}
+			MSDOS_I(inode)->i_logstart = 0;
+		} else {
+			MSDOS_I(inode)->i_start = le16_to_cpu(de->start);
+			if (sbi->fat_bits == 32)
+				MSDOS_I(inode)->i_start |=
+					(le16_to_cpu(de->starthi) << 16);
+			MSDOS_I(inode)->i_logstart = MSDOS_I(inode)->i_start;
+			error = fat_calc_dir_size(inode);
+		}
 		if (error < 0)
 			return error;
 		MSDOS_I(inode)->mmu_private = inode->i_size;
 
 		inode->i_nlink = fat_subdirs(inode);
+		if (is_root)
+			inode->i_nlink += 2;
+
 	} else { /* not a directory */
 		inode->i_generation |= 1;
 		inode->i_mode = fat_make_mode(sbi, de->attr,
@@ -375,22 +402,30 @@ static int fat_fill_inode(struct inode *inode, struct msdos_dir_entry *de)
 		inode->i_mapping->a_ops = &fat_aops;
 		MSDOS_I(inode)->mmu_private = inode->i_size;
 	}
-	if (de->attr & ATTR_SYS) {
+	if (attr & ATTR_SYS) {
 		if (sbi->options.sys_immutable)
 			inode->i_flags |= S_IMMUTABLE;
 	}
-	fat_save_attrs(inode, de->attr);
+	fat_save_attrs(inode, attr);
 
 	inode->i_blocks = ((inode->i_size + (sbi->cluster_size - 1))
 			   & ~((loff_t)sbi->cluster_size - 1)) >> 9;
 
-	fat_time_fat2unix(sbi, &inode->i_mtime, de->time, de->date, 0);
-	if (sbi->options.isvfat) {
-		fat_time_fat2unix(sbi, &inode->i_ctime, de->ctime,
-				  de->cdate, de->ctime_cs);
-		fat_time_fat2unix(sbi, &inode->i_atime, 0, de->adate, 0);
-	} else
-		inode->i_ctime = inode->i_atime = inode->i_mtime;
+	if (!de) {
+		inode->i_mtime.tv_sec = inode->i_atime.tv_sec =
+			inode->i_ctime.tv_sec = 0;
+		inode->i_mtime.tv_nsec = inode->i_atime.tv_nsec =
+			inode->i_ctime.tv_nsec = 0;
+	} else {
+		fat_time_fat2unix(sbi, &inode->i_mtime, de->time, de->date, 0);
+		if (sbi->options.isvfat) {
+			fat_time_fat2unix(sbi, &inode->i_ctime, de->ctime,
+					  de->cdate, de->ctime_cs);
+			fat_time_fat2unix(sbi, &inode->i_atime, 0,
+					  de->adate, 0);
+		} else
+			inode->i_ctime = inode->i_atime = inode->i_mtime;
+	}
 
 	return 0;
 }
@@ -590,8 +625,41 @@ static int fat_write_inode(struct inode *inode, int wait)
 	loff_t i_pos;
 	int err;
 
-	if (inode->i_ino == MSDOS_ROOT_INO)
-		return 0;
+	if (inode->i_ino == MSDOS_ROOT_INO) {
+		if (sbi->options.root_ts == FAT_ROOT_TS_IGNORE)
+			return 0;
+
+		/* Write the timestamps of a FAT file system's root directory
+		 * as the timestamps of the file system's label dir entry. */
+
+		if (!MSDOS_I(inode)->i_pos) {
+			struct buffer_head *bh;
+			struct msdos_dir_entry *de;
+			loff_t i_pos;
+
+			if (sbi->options.root_ts != FAT_ROOT_TS_SAVE)
+				/* No label present, but we should not
+				   create one. Thus not complaining. */
+				return 0;
+
+			printk(KERN_INFO "FAT: creating volume label "
+			       "on %s to save root dir timestamps\n",
+			       sb->s_id);
+
+			err = fat_add_label_entry(inode, NULL);
+			if (err)
+				return err;
+
+			err = fat_get_label_entry(inode, &bh, &de, &i_pos);
+			if (err)
+				return err;
+
+			remove_inode_hash(inode);
+			fat_attach(inode, i_pos);
+			insert_inode_hash(inode);
+			brelse(bh);
+		}
+	}
 
 retry:
 	i_pos = fat_i_pos_read(sbi, inode);
@@ -613,13 +681,17 @@ retry:
 
 	raw_entry = &((struct msdos_dir_entry *) (bh->b_data))
 	    [i_pos & (sbi->dir_per_block - 1)];
-	if (S_ISDIR(inode->i_mode))
-		raw_entry->size = 0;
-	else
-		raw_entry->size = cpu_to_le32(inode->i_size);
-	raw_entry->attr = fat_make_attrs(inode);
-	raw_entry->start = cpu_to_le16(MSDOS_I(inode)->i_logstart);
-	raw_entry->starthi = cpu_to_le16(MSDOS_I(inode)->i_logstart >> 16);
+
+	if (inode->i_ino != MSDOS_ROOT_INO) {
+		if (S_ISDIR(inode->i_mode))
+			raw_entry->size = 0;
+		else
+			raw_entry->size = cpu_to_le32(inode->i_size);
+		raw_entry->attr = fat_make_attrs(inode);
+		raw_entry->start = cpu_to_le16(MSDOS_I(inode)->i_logstart);
+		raw_entry->starthi =
+			cpu_to_le16(MSDOS_I(inode)->i_logstart >> 16);
+	}
 	fat_time_unix2fat(sbi, &inode->i_mtime, &raw_entry->time,
 			  &raw_entry->date, NULL);
 	if (sbi->options.isvfat) {
@@ -875,7 +947,8 @@ enum {
 	Opt_shortname_winnt, Opt_shortname_mixed, Opt_utf8_no, Opt_utf8_yes,
 	Opt_uni_xl_no, Opt_uni_xl_yes, Opt_nonumtail_no, Opt_nonumtail_yes,
 	Opt_obsolate, Opt_flush, Opt_tz_utc, Opt_rodir, Opt_err_cont,
-	Opt_err_panic, Opt_err_ro, Opt_err,
+	Opt_err_panic, Opt_err_ro, Opt_rootts_ignore, Opt_rootts_preserve,
+	Opt_rootts_save, Opt_err,
 };
 
 static const match_table_t fat_tokens = {
@@ -903,6 +976,9 @@ static const match_table_t fat_tokens = {
 	{Opt_err_cont, "errors=continue"},
 	{Opt_err_panic, "errors=panic"},
 	{Opt_err_ro, "errors=remount-ro"},
+	{Opt_rootts_ignore, "rootts=ignore"},
+	{Opt_rootts_preserve, "rootts=preserve"},
+	{Opt_rootts_save, "rootts=save"},
 	{Opt_obsolate, "conv=binary"},
 	{Opt_obsolate, "conv=text"},
 	{Opt_obsolate, "conv=auto"},
@@ -984,6 +1060,7 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,
 	opts->usefree = opts->nocase = 0;
 	opts->tz_utc = 0;
 	opts->errors = FAT_ERRORS_RO;
+	opts->root_ts = FAT_ROOT_TS_PRESERVE;
 	*debug = 0;
 
 	if (!options)
@@ -1085,6 +1162,15 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,
 		case Opt_err_ro:
 			opts->errors = FAT_ERRORS_RO;
 			break;
+		case Opt_rootts_ignore:
+			opts->root_ts = FAT_ROOT_TS_IGNORE;
+			break;
+		case Opt_rootts_preserve:
+			opts->root_ts = FAT_ROOT_TS_PRESERVE;
+			break;
+		case Opt_rootts_save:
+			opts->root_ts = FAT_ROOT_TS_SAVE;
+			break;
 
 		/* msdos specific */
 		case Opt_dots:
@@ -1180,34 +1266,28 @@ static int fat_read_root(struct inode *inode)
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 	int error;
 
-	MSDOS_I(inode)->i_pos = 0;
-	inode->i_uid = sbi->options.fs_uid;
-	inode->i_gid = sbi->options.fs_gid;
-	inode->i_version++;
-	inode->i_generation = 0;
-	inode->i_mode = fat_make_mode(sbi, ATTR_DIR, S_IRWXUGO);
-	inode->i_op = sbi->dir_ops;
-	inode->i_fop = &fat_dir_operations;
-	if (sbi->fat_bits == 32) {
-		MSDOS_I(inode)->i_start = sbi->root_cluster;
-		error = fat_calc_dir_size(inode);
-		if (error < 0)
-			return error;
-	} else {
-		MSDOS_I(inode)->i_start = 0;
-		inode->i_size = sbi->dir_entries * sizeof(struct msdos_dir_entry);
-	}
-	inode->i_blocks = ((inode->i_size + (sbi->cluster_size - 1))
-			   & ~((loff_t)sbi->cluster_size - 1)) >> 9;
-	MSDOS_I(inode)->i_logstart = 0;
-	MSDOS_I(inode)->mmu_private = inode->i_size;
+	error = fat_fill_inode(inode, NULL);
+
+	/* Try to restore the root dir's timestamps from the FAT volume label
+	   entry */
+	if (!error && sbi->options.root_ts != FAT_ROOT_TS_IGNORE) {
+		struct buffer_head *bh;
+		struct msdos_dir_entry *de;
+		loff_t i_pos;
 
-	fat_save_attrs(inode, ATTR_DIR);
-	inode->i_mtime.tv_sec = inode->i_atime.tv_sec = inode->i_ctime.tv_sec = 0;
-	inode->i_mtime.tv_nsec = inode->i_atime.tv_nsec = inode->i_ctime.tv_nsec = 0;
-	inode->i_nlink = fat_subdirs(inode)+2;
+		error = fat_get_label_entry(inode, &bh, &de, &i_pos);
+		if (!error) {
+			error = fat_fill_inode(inode, de);
+			if (!error)
+				fat_attach(inode, i_pos);
+			brelse(bh);
+		} else if (error == -ENOENT)
+			error = 0;
 
-	return 0;
+		fat_correct_root_ts(inode);
+	}
+
+	return error;
 }
 
 /*
-- 
1.5.4.3

Re: [PATCH take 3][RFC] fat: Save FAT root directory timestamps to volume label

[Jamie Lokier]

Jorg Schummer wrote:
> Standard FAT implementations cannot store any of the FAT root directory's
> timestamps. This commit adds the mount option 'rootts', which allows saving
> the FAT root directory timestamps as the timestamps of the FAT volume label
> directory entry. At least Mac OS X is known to support the same mechanism
> and interoperate with this commit.
> 
> When mounting, the following values can be specified for the 'rootts' mount
> option:
> 
>   "rootts=ignore"   ignores root directory timestamps. All timestamps are
>                     reset to 0 (1/1/1970). This has been the FAT behaviour
>                     prior to this patch.
> 
>   "rootts=preserve" tries to load and save the root directory's timestamps
>                     if a volume label entry exists. The mtime and atime are
>                     corrected based on root directory entries' ctime. This
>                     is the default.
> 
>   "rootts=save"     tries to load and save the root directory's timestamps.
>                     The mtime and atime are corrected based on root
>                     directory entries' ctime. If the root directory was
>                     accessed but no volume label entry exists, the label
>                     "NO NAME" is created.

Does Mac OS X do the same "NO NAME" thing?

-- Jamie

Re: [PATCH take 3][RFC] fat: Save FAT root directory timestamps to volume label

[Jörg Schummer]

On Tue, 2009-09-08 at 17:39 +0200, ext Jamie Lokier wrote:
> Jorg Schummer wrote:
> > Standard FAT implementations cannot store any of the FAT root directory's
> > timestamps. This commit adds the mount option 'rootts', which allows saving
> > the FAT root directory timestamps as the timestamps of the FAT volume label
> > directory entry. At least Mac OS X is known to support the same mechanism
> > and interoperate with this commit.
> > 
> > When mounting, the following values can be specified for the 'rootts' mount
> > option:
> > 
> >   "rootts=ignore"   ignores root directory timestamps. All timestamps are
> >                     reset to 0 (1/1/1970). This has been the FAT behaviour
> >                     prior to this patch.
> > 
> >   "rootts=preserve" tries to load and save the root directory's timestamps
> >                     if a volume label entry exists. The mtime and atime are
> >                     corrected based on root directory entries' ctime. This
> >                     is the default.
> > 
> >   "rootts=save"     tries to load and save the root directory's timestamps.
> >                     The mtime and atime are corrected based on root
> >                     directory entries' ctime. If the root directory was
> >                     accessed but no volume label entry exists, the label
> >                     "NO NAME" is created.
> 
> Does Mac OS X do the same "NO NAME" thing?

AFAIK: No. It only supports the general mechanism of storing / loading
the root dir timestamps to / from the volume label entry.

But feel free to re-check that one.

Jörg


--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH take 3][RFC] fat: Save FAT root directory timestamps to volume label

[Jörg Schummer]

On Tue, 2009-09-08 at 16:34 +0200, Schummer Jorg.2
(EXT-TietoEnator/Helsinki) wrote:
> Standard FAT implementations cannot store any of the FAT root directory's
> timestamps. This commit adds the mount option 'rootts', which allows saving
> the FAT root directory timestamps as the timestamps of the FAT volume label
> directory entry. At least Mac OS X is known to support the same mechanism
> and interoperate with this commit.
> 
> When mounting, the following values can be specified for the 'rootts' mount
> option:
> 
>   "rootts=ignore"   ignores root directory timestamps. All timestamps are
>                     reset to 0 (1/1/1970). This has been the FAT behaviour
>                     prior to this patch.
> 
>   "rootts=preserve" tries to load and save the root directory's timestamps
>                     if a volume label entry exists. The mtime and atime are
>                     corrected based on root directory entries' ctime. This
>                     is the default.
> 
>   "rootts=save"     tries to load and save the root directory's timestamps.
>                     The mtime and atime are corrected based on root
>                     directory entries' ctime. If the root directory was
>                     accessed but no volume label entry exists, the label
>                     "NO NAME" is created.

Hello again,

in case anyone would like to send some feedback or comments about the
patch, please use this email address from now on: jorg@ml1.net .

Thanks,
Jörg


--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH take 3][RFC] fat: Save FAT root directory timestamps to volume label

[OGAWA Hirofumi]

Jorg Schummer <ext-jorg.2.schummer@nokia.com> writes:

> Standard FAT implementations cannot store any of the FAT root directory's
> timestamps. This commit adds the mount option 'rootts', which allows saving
> the FAT root directory timestamps as the timestamps of the FAT volume label
> directory entry. At least Mac OS X is known to support the same mechanism
> and interoperate with this commit.
>
> When mounting, the following values can be specified for the 'rootts' mount
> option:
>
>   "rootts=ignore"   ignores root directory timestamps. All timestamps are
>                     reset to 0 (1/1/1970). This has been the FAT behaviour
>                     prior to this patch.
>
>   "rootts=preserve" tries to load and save the root directory's timestamps
>                     if a volume label entry exists. The mtime and atime are
>                     corrected based on root directory entries' ctime. This
>                     is the default.
>
>   "rootts=save"     tries to load and save the root directory's timestamps.
>                     The mtime and atime are corrected based on root
>                     directory entries' ctime. If the root directory was
>                     accessed but no volume label entry exists, the label
>                     "NO NAME" is created.

Um..., I'm still not sure though. rootts=save may not be needed for
this, because it seems a bit complex, and it can do on userland...

And some quick review of implement..

> +	if (inode->i_ino == MSDOS_ROOT_INO) {
> +		if (sbi->options.root_ts == FAT_ROOT_TS_IGNORE)
> +			return 0;
> +
> +		/* Write the timestamps of a FAT file system's root directory
> +		 * as the timestamps of the file system's label dir entry. */
> +
> +		if (!MSDOS_I(inode)->i_pos) {
> +			struct buffer_head *bh;
> +			struct msdos_dir_entry *de;
> +			loff_t i_pos;
> +
> +			if (sbi->options.root_ts != FAT_ROOT_TS_SAVE)
> +				/* No label present, but we should not
> +				   create one. Thus not complaining. */
> +				return 0;
> +
> +			printk(KERN_INFO "FAT: creating volume label "
> +			       "on %s to save root dir timestamps\n",
> +			       sb->s_id);
> +
> +			err = fat_add_label_entry(inode, NULL);
> +			if (err)
> +				return err;
> +			err = fat_get_label_entry(inode, &bh, &de, &i_pos);
> +			if (err)
> +				return err;

In this path, we don't have inode->i_mutex, so to look directry up is
wrong. I think adding label may move to userland.

I guess we should do all of preparation in fill_super().

> +			remove_inode_hash(inode);
> +			fat_attach(inode, i_pos);
> +			insert_inode_hash(inode);
> +			brelse(bh);

Root inode should never change the position, so this shouldn't be
needed.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH take 3][RFC] fat: Save FAT root directory timestamps to volume label

[jorg]

Hello and thanks for your comments!

On Fri, 18 Sep 2009 20:33 +0900, "OGAWA Hirofumi"
<hirofumi@mail.parknet.co.jp> wrote:
> Jorg Schummer <ext-jorg.2.schummer@nokia.com> writes:
> 
> > Standard FAT implementations cannot store any of the FAT root directory's
> > timestamps. This commit adds the mount option 'rootts', which allows saving
> > the FAT root directory timestamps as the timestamps of the FAT volume label
> > directory entry. At least Mac OS X is known to support the same mechanism
> > and interoperate with this commit.
> >
> > When mounting, the following values can be specified for the 'rootts' mount
> > option:
> >
...
> >
> >   "rootts=save"     tries to load and save the root directory's timestamps.
> >                     The mtime and atime are corrected based on root
> >                     directory entries' ctime. If the root directory was
> >                     accessed but no volume label entry exists, the label
> >                     "NO NAME" is created.
> 
> Um..., I'm still not sure though. rootts=save may not be needed for
> this, because it seems a bit complex, and it can do on userland...

It's true, that is quite of a hack after all. I just thought I'll try to
add the non-plus-ultra-feature on top of what Mac OS can do already.

But you're right, I'll get rid of the label creation. (And I think I
should get rid of the time stamp correction as well. If there is no
timestamp to be loaded, then 1970/1/1 should suffice. In case anyone
thinks that the timestamp 'correction' is anyhow useful, let me know...)

> 
> And some quick review of implement..
> 
> > +	if (inode->i_ino == MSDOS_ROOT_INO) {
> 
> In this path, we don't have inode->i_mutex, so to look directry up is
> wrong. I think adding label may move to userland.
> 
> I guess we should do all of preparation in fill_super().

True. This code is only needed for the time stamp creation, so it'll go
in any case.

> 
> > +			remove_inode_hash(inode);
> > +			fat_attach(inode, i_pos);
> > +			insert_inode_hash(inode);
> > +			brelse(bh);
> 
> Root inode should never change the position, so this shouldn't be
> needed.

Like above, this will be removed.

If 'preserve' (i.e. saving and loading the timestamp only IF a label is
present) is the default, I'll remove the 'rootts=preserve' option.
However, should there still be an option to switch this feature off
completely ('rootts=ignore')?

Thanks very much for your comments, the next version will come soon!

Jörg

Re: [PATCH take 3][RFC] fat: Save FAT root directory timestamps to volume label

[OGAWA Hirofumi]

jorg@ml1.net writes:

> If 'preserve' (i.e. saving and loading the timestamp only IF a label is
> present) is the default, I'll remove the 'rootts=preserve' option.
> However, should there still be an option to switch this feature off
> completely ('rootts=ignore')?

I think it should be boolean (I.e. "rootts" as "rootts=preserve", and no
option as "rootts=ignore").

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>