* [PATCH 3/4] simplify xfs_vn_getattr
@ 2007-09-14 16:27 Christoph Hellwig
2007-09-19 2:55 ` Lachlan McIlroy
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2007-09-14 16:27 UTC (permalink / raw)
To: xfs
Just fill in struct kstat directly from the xfs_inode instead of doing
a detour through a bhv_vattr_t and xfs_getattr.
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:42:40.000000000 +0200
+++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c 2007-09-06 10:44:33.000000000 +0200
@@ -564,33 +564,61 @@ xfs_vn_permission(
STATIC int
xfs_vn_getattr(
- struct vfsmount *mnt,
- struct dentry *dentry,
- struct kstat *stat)
+ struct vfsmount *mnt,
+ struct dentry *dentry,
+ struct kstat *stat)
{
- struct inode *inode = dentry->d_inode;
- bhv_vattr_t vattr = { .va_mask = XFS_AT_STAT };
- int error;
-
- error = xfs_getattr(XFS_I(inode), &vattr, ATTR_LAZY);
- if (likely(!error)) {
- stat->size = i_size_read(inode);
- stat->dev = inode->i_sb->s_dev;
- stat->rdev = (vattr.va_rdev == 0) ? 0 :
- MKDEV(sysv_major(vattr.va_rdev) & 0x1ff,
- sysv_minor(vattr.va_rdev));
- stat->mode = vattr.va_mode;
- stat->nlink = vattr.va_nlink;
- stat->uid = vattr.va_uid;
- stat->gid = vattr.va_gid;
- stat->ino = vattr.va_nodeid;
- stat->atime = vattr.va_atime;
- stat->mtime = vattr.va_mtime;
- stat->ctime = vattr.va_ctime;
- stat->blocks = vattr.va_nblocks;
- stat->blksize = vattr.va_blocksize;
+ struct inode *inode = dentry->d_inode;
+ struct xfs_inode *ip = XFS_I(inode);
+ struct xfs_mount *mp = ip->i_mount;
+
+ xfs_itrace_entry(ip);
+
+ if (XFS_FORCED_SHUTDOWN(mp))
+ return XFS_ERROR(EIO);
+
+ stat->size = XFS_ISIZE(ip);
+ stat->dev = inode->i_sb->s_dev;
+ stat->mode = ip->i_d.di_mode;
+ stat->nlink = ip->i_d.di_nlink;
+ stat->uid = ip->i_d.di_uid;
+ stat->gid = ip->i_d.di_gid;
+ stat->ino = ip->i_ino;
+#if XFS_BIG_INUMS
+ stat->ino += mp->m_inoadd;
+#endif
+ stat->atime = inode->i_atime;
+ stat->mtime.tv_sec = ip->i_d.di_mtime.t_sec;
+ stat->mtime.tv_nsec = ip->i_d.di_mtime.t_nsec;
+ stat->ctime.tv_sec = ip->i_d.di_ctime.t_sec;
+ stat->ctime.tv_nsec = ip->i_d.di_ctime.t_nsec;
+ stat->blocks =
+ XFS_FSB_TO_BB(mp, ip->i_d.di_nblocks + ip->i_delayed_blks);
+
+
+ switch (inode->i_mode & S_IFMT) {
+ case S_IFBLK:
+ case S_IFCHR:
+ stat->blksize = BLKDEV_IOSIZE;
+ stat->rdev = MKDEV(sysv_major(ip->i_df.if_u2.if_rdev) & 0x1ff,
+ sysv_minor(ip->i_df.if_u2.if_rdev));
+ break;
+ default:
+ if (ip->i_d.di_flags & XFS_DIFLAG_REALTIME) {
+ /*
+ * If the file blocks are being allocated from a
+ * realtime volume, then return the inode's realtime
+ * extent size or the realtime volume's extent size.
+ */
+ stat->blksize =
+ xfs_get_extsz_hint(ip) << mp->m_sb.sb_blocklog;
+ } else
+ stat->blksize = xfs_preferred_iosize(mp);
+ stat->rdev = 0;
+ break;
}
- return -error;
+
+ return 0;
}
STATIC int
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 3/4] simplify xfs_vn_getattr
2007-09-14 16:27 [PATCH 3/4] simplify xfs_vn_getattr Christoph Hellwig
@ 2007-09-19 2:55 ` Lachlan McIlroy
2007-09-19 11:04 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Lachlan McIlroy @ 2007-09-19 2:55 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
Just one question below - patch looks fine.
Christoph Hellwig wrote:
> Just fill in struct kstat directly from the xfs_inode instead of doing
> a detour through a bhv_vattr_t and xfs_getattr.
>
>
> 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:42:40.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c 2007-09-06 10:44:33.000000000 +0200
> @@ -564,33 +564,61 @@ xfs_vn_permission(
>
> STATIC int
> xfs_vn_getattr(
> - struct vfsmount *mnt,
> - struct dentry *dentry,
> - struct kstat *stat)
> + struct vfsmount *mnt,
> + struct dentry *dentry,
> + struct kstat *stat)
> {
> - struct inode *inode = dentry->d_inode;
> - bhv_vattr_t vattr = { .va_mask = XFS_AT_STAT };
> - int error;
> -
> - error = xfs_getattr(XFS_I(inode), &vattr, ATTR_LAZY);
> - if (likely(!error)) {
> - stat->size = i_size_read(inode);
> - stat->dev = inode->i_sb->s_dev;
> - stat->rdev = (vattr.va_rdev == 0) ? 0 :
> - MKDEV(sysv_major(vattr.va_rdev) & 0x1ff,
> - sysv_minor(vattr.va_rdev));
> - stat->mode = vattr.va_mode;
> - stat->nlink = vattr.va_nlink;
> - stat->uid = vattr.va_uid;
> - stat->gid = vattr.va_gid;
> - stat->ino = vattr.va_nodeid;
> - stat->atime = vattr.va_atime;
> - stat->mtime = vattr.va_mtime;
> - stat->ctime = vattr.va_ctime;
> - stat->blocks = vattr.va_nblocks;
> - stat->blksize = vattr.va_blocksize;
> + struct inode *inode = dentry->d_inode;
> + struct xfs_inode *ip = XFS_I(inode);
> + struct xfs_mount *mp = ip->i_mount;
> +
> + xfs_itrace_entry(ip);
> +
> + if (XFS_FORCED_SHUTDOWN(mp))
> + return XFS_ERROR(EIO);
> +
> + stat->size = XFS_ISIZE(ip);
> + stat->dev = inode->i_sb->s_dev;
> + stat->mode = ip->i_d.di_mode;
> + stat->nlink = ip->i_d.di_nlink;
> + stat->uid = ip->i_d.di_uid;
> + stat->gid = ip->i_d.di_gid;
> + stat->ino = ip->i_ino;
> +#if XFS_BIG_INUMS
> + stat->ino += mp->m_inoadd;
> +#endif
> + stat->atime = inode->i_atime;
> + stat->mtime.tv_sec = ip->i_d.di_mtime.t_sec;
> + stat->mtime.tv_nsec = ip->i_d.di_mtime.t_nsec;
> + stat->ctime.tv_sec = ip->i_d.di_ctime.t_sec;
> + stat->ctime.tv_nsec = ip->i_d.di_ctime.t_nsec;
Why do we copy the atime from the inode and not the xfs_inode?
I think I see what's the deal here - the atime in the inode is
the authoritative atime. It gets updated from various places and
we synchronise it to the xfs inode before flushing the xfs inode
to disk. This means we shouldn't be using the atime in the
xfs_inode because it will be stale - is this correct?
> + stat->blocks =
> + XFS_FSB_TO_BB(mp, ip->i_d.di_nblocks + ip->i_delayed_blks);
> +
> +
> + switch (inode->i_mode & S_IFMT) {
> + case S_IFBLK:
> + case S_IFCHR:
> + stat->blksize = BLKDEV_IOSIZE;
> + stat->rdev = MKDEV(sysv_major(ip->i_df.if_u2.if_rdev) & 0x1ff,
> + sysv_minor(ip->i_df.if_u2.if_rdev));
> + break;
> + default:
> + if (ip->i_d.di_flags & XFS_DIFLAG_REALTIME) {
> + /*
> + * If the file blocks are being allocated from a
> + * realtime volume, then return the inode's realtime
> + * extent size or the realtime volume's extent size.
> + */
> + stat->blksize =
> + xfs_get_extsz_hint(ip) << mp->m_sb.sb_blocklog;
> + } else
> + stat->blksize = xfs_preferred_iosize(mp);
> + stat->rdev = 0;
> + break;
> }
> - return -error;
> +
> + return 0;
> }
>
> STATIC int
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 3/4] simplify xfs_vn_getattr
2007-09-19 2:55 ` Lachlan McIlroy
@ 2007-09-19 11:04 ` Christoph Hellwig
0 siblings, 0 replies; 3+ messages in thread
From: Christoph Hellwig @ 2007-09-19 11:04 UTC (permalink / raw)
To: Lachlan McIlroy; +Cc: Christoph Hellwig, xfs
On Wed, Sep 19, 2007 at 12:55:20PM +1000, Lachlan McIlroy wrote:
> Why do we copy the atime from the inode and not the xfs_inode?
>
> I think I see what's the deal here - the atime in the inode is
> the authoritative atime. It gets updated from various places and
> we synchronise it to the xfs inode before flushing the xfs inode
> to disk. This means we shouldn't be using the atime in the
> xfs_inode because it will be stale - is this correct?
Yes. atime is updated by the vfs without callouts to the filesystem
in various places unfortunately.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-09-19 15:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-09-14 16:27 [PATCH 3/4] simplify xfs_vn_getattr Christoph Hellwig
2007-09-19 2:55 ` Lachlan McIlroy
2007-09-19 11:04 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox