* [PATCH 1/4] simplify validata_fields
@ 2007-09-14 16:27 Christoph Hellwig
2007-09-18 3:56 ` Lachlan McIlroy
0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2007-09-14 16:27 UTC (permalink / raw)
To: xfs
Stop using xfs_getattr and a onstack bhv_vattr_t just to get three
fields from the underlying inode and opencode copying from the inode
fields instead.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_iops.c 2007-09-06 10:13:41.000000000 +0200
+++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c 2007-09-06 10:18:06.000000000 +0200
@@ -179,18 +179,19 @@ xfs_ichgtime_fast(
*/
STATIC void
xfs_validate_fields(
- struct inode *ip,
- bhv_vattr_t *vattr)
+ struct inode *inode)
{
- vattr->va_mask = XFS_AT_NLINK|XFS_AT_SIZE|XFS_AT_NBLOCKS;
- if (!xfs_getattr(XFS_I(ip), vattr, ATTR_LAZY)) {
- ip->i_nlink = vattr->va_nlink;
- ip->i_blocks = vattr->va_nblocks;
-
- /* we're under i_sem so i_size can't change under us */
- if (i_size_read(ip) != vattr->va_size)
- i_size_write(ip, vattr->va_size);
- }
+ struct xfs_inode *ip = XFS_I(inode);
+ loff_t size;
+
+ inode->i_nlink = ip->i_d.di_nlink;
+ inode->i_blocks =
+ XFS_FSB_TO_BB(ip->i_mount, ip->i_d.di_nblocks +
+ ip->i_delayed_blks);
+ /* we're under i_sem so i_size can't change under us */
+ size = XFS_ISIZE(ip);
+ if (i_size_read(inode) != size)
+ i_size_write(inode, size);
}
/*
@@ -340,9 +341,9 @@ xfs_vn_mknod(
if (S_ISCHR(mode) || S_ISBLK(mode))
ip->i_rdev = rdev;
else if (S_ISDIR(mode))
- xfs_validate_fields(ip, &vattr);
+ xfs_validate_fields(ip);
d_instantiate(dentry, ip);
- xfs_validate_fields(dir, &vattr);
+ xfs_validate_fields(dir);
}
return -error;
}
@@ -397,7 +398,6 @@ xfs_vn_link(
{
struct inode *ip; /* inode of guy being linked to */
bhv_vnode_t *vp; /* vp of name being linked */
- bhv_vattr_t vattr;
int error;
ip = old_dentry->d_inode; /* inode being linked to */
@@ -409,7 +409,7 @@ xfs_vn_link(
VN_RELE(vp);
} else {
xfs_iflags_set(XFS_I(dir), XFS_IMODIFIED);
- xfs_validate_fields(ip, &vattr);
+ xfs_validate_fields(ip);
d_instantiate(dentry, ip);
}
return -error;
@@ -421,15 +421,14 @@ xfs_vn_unlink(
struct dentry *dentry)
{
struct inode *inode;
- bhv_vattr_t vattr;
int error;
inode = dentry->d_inode;
error = xfs_remove(XFS_I(dir), dentry);
if (likely(!error)) {
- xfs_validate_fields(dir, &vattr); /* size needs update */
- xfs_validate_fields(inode, &vattr);
+ xfs_validate_fields(dir); /* size needs update */
+ xfs_validate_fields(inode);
}
return -error;
}
@@ -458,8 +457,8 @@ xfs_vn_symlink(
if (likely(!error)) {
ip = vn_to_inode(cvp);
d_instantiate(dentry, ip);
- xfs_validate_fields(dir, &va);
- xfs_validate_fields(ip, &va);
+ xfs_validate_fields(dir);
+ xfs_validate_fields(ip);
} else {
xfs_cleanup_inode(dir, cvp, dentry, 0);
}
@@ -473,13 +472,12 @@ xfs_vn_rmdir(
struct dentry *dentry)
{
struct inode *inode = dentry->d_inode;
- bhv_vattr_t vattr;
int error;
error = xfs_rmdir(XFS_I(dir), dentry);
if (likely(!error)) {
- xfs_validate_fields(inode, &vattr);
- xfs_validate_fields(dir, &vattr);
+ xfs_validate_fields(inode);
+ xfs_validate_fields(dir);
}
return -error;
}
@@ -493,7 +491,6 @@ xfs_vn_rename(
{
struct inode *new_inode = ndentry->d_inode;
bhv_vnode_t *tvp; /* target directory */
- bhv_vattr_t vattr;
int error;
tvp = vn_from_inode(ndir);
@@ -501,10 +498,10 @@ xfs_vn_rename(
error = xfs_rename(XFS_I(odir), odentry, tvp, ndentry);
if (likely(!error)) {
if (new_inode)
- xfs_validate_fields(new_inode, &vattr);
- xfs_validate_fields(odir, &vattr);
+ xfs_validate_fields(new_inode);
+ xfs_validate_fields(odir);
if (ndir != odir)
- xfs_validate_fields(ndir, &vattr);
+ xfs_validate_fields(ndir);
}
return -error;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/4] simplify validata_fields
2007-09-14 16:27 [PATCH 1/4] simplify validata_fields Christoph Hellwig
@ 2007-09-18 3:56 ` Lachlan McIlroy
2007-09-18 3:56 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Lachlan McIlroy @ 2007-09-18 3:56 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
This looks good, just one comment...
Christoph Hellwig wrote:
> Stop using xfs_getattr and a onstack bhv_vattr_t just to get three
> fields from the underlying inode and opencode copying from the inode
> fields instead.
>
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_iops.c 2007-09-06 10:13:41.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c 2007-09-06 10:18:06.000000000 +0200
> @@ -179,18 +179,19 @@ xfs_ichgtime_fast(
> */
> STATIC void
> xfs_validate_fields(
> - struct inode *ip,
> - bhv_vattr_t *vattr)
> + struct inode *inode)
> {
> - vattr->va_mask = XFS_AT_NLINK|XFS_AT_SIZE|XFS_AT_NBLOCKS;
> - if (!xfs_getattr(XFS_I(ip), vattr, ATTR_LAZY)) {
> - ip->i_nlink = vattr->va_nlink;
> - ip->i_blocks = vattr->va_nblocks;
> -
> - /* we're under i_sem so i_size can't change under us */
> - if (i_size_read(ip) != vattr->va_size)
> - i_size_write(ip, vattr->va_size);
> - }
> + struct xfs_inode *ip = XFS_I(inode);
> + loff_t size;
> +
> + inode->i_nlink = ip->i_d.di_nlink;
> + inode->i_blocks =
> + XFS_FSB_TO_BB(ip->i_mount, ip->i_d.di_nblocks +
> + ip->i_delayed_blks);
> + /* we're under i_sem so i_size can't change under us */
> + size = XFS_ISIZE(ip);
> + if (i_size_read(inode) != size)
> + i_size_write(inode, size);
Can we drop the i_size_read() check and just call i_size_write()?
The work we may not do by not calling i_size_write() we have to do
in i_size_read() anyway, sometimes doing the work twice. And we
get to lose another stack variable.
> }
>
> /*
> @@ -340,9 +341,9 @@ xfs_vn_mknod(
> if (S_ISCHR(mode) || S_ISBLK(mode))
> ip->i_rdev = rdev;
> else if (S_ISDIR(mode))
> - xfs_validate_fields(ip, &vattr);
> + xfs_validate_fields(ip);
> d_instantiate(dentry, ip);
> - xfs_validate_fields(dir, &vattr);
> + xfs_validate_fields(dir);
> }
> return -error;
> }
> @@ -397,7 +398,6 @@ xfs_vn_link(
> {
> struct inode *ip; /* inode of guy being linked to */
> bhv_vnode_t *vp; /* vp of name being linked */
> - bhv_vattr_t vattr;
> int error;
>
> ip = old_dentry->d_inode; /* inode being linked to */
> @@ -409,7 +409,7 @@ xfs_vn_link(
> VN_RELE(vp);
> } else {
> xfs_iflags_set(XFS_I(dir), XFS_IMODIFIED);
> - xfs_validate_fields(ip, &vattr);
> + xfs_validate_fields(ip);
> d_instantiate(dentry, ip);
> }
> return -error;
> @@ -421,15 +421,14 @@ xfs_vn_unlink(
> struct dentry *dentry)
> {
> struct inode *inode;
> - bhv_vattr_t vattr;
> int error;
>
> inode = dentry->d_inode;
>
> error = xfs_remove(XFS_I(dir), dentry);
> if (likely(!error)) {
> - xfs_validate_fields(dir, &vattr); /* size needs update */
> - xfs_validate_fields(inode, &vattr);
> + xfs_validate_fields(dir); /* size needs update */
> + xfs_validate_fields(inode);
> }
> return -error;
> }
> @@ -458,8 +457,8 @@ xfs_vn_symlink(
> if (likely(!error)) {
> ip = vn_to_inode(cvp);
> d_instantiate(dentry, ip);
> - xfs_validate_fields(dir, &va);
> - xfs_validate_fields(ip, &va);
> + xfs_validate_fields(dir);
> + xfs_validate_fields(ip);
> } else {
> xfs_cleanup_inode(dir, cvp, dentry, 0);
> }
> @@ -473,13 +472,12 @@ xfs_vn_rmdir(
> struct dentry *dentry)
> {
> struct inode *inode = dentry->d_inode;
> - bhv_vattr_t vattr;
> int error;
>
> error = xfs_rmdir(XFS_I(dir), dentry);
> if (likely(!error)) {
> - xfs_validate_fields(inode, &vattr);
> - xfs_validate_fields(dir, &vattr);
> + xfs_validate_fields(inode);
> + xfs_validate_fields(dir);
> }
> return -error;
> }
> @@ -493,7 +491,6 @@ xfs_vn_rename(
> {
> struct inode *new_inode = ndentry->d_inode;
> bhv_vnode_t *tvp; /* target directory */
> - bhv_vattr_t vattr;
> int error;
>
> tvp = vn_from_inode(ndir);
> @@ -501,10 +498,10 @@ xfs_vn_rename(
> error = xfs_rename(XFS_I(odir), odentry, tvp, ndentry);
> if (likely(!error)) {
> if (new_inode)
> - xfs_validate_fields(new_inode, &vattr);
> - xfs_validate_fields(odir, &vattr);
> + xfs_validate_fields(new_inode);
> + xfs_validate_fields(odir);
> if (ndir != odir)
> - xfs_validate_fields(ndir, &vattr);
> + xfs_validate_fields(ndir);
> }
> return -error;
> }
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/4] simplify validata_fields
2007-09-18 3:56 ` Lachlan McIlroy
@ 2007-09-18 3:56 ` Christoph Hellwig
2007-09-18 5:09 ` Lachlan McIlroy
0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2007-09-18 3:56 UTC (permalink / raw)
To: Lachlan McIlroy; +Cc: Christoph Hellwig, xfs
On Tue, Sep 18, 2007 at 01:56:01PM +1000, Lachlan McIlroy wrote:
> Can we drop the i_size_read() check and just call i_size_write()?
> The work we may not do by not calling i_size_write() we have to do
> in i_size_read() anyway, sometimes doing the work twice. And we
> get to lose another stack variable.
Sounds good to me, but I'd prefer to keep it in a separate patch
so this is just the mechanical removal of the xfs_getattr call.
I'll send a separate patch for it today or tomorrow.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/4] simplify validata_fields
2007-09-18 3:56 ` Christoph Hellwig
@ 2007-09-18 5:09 ` Lachlan McIlroy
0 siblings, 0 replies; 4+ messages in thread
From: Lachlan McIlroy @ 2007-09-18 5:09 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
Christoph Hellwig wrote:
> On Tue, Sep 18, 2007 at 01:56:01PM +1000, Lachlan McIlroy wrote:
>> Can we drop the i_size_read() check and just call i_size_write()?
>> The work we may not do by not calling i_size_write() we have to do
>> in i_size_read() anyway, sometimes doing the work twice. And we
>> get to lose another stack variable.
>
> Sounds good to me, but I'd prefer to keep it in a separate patch
> so this is just the mechanical removal of the xfs_getattr call.
>
> I'll send a separate patch for it today or tomorrow.
Okay, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2007-09-18 5:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-09-14 16:27 [PATCH 1/4] simplify validata_fields Christoph Hellwig
2007-09-18 3:56 ` Lachlan McIlroy
2007-09-18 3:56 ` Christoph Hellwig
2007-09-18 5:09 ` Lachlan McIlroy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox