Re: ffs1 and the future

2020-02-19 Thread Otto Moerbeek
On Wed, Feb 19, 2020 at 07:23:13PM -0600, Scott Cheloha wrote:

> On Wed, Feb 19, 2020 at 05:26:40PM +0100, Otto Moerbeek wrote:
> > On Wed, Feb 19, 2020 at 05:10:11PM +0100, Otto Moerbeek wrote:
> > 
> > > On Wed, Feb 19, 2020 at 10:02:10AM -0600, Scott Cheloha wrote:
> > > 
> > > > On Wed, Feb 19, 2020 at 04:00:34PM +0100, Otto Moerbeek wrote:
> > > > > 
> > > > > [...]
> > > > > 
> > > > > FFS1, the default filesystem, uses 32-bit signed timestamps on disk.
> > > > > That means that in 2038, there's going to be a problem, timestamps
> > > > > will the be interperet as coming from the start of the 1900's.
> > > > > 
> > > > > FFS2 does not have this limitation, but at the moment, we cannot boot
> > > > > from it. I'm working on that as well, but for now I like to propose a
> > > > > diff that interprets all timestamps in FFS1 as unsigned.
> > > > > 
> > > > > * On disk format dos not change
> > > > > * Current timestamp values do not change
> > > > 
> > > > Doesn't this change the interpretation of timestamps before 1970?
> > > > 
> > > > Humor me:
> > > > 
> > > > # date 19690101
> > > > Wed Jan  1 00:00:00 CST 1969
> > > > # touch test && ls -l test
> > > > -rw-r--r--  1 ssc  ssc  0 Jan  1 00:00 test
> > > > # stat test
> > > > 1038 8878266 -rw-r--r-- 1 ssc ssc 0 0 "Jan  1 00:00:58 1969" "Jan  1 
> > > > 00:00:58 1969" "Jan  1 00:00:58 1969" 32768 0 0 test
> > > > 
> > > > ... so what happens to such files?  The timestamps wrap around?
> > > > 
> > > > I'm not sure if that should prevent us from implementing this stopgap,
> > > > but it's worth considering.
> > > > 
> > > > > I have checked various tools like dump(8) and restore(8), they work
> > > > > properly. Code normally works with the fields from struct stat, which
> > > > > is already 64-bit. I can imagine code setting timestamps to -1
> > > > > explicitly, that could cause surprises.
> > > > > 
> > > > > So I'm asking for wider testing of the diff below.
> > > > 
> > > > One small bug below.
> > > > 
> > > > > Index: ufs/ffs/ffs_alloc.c
> > > > > ===
> > > > > RCS file: /cvs/src/sys/ufs/ffs/ffs_alloc.c,v
> > > > > retrieving revision 1.109
> > > > > diff -u -p -r1.109 ffs_alloc.c
> > > > > --- ufs/ffs/ffs_alloc.c   19 Jul 2019 00:24:31 -  1.109
> > > > > +++ ufs/ffs/ffs_alloc.c   16 Feb 2020 19:33:07 -
> > > > > @@ -888,7 +888,8 @@ ffs_fragextend(struct inode *ip, int cg,
> > > > >   return (0);
> > > > >  
> > > > >   cgp = (struct cg *)bp->b_data;
> > > > > - cgp->cg_ffs2_time = cgp->cg_time = time_second;
> > > > > + cgp->cg_ffs2_time = time_second;
> > > > > + cgp->cg_time = time_second;
> > > > 
> > > > You shouldn't re-read time_second here unless you want to introduce a
> > > > possible difference between cg_ffs2_time and cg_time.
> > > > 
> > > > You should also, in general, avoid time_second.  There is a split-read
> > > > bug on 32-bit platforms at the 2038 cross-over.
> > > > 
> > > > That one I'm less certain about, though.  time_second assignment is
> > > > brief compared to the alternative:
> > > > 
> > > > struct timespec now;
> > > > 
> > > > nanotime();
> > > > cgp->cg_ffs2_time = now.tv_sec;
> > > > cgp->cg_time = now.tv_sec;
> > > > 
> > > > ... and the window for the split-read bug is very small...
> > > > 
> > > > At minimum, don't re-read time_second.
> > > 
> > > OK, I'll change that. Ne aware the filesystems is full of time_second.
> > 
> > It's not as bad as I thought.
> 
> The changes to from time_second(9) to nanotime(9) are ok cheloha@.
> 
> I'm a bit uneasy about changing the sign of the timestamp if the
> ultimate solution is just "switch to ffs2".
> 
> If that's the case, why not make ffs2 bootable and leave ffs1 as-is?

We could do that, but making ffs2 bootable on all platforms is quite a
job. Also, ffs2 has a larger meta-data overhead. So for small
filesystems (think floppy and other bootblocks) it wastes more space.
Same for mfs filesystems.

I like to keep ffs1 usable for the future.

-Otto






Re: ffs1 and the future

2020-02-19 Thread Scott Cheloha
On Wed, Feb 19, 2020 at 05:26:40PM +0100, Otto Moerbeek wrote:
> On Wed, Feb 19, 2020 at 05:10:11PM +0100, Otto Moerbeek wrote:
> 
> > On Wed, Feb 19, 2020 at 10:02:10AM -0600, Scott Cheloha wrote:
> > 
> > > On Wed, Feb 19, 2020 at 04:00:34PM +0100, Otto Moerbeek wrote:
> > > > 
> > > > [...]
> > > > 
> > > > FFS1, the default filesystem, uses 32-bit signed timestamps on disk.
> > > > That means that in 2038, there's going to be a problem, timestamps
> > > > will the be interperet as coming from the start of the 1900's.
> > > > 
> > > > FFS2 does not have this limitation, but at the moment, we cannot boot
> > > > from it. I'm working on that as well, but for now I like to propose a
> > > > diff that interprets all timestamps in FFS1 as unsigned.
> > > > 
> > > > * On disk format dos not change
> > > > * Current timestamp values do not change
> > > 
> > > Doesn't this change the interpretation of timestamps before 1970?
> > > 
> > > Humor me:
> > > 
> > > # date 19690101
> > > Wed Jan  1 00:00:00 CST 1969
> > > # touch test && ls -l test
> > > -rw-r--r--  1 ssc  ssc  0 Jan  1 00:00 test
> > > # stat test
> > > 1038 8878266 -rw-r--r-- 1 ssc ssc 0 0 "Jan  1 00:00:58 1969" "Jan  1 
> > > 00:00:58 1969" "Jan  1 00:00:58 1969" 32768 0 0 test
> > > 
> > > ... so what happens to such files?  The timestamps wrap around?
> > > 
> > > I'm not sure if that should prevent us from implementing this stopgap,
> > > but it's worth considering.
> > > 
> > > > I have checked various tools like dump(8) and restore(8), they work
> > > > properly. Code normally works with the fields from struct stat, which
> > > > is already 64-bit. I can imagine code setting timestamps to -1
> > > > explicitly, that could cause surprises.
> > > > 
> > > > So I'm asking for wider testing of the diff below.
> > > 
> > > One small bug below.
> > > 
> > > > Index: ufs/ffs/ffs_alloc.c
> > > > ===
> > > > RCS file: /cvs/src/sys/ufs/ffs/ffs_alloc.c,v
> > > > retrieving revision 1.109
> > > > diff -u -p -r1.109 ffs_alloc.c
> > > > --- ufs/ffs/ffs_alloc.c 19 Jul 2019 00:24:31 -  1.109
> > > > +++ ufs/ffs/ffs_alloc.c 16 Feb 2020 19:33:07 -
> > > > @@ -888,7 +888,8 @@ ffs_fragextend(struct inode *ip, int cg,
> > > > return (0);
> > > >  
> > > > cgp = (struct cg *)bp->b_data;
> > > > -   cgp->cg_ffs2_time = cgp->cg_time = time_second;
> > > > +   cgp->cg_ffs2_time = time_second;
> > > > +   cgp->cg_time = time_second;
> > > 
> > > You shouldn't re-read time_second here unless you want to introduce a
> > > possible difference between cg_ffs2_time and cg_time.
> > > 
> > > You should also, in general, avoid time_second.  There is a split-read
> > > bug on 32-bit platforms at the 2038 cross-over.
> > > 
> > > That one I'm less certain about, though.  time_second assignment is
> > > brief compared to the alternative:
> > > 
> > >   struct timespec now;
> > > 
> > >   nanotime();
> > >   cgp->cg_ffs2_time = now.tv_sec;
> > >   cgp->cg_time = now.tv_sec;
> > > 
> > > ... and the window for the split-read bug is very small...
> > > 
> > > At minimum, don't re-read time_second.
> > 
> > OK, I'll change that. Ne aware the filesystems is full of time_second.
> 
> It's not as bad as I thought.

The changes to from time_second(9) to nanotime(9) are ok cheloha@.

I'm a bit uneasy about changing the sign of the timestamp if the
ultimate solution is just "switch to ffs2".

If that's the case, why not make ffs2 bootable and leave ffs1 as-is?



Re: ffs1 and the future

2020-02-19 Thread Otto Moerbeek
On Wed, Feb 19, 2020 at 05:10:11PM +0100, Otto Moerbeek wrote:

> On Wed, Feb 19, 2020 at 10:02:10AM -0600, Scott Cheloha wrote:
> 
> > On Wed, Feb 19, 2020 at 04:00:34PM +0100, Otto Moerbeek wrote:
> > > 
> > > [...]
> > > 
> > > FFS1, the default filesystem, uses 32-bit signed timestamps on disk.
> > > That means that in 2038, there's going to be a problem, timestamps
> > > will the be interperet as coming from the start of the 1900's.
> > > 
> > > FFS2 does not have this limitation, but at the moment, we cannot boot
> > > from it. I'm working on that as well, but for now I like to propose a
> > > diff that interprets all timestamps in FFS1 as unsigned.
> > > 
> > > * On disk format dos not change
> > > * Current timestamp values do not change
> > 
> > Doesn't this change the interpretation of timestamps before 1970?
> > 
> > Humor me:
> > 
> > # date 19690101
> > Wed Jan  1 00:00:00 CST 1969
> > # touch test && ls -l test
> > -rw-r--r--  1 ssc  ssc  0 Jan  1 00:00 test
> > # stat test
> > 1038 8878266 -rw-r--r-- 1 ssc ssc 0 0 "Jan  1 00:00:58 1969" "Jan  1 
> > 00:00:58 1969" "Jan  1 00:00:58 1969" 32768 0 0 test
> > 
> > ... so what happens to such files?  The timestamps wrap around?
> > 
> > I'm not sure if that should prevent us from implementing this stopgap,
> > but it's worth considering.
> > 
> > > I have checked various tools like dump(8) and restore(8), they work
> > > properly. Code normally works with the fields from struct stat, which
> > > is already 64-bit. I can imagine code setting timestamps to -1
> > > explicitly, that could cause surprises.
> > > 
> > > So I'm asking for wider testing of the diff below.
> > 
> > One small bug below.
> > 
> > > Index: ufs/ffs/ffs_alloc.c
> > > ===
> > > RCS file: /cvs/src/sys/ufs/ffs/ffs_alloc.c,v
> > > retrieving revision 1.109
> > > diff -u -p -r1.109 ffs_alloc.c
> > > --- ufs/ffs/ffs_alloc.c   19 Jul 2019 00:24:31 -  1.109
> > > +++ ufs/ffs/ffs_alloc.c   16 Feb 2020 19:33:07 -
> > > @@ -888,7 +888,8 @@ ffs_fragextend(struct inode *ip, int cg,
> > >   return (0);
> > >  
> > >   cgp = (struct cg *)bp->b_data;
> > > - cgp->cg_ffs2_time = cgp->cg_time = time_second;
> > > + cgp->cg_ffs2_time = time_second;
> > > + cgp->cg_time = time_second;
> > 
> > You shouldn't re-read time_second here unless you want to introduce a
> > possible difference between cg_ffs2_time and cg_time.
> > 
> > You should also, in general, avoid time_second.  There is a split-read
> > bug on 32-bit platforms at the 2038 cross-over.
> > 
> > That one I'm less certain about, though.  time_second assignment is
> > brief compared to the alternative:
> > 
> > struct timespec now;
> > 
> > nanotime();
> > cgp->cg_ffs2_time = now.tv_sec;
> > cgp->cg_time = now.tv_sec;
> > 
> > ... and the window for the split-read bug is very small...
> > 
> > At minimum, don't re-read time_second.
> 
> OK, I'll change that. Ne aware the filesystems is full of time_second.

It's not as bad as I thought.

-Otto

Index: ufs/ffs/ffs_alloc.c
===
RCS file: /cvs/src/sys/ufs/ffs/ffs_alloc.c,v
retrieving revision 1.109
diff -u -p -r1.109 ffs_alloc.c
--- ufs/ffs/ffs_alloc.c 19 Jul 2019 00:24:31 -  1.109
+++ ufs/ffs/ffs_alloc.c 19 Feb 2020 16:24:23 -
@@ -871,6 +871,7 @@ ffs_fragextend(struct inode *ip, int cg,
struct fs *fs;
struct cg *cgp;
struct buf *bp;
+   struct timespec now;
daddr_t bno;
int i, frags, bbase;
 
@@ -888,7 +889,9 @@ ffs_fragextend(struct inode *ip, int cg,
return (0);
 
cgp = (struct cg *)bp->b_data;
-   cgp->cg_ffs2_time = cgp->cg_time = time_second;
+   nanotime();
+   cgp->cg_ffs2_time = now.tv_sec;
+   cgp->cg_time = now.tv_sec;
 
bno = dtogd(fs, bprev);
for (i = numfrags(fs, osize); i < frags; i++)
@@ -934,6 +937,7 @@ ffs_alloccg(struct inode *ip, int cg, da
struct fs *fs;
struct cg *cgp;
struct buf *bp;
+   struct timespec now;
daddr_t bno, blkno;
int i, frags, allocsiz;
 
@@ -950,7 +954,9 @@ ffs_alloccg(struct inode *ip, int cg, da
return (0);
}
 
-   cgp->cg_ffs2_time = cgp->cg_time = time_second;
+   nanotime();
+   cgp->cg_ffs2_time = now.tv_sec;
+   cgp->cg_time = now.tv_sec;
 
if (size == fs->fs_bsize) {
/* allocate and return a complete data block */
@@ -1086,6 +1092,7 @@ ffs_nodealloccg(struct inode *ip, int cg
struct fs *fs;
struct cg *cgp;
struct buf *bp;
+   struct timespec now;
int start, len, loc, map, i;
 #ifdef FFS2
struct buf *ibp = NULL;
@@ -1114,7 +1121,9 @@ ffs_nodealloccg(struct inode *ip, int cg
 * We are committed to the allocation from now on, so update the time
 * on the cylinder group.
 */
-   

Re: ffs1 and the future

2020-02-19 Thread Otto Moerbeek
On Wed, Feb 19, 2020 at 10:02:10AM -0600, Scott Cheloha wrote:

> On Wed, Feb 19, 2020 at 04:00:34PM +0100, Otto Moerbeek wrote:
> > 
> > [...]
> > 
> > FFS1, the default filesystem, uses 32-bit signed timestamps on disk.
> > That means that in 2038, there's going to be a problem, timestamps
> > will the be interperet as coming from the start of the 1900's.
> > 
> > FFS2 does not have this limitation, but at the moment, we cannot boot
> > from it. I'm working on that as well, but for now I like to propose a
> > diff that interprets all timestamps in FFS1 as unsigned.
> > 
> > * On disk format dos not change
> > * Current timestamp values do not change
> 
> Doesn't this change the interpretation of timestamps before 1970?
> 
> Humor me:
> 
> # date 19690101
> Wed Jan  1 00:00:00 CST 1969
> # touch test && ls -l test
> -rw-r--r--  1 ssc  ssc  0 Jan  1 00:00 test
> # stat test
> 1038 8878266 -rw-r--r-- 1 ssc ssc 0 0 "Jan  1 00:00:58 1969" "Jan  1 00:00:58 
> 1969" "Jan  1 00:00:58 1969" 32768 0 0 test
> 
> ... so what happens to such files?  The timestamps wrap around?
> 
> I'm not sure if that should prevent us from implementing this stopgap,
> but it's worth considering.
> 
> > I have checked various tools like dump(8) and restore(8), they work
> > properly. Code normally works with the fields from struct stat, which
> > is already 64-bit. I can imagine code setting timestamps to -1
> > explicitly, that could cause surprises.
> > 
> > So I'm asking for wider testing of the diff below.
> 
> One small bug below.
> 
> > Index: ufs/ffs/ffs_alloc.c
> > ===
> > RCS file: /cvs/src/sys/ufs/ffs/ffs_alloc.c,v
> > retrieving revision 1.109
> > diff -u -p -r1.109 ffs_alloc.c
> > --- ufs/ffs/ffs_alloc.c 19 Jul 2019 00:24:31 -  1.109
> > +++ ufs/ffs/ffs_alloc.c 16 Feb 2020 19:33:07 -
> > @@ -888,7 +888,8 @@ ffs_fragextend(struct inode *ip, int cg,
> > return (0);
> >  
> > cgp = (struct cg *)bp->b_data;
> > -   cgp->cg_ffs2_time = cgp->cg_time = time_second;
> > +   cgp->cg_ffs2_time = time_second;
> > +   cgp->cg_time = time_second;
> 
> You shouldn't re-read time_second here unless you want to introduce a
> possible difference between cg_ffs2_time and cg_time.
> 
> You should also, in general, avoid time_second.  There is a split-read
> bug on 32-bit platforms at the 2038 cross-over.
> 
> That one I'm less certain about, though.  time_second assignment is
> brief compared to the alternative:
> 
>   struct timespec now;
> 
>   nanotime();
>   cgp->cg_ffs2_time = now.tv_sec;
>   cgp->cg_time = now.tv_sec;
> 
> ... and the window for the split-read bug is very small...
> 
> At minimum, don't re-read time_second.

OK, I'll change that. Ne aware the filesystems is full of time_second.

-Otto

> 
> > bno = dtogd(fs, bprev);
> > for (i = numfrags(fs, osize); i < frags; i++)
> > Index: ufs/ffs/fs.h
> > ===
> > RCS file: /cvs/src/sys/ufs/ffs/fs.h,v
> > retrieving revision 1.42
> > diff -u -p -r1.42 fs.h
> > --- ufs/ffs/fs.h27 Nov 2016 13:27:55 -  1.42
> > +++ ufs/ffs/fs.h16 Feb 2020 19:33:07 -
> > @@ -199,7 +199,7 @@ struct fs {
> > int32_t  fs_dblkno; /* offset of first data / frags */
> > int32_t  fs_cgoffset;   /* cylinder group offset in cylinder */
> > int32_t  fs_cgmask; /* used to calc mod fs_ntrak */
> > -   int32_t  fs_ffs1_time;  /* last time written */
> > +   u_int32_t fs_ffs1_time; /* last time written */
> > int32_t  fs_ffs1_size;  /* # of blocks in fs / frags */
> > int32_t  fs_ffs1_dsize; /* # of data blocks in fs */
> > int32_t  fs_ncg;/* # of cylinder groups */
> > @@ -285,7 +285,7 @@ struct fs {
> > int32_t  fs_avgfpdir;   /* expected # of files per directory */
> > int32_t  fs_sparecon[26];   /* reserved for future constants */
> > u_int32_t fs_flags; /* see FS_ flags below */
> > -   int32_t  fs_fscktime;   /* last time fsck(8)ed */
> > +   u_int32_t fs_fscktime;  /* last time fsck(8)ed */
> > int32_t  fs_contigsumsize;  /* size of cluster summary array */ 
> > int32_t  fs_maxsymlinklen;  /* max length of an internal symlink */
> > int32_t  fs_inodefmt;   /* format of on-disk inodes */
> > @@ -376,7 +376,7 @@ struct fs {
> >  struct cg {
> > int32_t  cg_firstfield; /* historic cyl groups linked list */
> > int32_t  cg_magic;  /* magic number */
> > -   int32_t  cg_time;   /* time last written */
> > +   u_int32_t cg_time;  /* time last written */
> > int32_t  cg_cgx;/* we are the cgx'th cylinder group */
> > int16_t  cg_ncyl;   /* number of cyl's this cg */
> > int16_t  cg_niblk;  /* number of inode blocks 

Re: ffs1 and the future

2020-02-19 Thread Otto Moerbeek
On Wed, Feb 19, 2020 at 10:02:10AM -0600, Scott Cheloha wrote:

> On Wed, Feb 19, 2020 at 04:00:34PM +0100, Otto Moerbeek wrote:
> > 
> > [...]
> > 
> > FFS1, the default filesystem, uses 32-bit signed timestamps on disk.
> > That means that in 2038, there's going to be a problem, timestamps
> > will the be interperet as coming from the start of the 1900's.
> > 
> > FFS2 does not have this limitation, but at the moment, we cannot boot
> > from it. I'm working on that as well, but for now I like to propose a
> > diff that interprets all timestamps in FFS1 as unsigned.
> > 
> > * On disk format dos not change
> > * Current timestamp values do not change
> 
> Doesn't this change the interpretation of timestamps before 1970?

yes, that seemed obvious to me...

> 
> Humor me:
> 
> # date 19690101
> Wed Jan  1 00:00:00 CST 1969
> # touch test && ls -l test
> -rw-r--r--  1 ssc  ssc  0 Jan  1 00:00 test
> # stat test
> 1038 8878266 -rw-r--r-- 1 ssc ssc 0 0 "Jan  1 00:00:58 1969" "Jan  1 00:00:58 
> 1969" "Jan  1 00:00:58 1969" 32768 0 0 test
> 
> ... so what happens to such files?  The timestamps wrap around?
> 
> I'm not sure if that should prevent us from implementing this stopgap,
> but it's worth considering.
> 
> > I have checked various tools like dump(8) and restore(8), they work
> > properly. Code normally works with the fields from struct stat, which
> > is already 64-bit. I can imagine code setting timestamps to -1
> > explicitly, that could cause surprises.
> > 
> > So I'm asking for wider testing of the diff below.
> 
> One small bug below.
> 
> > Index: ufs/ffs/ffs_alloc.c
> > ===
> > RCS file: /cvs/src/sys/ufs/ffs/ffs_alloc.c,v
> > retrieving revision 1.109
> > diff -u -p -r1.109 ffs_alloc.c
> > --- ufs/ffs/ffs_alloc.c 19 Jul 2019 00:24:31 -  1.109
> > +++ ufs/ffs/ffs_alloc.c 16 Feb 2020 19:33:07 -
> > @@ -888,7 +888,8 @@ ffs_fragextend(struct inode *ip, int cg,
> > return (0);
> >  
> > cgp = (struct cg *)bp->b_data;
> > -   cgp->cg_ffs2_time = cgp->cg_time = time_second;
> > +   cgp->cg_ffs2_time = time_second;
> > +   cgp->cg_time = time_second;
> 
> You shouldn't re-read time_second here unless you want to introduce a
> possible difference between cg_ffs2_time and cg_time.
> 
> You should also, in general, avoid time_second.  There is a split-read
> bug on 32-bit platforms at the 2038 cross-over.
> 
> That one I'm less certain about, though.  time_second assignment is
> brief compared to the alternative:
> 
>   struct timespec now;
> 
>   nanotime();
>   cgp->cg_ffs2_time = now.tv_sec;
>   cgp->cg_time = now.tv_sec;
> 
> ... and the window for the split-read bug is very small...
> 
> At minimum, don't re-read time_second.
> 
> > bno = dtogd(fs, bprev);
> > for (i = numfrags(fs, osize); i < frags; i++)
> > Index: ufs/ffs/fs.h
> > ===
> > RCS file: /cvs/src/sys/ufs/ffs/fs.h,v
> > retrieving revision 1.42
> > diff -u -p -r1.42 fs.h
> > --- ufs/ffs/fs.h27 Nov 2016 13:27:55 -  1.42
> > +++ ufs/ffs/fs.h16 Feb 2020 19:33:07 -
> > @@ -199,7 +199,7 @@ struct fs {
> > int32_t  fs_dblkno; /* offset of first data / frags */
> > int32_t  fs_cgoffset;   /* cylinder group offset in cylinder */
> > int32_t  fs_cgmask; /* used to calc mod fs_ntrak */
> > -   int32_t  fs_ffs1_time;  /* last time written */
> > +   u_int32_t fs_ffs1_time; /* last time written */
> > int32_t  fs_ffs1_size;  /* # of blocks in fs / frags */
> > int32_t  fs_ffs1_dsize; /* # of data blocks in fs */
> > int32_t  fs_ncg;/* # of cylinder groups */
> > @@ -285,7 +285,7 @@ struct fs {
> > int32_t  fs_avgfpdir;   /* expected # of files per directory */
> > int32_t  fs_sparecon[26];   /* reserved for future constants */
> > u_int32_t fs_flags; /* see FS_ flags below */
> > -   int32_t  fs_fscktime;   /* last time fsck(8)ed */
> > +   u_int32_t fs_fscktime;  /* last time fsck(8)ed */
> > int32_t  fs_contigsumsize;  /* size of cluster summary array */ 
> > int32_t  fs_maxsymlinklen;  /* max length of an internal symlink */
> > int32_t  fs_inodefmt;   /* format of on-disk inodes */
> > @@ -376,7 +376,7 @@ struct fs {
> >  struct cg {
> > int32_t  cg_firstfield; /* historic cyl groups linked list */
> > int32_t  cg_magic;  /* magic number */
> > -   int32_t  cg_time;   /* time last written */
> > +   u_int32_t cg_time;  /* time last written */
> > int32_t  cg_cgx;/* we are the cgx'th cylinder group */
> > int16_t  cg_ncyl;   /* number of cyl's this cg */
> > int16_t  cg_niblk;  /* number of inode blocks this cg */
> > Index: ufs/ufs/dinode.h
> > 

Re: ffs1 and the future

2020-02-19 Thread Scott Cheloha
On Wed, Feb 19, 2020 at 04:00:34PM +0100, Otto Moerbeek wrote:
> 
> [...]
> 
> FFS1, the default filesystem, uses 32-bit signed timestamps on disk.
> That means that in 2038, there's going to be a problem, timestamps
> will the be interperet as coming from the start of the 1900's.
> 
> FFS2 does not have this limitation, but at the moment, we cannot boot
> from it. I'm working on that as well, but for now I like to propose a
> diff that interprets all timestamps in FFS1 as unsigned.
> 
> * On disk format dos not change
> * Current timestamp values do not change

Doesn't this change the interpretation of timestamps before 1970?

Humor me:

# date 19690101
Wed Jan  1 00:00:00 CST 1969
# touch test && ls -l test
-rw-r--r--  1 ssc  ssc  0 Jan  1 00:00 test
# stat test
1038 8878266 -rw-r--r-- 1 ssc ssc 0 0 "Jan  1 00:00:58 1969" "Jan  1 00:00:58 
1969" "Jan  1 00:00:58 1969" 32768 0 0 test

... so what happens to such files?  The timestamps wrap around?

I'm not sure if that should prevent us from implementing this stopgap,
but it's worth considering.

> I have checked various tools like dump(8) and restore(8), they work
> properly. Code normally works with the fields from struct stat, which
> is already 64-bit. I can imagine code setting timestamps to -1
> explicitly, that could cause surprises.
> 
> So I'm asking for wider testing of the diff below.

One small bug below.

> Index: ufs/ffs/ffs_alloc.c
> ===
> RCS file: /cvs/src/sys/ufs/ffs/ffs_alloc.c,v
> retrieving revision 1.109
> diff -u -p -r1.109 ffs_alloc.c
> --- ufs/ffs/ffs_alloc.c   19 Jul 2019 00:24:31 -  1.109
> +++ ufs/ffs/ffs_alloc.c   16 Feb 2020 19:33:07 -
> @@ -888,7 +888,8 @@ ffs_fragextend(struct inode *ip, int cg,
>   return (0);
>  
>   cgp = (struct cg *)bp->b_data;
> - cgp->cg_ffs2_time = cgp->cg_time = time_second;
> + cgp->cg_ffs2_time = time_second;
> + cgp->cg_time = time_second;

You shouldn't re-read time_second here unless you want to introduce a
possible difference between cg_ffs2_time and cg_time.

You should also, in general, avoid time_second.  There is a split-read
bug on 32-bit platforms at the 2038 cross-over.

That one I'm less certain about, though.  time_second assignment is
brief compared to the alternative:

struct timespec now;

nanotime();
cgp->cg_ffs2_time = now.tv_sec;
cgp->cg_time = now.tv_sec;

... and the window for the split-read bug is very small...

At minimum, don't re-read time_second.

>   bno = dtogd(fs, bprev);
>   for (i = numfrags(fs, osize); i < frags; i++)
> Index: ufs/ffs/fs.h
> ===
> RCS file: /cvs/src/sys/ufs/ffs/fs.h,v
> retrieving revision 1.42
> diff -u -p -r1.42 fs.h
> --- ufs/ffs/fs.h  27 Nov 2016 13:27:55 -  1.42
> +++ ufs/ffs/fs.h  16 Feb 2020 19:33:07 -
> @@ -199,7 +199,7 @@ struct fs {
>   int32_t  fs_dblkno; /* offset of first data / frags */
>   int32_t  fs_cgoffset;   /* cylinder group offset in cylinder */
>   int32_t  fs_cgmask; /* used to calc mod fs_ntrak */
> - int32_t  fs_ffs1_time;  /* last time written */
> + u_int32_t fs_ffs1_time; /* last time written */
>   int32_t  fs_ffs1_size;  /* # of blocks in fs / frags */
>   int32_t  fs_ffs1_dsize; /* # of data blocks in fs */
>   int32_t  fs_ncg;/* # of cylinder groups */
> @@ -285,7 +285,7 @@ struct fs {
>   int32_t  fs_avgfpdir;   /* expected # of files per directory */
>   int32_t  fs_sparecon[26];   /* reserved for future constants */
>   u_int32_t fs_flags; /* see FS_ flags below */
> - int32_t  fs_fscktime;   /* last time fsck(8)ed */
> + u_int32_t fs_fscktime;  /* last time fsck(8)ed */
>   int32_t  fs_contigsumsize;  /* size of cluster summary array */ 
>   int32_t  fs_maxsymlinklen;  /* max length of an internal symlink */
>   int32_t  fs_inodefmt;   /* format of on-disk inodes */
> @@ -376,7 +376,7 @@ struct fs {
>  struct cg {
>   int32_t  cg_firstfield; /* historic cyl groups linked list */
>   int32_t  cg_magic;  /* magic number */
> - int32_t  cg_time;   /* time last written */
> + u_int32_t cg_time;  /* time last written */
>   int32_t  cg_cgx;/* we are the cgx'th cylinder group */
>   int16_t  cg_ncyl;   /* number of cyl's this cg */
>   int16_t  cg_niblk;  /* number of inode blocks this cg */
> Index: ufs/ufs/dinode.h
> ===
> RCS file: /cvs/src/sys/ufs/ufs/dinode.h,v
> retrieving revision 1.18
> diff -u -p -r1.18 dinode.h
> --- ufs/ufs/dinode.h  30 May 2013 19:19:09 -  1.18
> +++ ufs/ufs/dinode.h  16 Feb 

ffs1 and the future

2020-02-19 Thread Otto Moerbeek
Hoi,

FFS1, the default filesystem, uses 32-bit signed timestamps on disk.
That means that in 2038, there's going to be a problem, timestamps
will the be interperet as coming from the start of the 1900's.

FFS2 does not have this limitation, but at the moment, we cannot boot
from it. I'm working on that as well, but for now I like to propose a
diff that interprets all timestamps in FFS1 as unsigned.

* On disk format dos not change
* Current timestamp values do not change

I have checked various tools like dump(8) and restore(8), they work
properly. Code normally works with the fields from struct stat, which
is already 64-bit. I can imagine code setting timestamps to -1
explicitly, that could cause surprises.

So I'm asking for wider testing of the diff below.

-Otto

Index: ufs/ffs/ffs_alloc.c
===
RCS file: /cvs/src/sys/ufs/ffs/ffs_alloc.c,v
retrieving revision 1.109
diff -u -p -r1.109 ffs_alloc.c
--- ufs/ffs/ffs_alloc.c 19 Jul 2019 00:24:31 -  1.109
+++ ufs/ffs/ffs_alloc.c 16 Feb 2020 19:33:07 -
@@ -888,7 +888,8 @@ ffs_fragextend(struct inode *ip, int cg,
return (0);
 
cgp = (struct cg *)bp->b_data;
-   cgp->cg_ffs2_time = cgp->cg_time = time_second;
+   cgp->cg_ffs2_time = time_second;
+   cgp->cg_time = time_second;
 
bno = dtogd(fs, bprev);
for (i = numfrags(fs, osize); i < frags; i++)
Index: ufs/ffs/fs.h
===
RCS file: /cvs/src/sys/ufs/ffs/fs.h,v
retrieving revision 1.42
diff -u -p -r1.42 fs.h
--- ufs/ffs/fs.h27 Nov 2016 13:27:55 -  1.42
+++ ufs/ffs/fs.h16 Feb 2020 19:33:07 -
@@ -199,7 +199,7 @@ struct fs {
int32_t  fs_dblkno; /* offset of first data / frags */
int32_t  fs_cgoffset;   /* cylinder group offset in cylinder */
int32_t  fs_cgmask; /* used to calc mod fs_ntrak */
-   int32_t  fs_ffs1_time;  /* last time written */
+   u_int32_t fs_ffs1_time; /* last time written */
int32_t  fs_ffs1_size;  /* # of blocks in fs / frags */
int32_t  fs_ffs1_dsize; /* # of data blocks in fs */
int32_t  fs_ncg;/* # of cylinder groups */
@@ -285,7 +285,7 @@ struct fs {
int32_t  fs_avgfpdir;   /* expected # of files per directory */
int32_t  fs_sparecon[26];   /* reserved for future constants */
u_int32_t fs_flags; /* see FS_ flags below */
-   int32_t  fs_fscktime;   /* last time fsck(8)ed */
+   u_int32_t fs_fscktime;  /* last time fsck(8)ed */
int32_t  fs_contigsumsize;  /* size of cluster summary array */ 
int32_t  fs_maxsymlinklen;  /* max length of an internal symlink */
int32_t  fs_inodefmt;   /* format of on-disk inodes */
@@ -376,7 +376,7 @@ struct fs {
 struct cg {
int32_t  cg_firstfield; /* historic cyl groups linked list */
int32_t  cg_magic;  /* magic number */
-   int32_t  cg_time;   /* time last written */
+   u_int32_t cg_time;  /* time last written */
int32_t  cg_cgx;/* we are the cgx'th cylinder group */
int16_t  cg_ncyl;   /* number of cyl's this cg */
int16_t  cg_niblk;  /* number of inode blocks this cg */
Index: ufs/ufs/dinode.h
===
RCS file: /cvs/src/sys/ufs/ufs/dinode.h,v
retrieving revision 1.18
diff -u -p -r1.18 dinode.h
--- ufs/ufs/dinode.h30 May 2013 19:19:09 -  1.18
+++ ufs/ufs/dinode.h16 Feb 2020 19:33:07 -
@@ -72,11 +72,11 @@ struct  ufs1_dinode {
u_int32_t inumber;  /*   4: Lfs: inode number. */
} di_u;
u_int64_t   di_size;/*   8: File byte count. */
-   int32_t di_atime;   /*  16: Last access time. */
+   u_int32_t   di_atime;   /*  16: Last access time. */
int32_t di_atimensec;   /*  20: Last access time. */
-   int32_t di_mtime;   /*  24: Last modified time. */
+   u_int32_t   di_mtime;   /*  24: Last modified time. */
int32_t di_mtimensec;   /*  28: Last modified time. */
-   int32_t di_ctime;   /*  32: Last inode change time. */
+   u_int32_t   di_ctime;   /*  32: Last inode change time. */
int32_t di_ctimensec;   /*  36: Last inode change time. */
int32_t di_db[NDADDR];  /*  40: Direct disk blocks. */
int32_t di_ib[NIADDR];  /*  88: Indirect disk blocks. */