On Mon, 11 Oct 1999, Mikulas Patocka wrote:
> > I mean the following: assume that call of hpfs_truncate() (via
> > ->truncate()) had been moved into the inode_setattr() (i.e. happens
> > immediately before the hpfs_write_inode() in hpfs_notify_change()). Will
> > it have any bad consequences? Right now the sequence looks so:
> > inode->i_size = newsize;
> > hpfs_write_inode(inode);
> > hpfs_truncate(inode);
> > Could we swap the last steps?
>
> Probably yes, the first hpfs_write_inode is not needed at all.
Aha. OK, since with this patch ->truncate() is called only from
inode_setattr() I'm leaving the call of hpfs_write_inode() in
hpfs_notify_change() and removing it from hpfs_truncate().
> You can do the same just by examining content of dnode (dirents have fnode
> numbers).
They do, but lookup in icache is going to be more expensive. Besides, we
will have to do it for every dirent when we are splitting dnode. And we
do not care for inodes not in core.
> > ->clear_inode() would exclude the sucker from the lists, all inodes marked
> > with I_FREEING (i.e. those that gave NULL upon igrab()) are skipped during
> > the cyclic list searches.
>
> There is another clash if you use async write_inode. Somebody is modifying
> directory, btree operation is in progress and now - system decides to call
> write_inode. You can't wait in write_inode until directory is in
> consistent state, you just must patch dirent somwhere in memory and
> return. That means - inode must hold pointer to dirent on disk or in
> memory.
No problems - in-core inode holds a location on disk, any dirent
moving starts from setting it to 0 (with spinlock held - see above) and
ends with setting it according to new location.
write_inode():
write fnode part;
retry:
loc = location;
if (!loc)
return;
get the block corresponding to loc;
grab the spinlock;
if (loc still points to the same block) {
update dirent;
mark it dirty;
release block;
release spinlock;
return;
}
/* we had been moved */
release block;
release spinlock;
goto retry;
That's it. This is what I've done in FAT and it doesn't take much code.
> It is possible to write it, but HPFS is currently very stable
and
> race-free (the last race I had was bug in kernel) and such modifications
> would surely make a lot of problems.
OK, I definitely agree with that. It's worth doing someday, but not now.
Anyway, the new variant of truncate patch follows. Please, check whether
you have problems with the hpfs part.
Cheers,
Al
diff -urN linux-2.3.20/fs/affs/inode.c linux-bird.truncate/fs/affs/inode.c
--- linux-2.3.20/fs/affs/inode.c Mon Jun 21 12:36:43 1999
+++ linux-bird.truncate/fs/affs/inode.c Mon Oct 11 10:55:11 1999
@@ -246,7 +246,6 @@
inode->u.affs_i.i_protect = mode_to_prot(attr->ia_mode);
inode_setattr(inode, attr);
- mark_inode_dirty(inode);
error = 0;
out:
return error;
diff -urN linux-2.3.20/fs/attr.c linux-bird.truncate/fs/attr.c
--- linux-2.3.20/fs/attr.c Mon Jun 21 12:36:42 1999
+++ linux-bird.truncate/fs/attr.c Mon Oct 11 10:54:44 1999
@@ -62,8 +62,6 @@
inode->i_uid = attr->ia_uid;
if (ia_valid & ATTR_GID)
inode->i_gid = attr->ia_gid;
- if (ia_valid & ATTR_SIZE)
- inode->i_size = attr->ia_size;
if (ia_valid & ATTR_ATIME)
inode->i_atime = attr->ia_atime;
if (ia_valid & ATTR_MTIME)
@@ -75,6 +73,12 @@
if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
inode->i_mode &= ~S_ISGID;
}
+ if (ia_valid & ATTR_SIZE) {
+ inode->i_size = attr->ia_size;
+ vmtruncate(inode, attr->ia_size);
+ if (inode->i_op && inode->i_op->truncate)
+ inode->i_op->truncate(inode);
+ }
mark_inode_dirty(inode);
}
diff -urN linux-2.3.20/fs/coda/inode.c linux-bird.truncate/fs/coda/inode.c
--- linux-2.3.20/fs/coda/inode.c Sun Sep 12 12:45:49 1999
+++ linux-bird.truncate/fs/coda/inode.c Mon Oct 11 10:59:23 1999
@@ -229,6 +229,8 @@
if ( !error ) {
coda_vattr_to_iattr(inode, &vattr);
coda_cache_clear_inode(inode);
+ if (iattr->ia_valid && ATTR_SIZE)
+ vmtruncate(inode, iattr->ia_size);
}
CDEBUG(D_SUPER, "inode.i_mode %o, error %d\n",
inode->i_mode, error);
diff -urN linux-2.3.20/fs/hpfs/file.c linux-bird.truncate/fs/hpfs/file.c
--- linux-2.3.20/fs/hpfs/file.c Sun Sep 12 06:00:35 1999
+++ linux-bird.truncate/fs/hpfs/file.c Mon Oct 11 17:44:44 1999
@@ -55,7 +55,6 @@
i->i_hpfs_n_secs = 0;
i->i_blocks = 1 + ((i->i_size + 511) >> 9);
hpfs_truncate_btree(i->i_sb, i->i_ino, 1, ((i->i_size + 511) >> 9));
- hpfs_write_inode(i);
}
int hpfs_getblk_block(struct inode *inode, long block, int create, int *err, int
*created)
diff -urN linux-2.3.20/fs/hpfs/namei.c linux-bird.truncate/fs/hpfs/namei.c
--- linux-2.3.20/fs/hpfs/namei.c Sun Sep 12 03:29:51 1999
+++ linux-bird.truncate/fs/hpfs/namei.c Sun Oct 10 14:35:51 1999
@@ -327,16 +327,14 @@
hpfs_unlock_2inodes(dir, inode);
if (rep || dentry->d_count > 1 || permission(inode, MAY_WRITE) ||
get_write_access(inode)) goto ret;
/*printk("HPFS: truncating file before delete.\n");*/
- down(&inode->i_sem); /* do_truncate should be called here, but it's
*/
- newattrs.ia_size = 0; /* not exported */
+ down(&inode->i_sem);
+ newattrs.ia_size = 0;
newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
if (notify_change(dentry, &newattrs)) {
up(&inode->i_sem);
put_write_access(inode);
goto ret;
}
- vmtruncate(inode, 0);
- if (inode->i_op && inode->i_op->truncate) inode->i_op->truncate(inode);
up(&inode->i_sem);
put_write_access(inode);
rep = 1;
diff -urN linux-2.3.20/fs/ncpfs/inode.c linux-bird.truncate/fs/ncpfs/inode.c
--- linux-2.3.20/fs/ncpfs/inode.c Sat Oct 9 16:10:12 1999
+++ linux-bird.truncate/fs/ncpfs/inode.c Mon Oct 11 11:00:32 1999
@@ -662,6 +662,8 @@
/* According to ndir, the changes only take effect after
closing the file */
result = ncp_make_closed(inode);
+ if (!result)
+ vmtruncate(inode, attr->ia_size);
}
out:
return result;
diff -urN linux-2.3.20/fs/nfs/inode.c linux-bird.truncate/fs/nfs/inode.c
--- linux-2.3.20/fs/nfs/inode.c Sun Sep 12 04:00:59 1999
+++ linux-bird.truncate/fs/nfs/inode.c Mon Oct 11 10:57:52 1999
@@ -677,6 +677,8 @@
if (sattr.mtime.seconds != (u32) -1)
inode->i_mtime = fattr.mtime.seconds;
error = nfs_refresh_inode(inode, &fattr);
+ if (!error && (sattr.size != (u32) -1))
+ vmtruncate(inode, sattr.size);
out:
return error;
}
diff -urN linux-2.3.20/fs/nfsd/vfs.c linux-bird.truncate/fs/nfsd/vfs.c
--- linux-2.3.20/fs/nfsd/vfs.c Sun Sep 12 13:29:57 1999
+++ linux-bird.truncate/fs/nfsd/vfs.c Sun Oct 10 14:30:09 1999
@@ -221,15 +221,19 @@
int ftype = 0;
int imode;
int err;
+ int do_size = 0;
+ kernel_cap_t saved_cap = 0;
if (iap->ia_valid & (ATTR_ATIME | ATTR_MTIME | ATTR_SIZE))
accmode |= MAY_WRITE;
- if (iap->ia_valid & ATTR_SIZE)
+ if (iap->ia_valid & ATTR_SIZE) {
ftype = S_IFREG;
+ do_size = 1;
+ }
/* Get inode */
err = fh_verify(rqstp, fhp, ftype, accmode);
- if (err)
+ if (err || !iap->ia_valid)
goto out;
dentry = fhp->fh_dentry;
@@ -240,7 +244,7 @@
goto out_nfserr;
/* The size case is special... */
- if (iap->ia_valid & ATTR_SIZE) {
+ if (do_size) {
if (!S_ISREG(inode->i_mode))
printk("nfsd_setattr: size change??\n");
if (iap->ia_size < inode->i_size) {
@@ -251,13 +255,6 @@
err = get_write_access(inode);
if (err)
goto out_nfserr;
- /* N.B. Should we update the inode cache here? */
- inode->i_size = iap->ia_size;
- if (inode->i_op && inode->i_op->truncate)
- inode->i_op->truncate(inode);
- mark_inode_dirty(inode);
- put_write_access(inode);
- iap->ia_valid &= ~ATTR_SIZE;
iap->ia_valid |= ATTR_MTIME;
iap->ia_mtime = CURRENT_TIME;
}
@@ -281,23 +278,23 @@
}
/* Change the attributes. */
- if (iap->ia_valid) {
- kernel_cap_t saved_cap = 0;
- iap->ia_valid |= ATTR_CTIME;
- iap->ia_ctime = CURRENT_TIME;
- if (current->fsuid != 0) {
- saved_cap = current->cap_effective;
- cap_clear(current->cap_effective);
- }
- err = notify_change(dentry, iap);
- if (current->fsuid != 0)
- current->cap_effective = saved_cap;
- if (err)
- goto out_nfserr;
- if (EX_ISSYNC(fhp->fh_export))
- write_inode_now(inode);
+ iap->ia_valid |= ATTR_CTIME;
+ iap->ia_ctime = CURRENT_TIME;
+ if (current->fsuid != 0) {
+ saved_cap = current->cap_effective;
+ cap_clear(current->cap_effective);
}
+ err = notify_change(dentry, iap);
+ if (current->fsuid != 0)
+ current->cap_effective = saved_cap;
+ if (do_size)
+ put_write_access(inode);
+ if (err)
+ goto out_nfserr;
+ if (EX_ISSYNC(fhp->fh_export))
+ write_inode_now(inode);
+
err = 0;
out:
return err;
@@ -786,11 +783,6 @@
err = notify_change(dentry, &newattrs);
if (current->fsuid != 0)
current->cap_effective = saved_cap;
- if (!err) {
- vmtruncate(inode, size);
- if (inode->i_op && inode->i_op->truncate)
- inode->i_op->truncate(inode);
- }
put_write_access(inode);
DQUOT_DROP(inode);
fh_unlock(fhp);
diff -urN linux-2.3.20/fs/open.c linux-bird.truncate/fs/open.c
--- linux-2.3.20/fs/open.c Sat Oct 9 16:10:12 1999
+++ linux-bird.truncate/fs/open.c Sun Oct 10 14:30:38 1999
@@ -69,12 +69,6 @@
newattrs.ia_size = length;
newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
error = notify_change(dentry, &newattrs);
- if (!error) {
- /* truncate virtual mappings of this file */
- vmtruncate(inode, length);
- if (inode->i_op && inode->i_op->truncate)
- inode->i_op->truncate(inode);
- }
up(&inode->i_sem);
return error;
}
diff -urN linux-2.3.20/fs/smbfs/inode.c linux-bird.truncate/fs/smbfs/inode.c
--- linux-2.3.20/fs/smbfs/inode.c Sun Sep 12 13:03:20 1999
+++ linux-bird.truncate/fs/smbfs/inode.c Mon Oct 11 11:01:58 1999
@@ -555,6 +555,8 @@
}
}
error = 0;
+ if (attr->ia_valid & ATTR_SIZE)
+ vmtruncate(inode, attr->ia_size);
out:
if (refresh)
