From: Christoph Hellwig <hch@lst.de>
To: xfs@oss.sgi.com
Subject: Re: [PATCH] sanitize xfs_initialize_vnode
Date: Wed, 23 Jul 2008 10:06:06 +0200 [thread overview]
Message-ID: <20080723080606.GD3417@lst.de> (raw)
In-Reply-To: <20080627130546.GA23431@lst.de>
ping^3
On Fri, Jun 27, 2008 at 03:05:46PM +0200, Christoph Hellwig wrote:
> ping^2
>
> On Tue, May 20, 2008 at 08:36:22AM +0200, Christoph Hellwig wrote:
> > ping?
> >
> > On Fri, May 02, 2008 at 12:52:15PM +0200, Christoph Hellwig wrote:
> > > Sanitize setting up the Linux indode.
> > >
> > > Setting up the xfs_inode <-> inode link is opencoded in xfs_iget_core
> > > now because that's the only place it needs to be done,
> > > xfs_initialize_vnode is renamed to xfs_setup_inode and loses all
> > > superflous paramaters. The check for I_NEW is removed because it always
> > > is true and the di_mode check moves into xfs_iget_core because it's only
> > > needed there.
> > >
> > > xfs_set_inodeops and xfs_revalidate_inode are merged into
> > > xfs_setup_inode and the whole things is moved into xfs_iops.c where it
> > > belongs.
> > >
> > >
> > > 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 2008-05-02 08:41:27.000000000 +0200
> > > +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c 2008-05-02 08:51:46.000000000 +0200
> > > @@ -777,7 +777,7 @@ out_error:
> > > return error;
> > > }
> > >
> > > -const struct inode_operations xfs_inode_operations = {
> > > +static const struct inode_operations xfs_inode_operations = {
> > > .permission = xfs_vn_permission,
> > > .truncate = xfs_vn_truncate,
> > > .getattr = xfs_vn_getattr,
> > > @@ -789,7 +789,7 @@ const struct inode_operations xfs_inode_
> > > .fallocate = xfs_vn_fallocate,
> > > };
> > >
> > > -const struct inode_operations xfs_dir_inode_operations = {
> > > +static const struct inode_operations xfs_dir_inode_operations = {
> > > .create = xfs_vn_create,
> > > .lookup = xfs_vn_lookup,
> > > .link = xfs_vn_link,
> > > @@ -808,7 +808,7 @@ const struct inode_operations xfs_dir_in
> > > .listxattr = xfs_vn_listxattr,
> > > };
> > >
> > > -const struct inode_operations xfs_symlink_inode_operations = {
> > > +static const struct inode_operations xfs_symlink_inode_operations = {
> > > .readlink = generic_readlink,
> > > .follow_link = xfs_vn_follow_link,
> > > .put_link = xfs_vn_put_link,
> > > @@ -820,3 +820,95 @@ const struct inode_operations xfs_symlin
> > > .removexattr = generic_removexattr,
> > > .listxattr = xfs_vn_listxattr,
> > > };
> > > +
> > > +STATIC void
> > > +xfs_diflags_to_iflags(
> > > + struct inode *inode,
> > > + struct xfs_inode *ip)
> > > +{
> > > + if (ip->i_d.di_flags & XFS_DIFLAG_IMMUTABLE)
> > > + inode->i_flags |= S_IMMUTABLE;
> > > + else
> > > + inode->i_flags &= ~S_IMMUTABLE;
> > > + if (ip->i_d.di_flags & XFS_DIFLAG_APPEND)
> > > + inode->i_flags |= S_APPEND;
> > > + else
> > > + inode->i_flags &= ~S_APPEND;
> > > + if (ip->i_d.di_flags & XFS_DIFLAG_SYNC)
> > > + inode->i_flags |= S_SYNC;
> > > + else
> > > + inode->i_flags &= ~S_SYNC;
> > > + if (ip->i_d.di_flags & XFS_DIFLAG_NOATIME)
> > > + inode->i_flags |= S_NOATIME;
> > > + else
> > > + inode->i_flags &= ~S_NOATIME;
> > > +}
> > > +
> > > +/*
> > > + * Initialize the Linux inode, set up the operation vectors and
> > > + * unlock the inode.
> > > + *
> > > + * When reading existing inodes from disk this is called directly
> > > + * from xfs_iget, when creating a new inode it is called from
> > > + * xfs_ialloc after setting up the inode.
> > > + */
> > > +void
> > > +xfs_setup_inode(
> > > + struct xfs_inode *ip)
> > > +{
> > > + struct inode *inode = ip->i_vnode;
> > > +
> > > + inode->i_mode = ip->i_d.di_mode;
> > > + inode->i_nlink = ip->i_d.di_nlink;
> > > + inode->i_uid = ip->i_d.di_uid;
> > > + inode->i_gid = ip->i_d.di_gid;
> > > +
> > > + switch (inode->i_mode & S_IFMT) {
> > > + case S_IFBLK:
> > > + case S_IFCHR:
> > > + inode->i_rdev =
> > > + MKDEV(sysv_major(ip->i_df.if_u2.if_rdev) & 0x1ff,
> > > + sysv_minor(ip->i_df.if_u2.if_rdev));
> > > + break;
> > > + default:
> > > + inode->i_rdev = 0;
> > > + break;
> > > + }
> > > +
> > > + inode->i_generation = ip->i_d.di_gen;
> > > + i_size_write(inode, ip->i_d.di_size);
> > > + inode->i_atime.tv_sec = ip->i_d.di_atime.t_sec;
> > > + inode->i_atime.tv_nsec = ip->i_d.di_atime.t_nsec;
> > > + inode->i_mtime.tv_sec = ip->i_d.di_mtime.t_sec;
> > > + inode->i_mtime.tv_nsec = ip->i_d.di_mtime.t_nsec;
> > > + inode->i_ctime.tv_sec = ip->i_d.di_ctime.t_sec;
> > > + inode->i_ctime.tv_nsec = ip->i_d.di_ctime.t_nsec;
> > > + xfs_diflags_to_iflags(inode, ip);
> > > + xfs_iflags_clear(ip, XFS_IMODIFIED);
> > > +
> > > + switch (inode->i_mode & S_IFMT) {
> > > + case S_IFREG:
> > > + inode->i_op = &xfs_inode_operations;
> > > + inode->i_fop = &xfs_file_operations;
> > > + inode->i_mapping->a_ops = &xfs_address_space_operations;
> > > + break;
> > > + case S_IFDIR:
> > > + inode->i_op = &xfs_dir_inode_operations;
> > > + inode->i_fop = &xfs_dir_file_operations;
> > > + break;
> > > + case S_IFLNK:
> > > + inode->i_op = &xfs_symlink_inode_operations;
> > > + if (!(ip->i_df.if_flags & XFS_IFINLINE))
> > > + inode->i_mapping->a_ops = &xfs_address_space_operations;
> > > + break;
> > > + default:
> > > + inode->i_op = &xfs_inode_operations;
> > > + init_special_inode(inode, inode->i_mode, inode->i_rdev);
> > > + break;
> > > + }
> > > +
> > > + xfs_iflags_clear(ip, XFS_INEW);
> > > + barrier();
> > > +
> > > + unlock_new_inode(inode);
> > > +}
> > > Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.h
> > > ===================================================================
> > > --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_iops.h 2008-05-02 08:41:27.000000000 +0200
> > > +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.h 2008-05-02 08:41:35.000000000 +0200
> > > @@ -18,9 +18,7 @@
> > > #ifndef __XFS_IOPS_H__
> > > #define __XFS_IOPS_H__
> > >
> > > -extern const struct inode_operations xfs_inode_operations;
> > > -extern const struct inode_operations xfs_dir_inode_operations;
> > > -extern const struct inode_operations xfs_symlink_inode_operations;
> > > +struct xfs_inode;
> > >
> > > extern const struct file_operations xfs_file_operations;
> > > extern const struct file_operations xfs_dir_file_operations;
> > > @@ -28,10 +26,11 @@ extern const struct file_operations xfs_
> > >
> > > extern ssize_t xfs_vn_listxattr(struct dentry *, char *data, size_t size);
> > >
> > > -struct xfs_inode;
> > > extern void xfs_ichgtime(struct xfs_inode *, int);
> > > extern void xfs_ichgtime_fast(struct xfs_inode *, struct inode *, int);
> > >
> > > +extern void xfs_setup_inode(struct xfs_inode *);
> > > +
> > > #define xfs_vtoi(vp) \
> > > ((struct xfs_inode *)vn_to_inode(vp)->i_private)
> > >
> > > Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ksyms.c
> > > ===================================================================
> > > --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_ksyms.c 2008-05-02 08:41:27.000000000 +0200
> > > +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ksyms.c 2008-05-02 08:41:35.000000000 +0200
> > > @@ -155,12 +155,9 @@ EXPORT_SYMBOL(kmem_zone_free);
> > > EXPORT_SYMBOL(kmem_zone_init);
> > > EXPORT_SYMBOL(kmem_zone_zalloc);
> > > EXPORT_SYMBOL(xfs_address_space_operations);
> > > -EXPORT_SYMBOL(xfs_dir_inode_operations);
> > > EXPORT_SYMBOL(xfs_dir_file_operations);
> > > -EXPORT_SYMBOL(xfs_inode_operations);
> > > EXPORT_SYMBOL(xfs_file_operations);
> > > EXPORT_SYMBOL(xfs_invis_file_operations);
> > > -EXPORT_SYMBOL(xfs_symlink_inode_operations);
> > > EXPORT_SYMBOL(xfs_buf_delwri_dequeue);
> > > EXPORT_SYMBOL(_xfs_buf_find);
> > > EXPORT_SYMBOL(xfs_buf_iostart);
> > > Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_super.c
> > > ===================================================================
> > > --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_super.c 2008-05-02 08:41:34.000000000 +0200
> > > +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_super.c 2008-05-02 08:51:43.000000000 +0200
> > > @@ -558,115 +558,6 @@ xfs_max_file_offset(
> > > return (((__uint64_t)pagefactor) << bitshift) - 1;
> > > }
> > >
> > > -STATIC_INLINE void
> > > -xfs_set_inodeops(
> > > - struct inode *inode)
> > > -{
> > > - switch (inode->i_mode & S_IFMT) {
> > > - case S_IFREG:
> > > - inode->i_op = &xfs_inode_operations;
> > > - inode->i_fop = &xfs_file_operations;
> > > - inode->i_mapping->a_ops = &xfs_address_space_operations;
> > > - break;
> > > - case S_IFDIR:
> > > - inode->i_op = &xfs_dir_inode_operations;
> > > - inode->i_fop = &xfs_dir_file_operations;
> > > - break;
> > > - case S_IFLNK:
> > > - inode->i_op = &xfs_symlink_inode_operations;
> > > - if (!(XFS_I(inode)->i_df.if_flags & XFS_IFINLINE))
> > > - inode->i_mapping->a_ops = &xfs_address_space_operations;
> > > - break;
> > > - default:
> > > - inode->i_op = &xfs_inode_operations;
> > > - init_special_inode(inode, inode->i_mode, inode->i_rdev);
> > > - break;
> > > - }
> > > -}
> > > -
> > > -STATIC_INLINE void
> > > -xfs_revalidate_inode(
> > > - xfs_mount_t *mp,
> > > - bhv_vnode_t *vp,
> > > - xfs_inode_t *ip)
> > > -{
> > > - struct inode *inode = vn_to_inode(vp);
> > > -
> > > - inode->i_mode = ip->i_d.di_mode;
> > > - inode->i_nlink = ip->i_d.di_nlink;
> > > - inode->i_uid = ip->i_d.di_uid;
> > > - inode->i_gid = ip->i_d.di_gid;
> > > -
> > > - switch (inode->i_mode & S_IFMT) {
> > > - case S_IFBLK:
> > > - case S_IFCHR:
> > > - inode->i_rdev =
> > > - MKDEV(sysv_major(ip->i_df.if_u2.if_rdev) & 0x1ff,
> > > - sysv_minor(ip->i_df.if_u2.if_rdev));
> > > - break;
> > > - default:
> > > - inode->i_rdev = 0;
> > > - break;
> > > - }
> > > -
> > > - inode->i_generation = ip->i_d.di_gen;
> > > - i_size_write(inode, ip->i_d.di_size);
> > > - inode->i_atime.tv_sec = ip->i_d.di_atime.t_sec;
> > > - inode->i_atime.tv_nsec = ip->i_d.di_atime.t_nsec;
> > > - inode->i_mtime.tv_sec = ip->i_d.di_mtime.t_sec;
> > > - inode->i_mtime.tv_nsec = ip->i_d.di_mtime.t_nsec;
> > > - inode->i_ctime.tv_sec = ip->i_d.di_ctime.t_sec;
> > > - inode->i_ctime.tv_nsec = ip->i_d.di_ctime.t_nsec;
> > > - if (ip->i_d.di_flags & XFS_DIFLAG_IMMUTABLE)
> > > - inode->i_flags |= S_IMMUTABLE;
> > > - else
> > > - inode->i_flags &= ~S_IMMUTABLE;
> > > - if (ip->i_d.di_flags & XFS_DIFLAG_APPEND)
> > > - inode->i_flags |= S_APPEND;
> > > - else
> > > - inode->i_flags &= ~S_APPEND;
> > > - if (ip->i_d.di_flags & XFS_DIFLAG_SYNC)
> > > - inode->i_flags |= S_SYNC;
> > > - else
> > > - inode->i_flags &= ~S_SYNC;
> > > - if (ip->i_d.di_flags & XFS_DIFLAG_NOATIME)
> > > - inode->i_flags |= S_NOATIME;
> > > - else
> > > - inode->i_flags &= ~S_NOATIME;
> > > - xfs_iflags_clear(ip, XFS_IMODIFIED);
> > > -}
> > > -
> > > -void
> > > -xfs_initialize_vnode(
> > > - struct xfs_mount *mp,
> > > - bhv_vnode_t *vp,
> > > - struct xfs_inode *ip)
> > > -{
> > > - struct inode *inode = vn_to_inode(vp);
> > > -
> > > - if (!ip->i_vnode) {
> > > - ip->i_vnode = vp;
> > > - inode->i_private = ip;
> > > - }
> > > -
> > > - /*
> > > - * We need to set the ops vectors, and unlock the inode, but if
> > > - * we have been called during the new inode create process, it is
> > > - * too early to fill in the Linux inode. We will get called a
> > > - * second time once the inode is properly set up, and then we can
> > > - * finish our work.
> > > - */
> > > - if (ip->i_d.di_mode != 0 && (inode->i_state & I_NEW)) {
> > > - xfs_revalidate_inode(mp, vp, ip);
> > > - xfs_set_inodeops(inode);
> > > -
> > > - xfs_iflags_clear(ip, XFS_INEW);
> > > - barrier();
> > > -
> > > - unlock_new_inode(inode);
> > > - }
> > > -}
> > > -
> > > int
> > > xfs_blkdev_get(
> > > xfs_mount_t *mp,
> > > Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_super.h
> > > ===================================================================
> > > --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_super.h 2008-05-02 08:41:27.000000000 +0200
> > > +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_super.h 2008-05-02 08:41:35.000000000 +0200
> > > @@ -72,9 +72,6 @@ struct block_device;
> > >
> > > extern __uint64_t xfs_max_file_offset(unsigned int);
> > >
> > > -extern void xfs_initialize_vnode(struct xfs_mount *mp, bhv_vnode_t *vp,
> > > - struct xfs_inode *ip);
> > > -
> > > extern void xfs_flush_inode(struct xfs_inode *);
> > > extern void xfs_flush_device(struct xfs_inode *);
> > >
> > > Index: linux-2.6-xfs/fs/xfs/xfs_iget.c
> > > ===================================================================
> > > --- linux-2.6-xfs.orig/fs/xfs/xfs_iget.c 2008-05-02 08:41:27.000000000 +0200
> > > +++ linux-2.6-xfs/fs/xfs/xfs_iget.c 2008-05-02 08:53:18.000000000 +0200
> > > @@ -288,10 +288,17 @@ finish_inode:
> > > *ipp = ip;
> > >
> > > /*
> > > + * Set up the Linux with the Linux inode.
> > > + */
> > > + ip->i_vnode = inode;
> > > + inode->i_private = ip;
> > > +
> > > + /*
> > > * If we have a real type for an on-disk inode, we can set ops(&unlock)
> > > * now. If it's a new inode being created, xfs_ialloc will handle it.
> > > */
> > > - xfs_initialize_vnode(mp, inode, ip);
> > > + if (ip->i_d.di_mode != 0)
> > > + xfs_setup_inode(ip);
> > > return 0;
> > > }
> > >
> > > Index: linux-2.6-xfs/fs/xfs/xfs_inode.c
> > > ===================================================================
> > > --- linux-2.6-xfs.orig/fs/xfs/xfs_inode.c 2008-05-02 08:41:27.000000000 +0200
> > > +++ linux-2.6-xfs/fs/xfs/xfs_inode.c 2008-05-02 08:41:35.000000000 +0200
> > > @@ -1046,7 +1046,6 @@ xfs_ialloc(
> > > {
> > > xfs_ino_t ino;
> > > xfs_inode_t *ip;
> > > - bhv_vnode_t *vp;
> > > uint flags;
> > > int error;
> > >
> > > @@ -1077,7 +1076,6 @@ xfs_ialloc(
> > > }
> > > ASSERT(ip != NULL);
> > >
> > > - vp = XFS_ITOV(ip);
> > > ip->i_d.di_mode = (__uint16_t)mode;
> > > ip->i_d.di_onlink = 0;
> > > ip->i_d.di_nlink = nlink;
> > > @@ -1220,7 +1218,7 @@ xfs_ialloc(
> > > xfs_trans_log_inode(tp, ip, flags);
> > >
> > > /* now that we have an i_mode we can setup inode ops and unlock */
> > > - xfs_initialize_vnode(tp->t_mountp, vp, ip);
> > > + xfs_setup_inode(ip);
> > >
> > > *ipp = ip;
> > > return 0;
> > ---end quoted text---
> ---end quoted text---
---end quoted text---
next prev parent reply other threads:[~2008-07-23 8:05 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-05-02 10:52 [PATCH] sanitize xfs_initialize_vnode Christoph Hellwig
2008-05-20 6:36 ` Christoph Hellwig
2008-06-27 13:05 ` Christoph Hellwig
2008-07-23 8:06 ` Christoph Hellwig [this message]
2008-05-21 8:21 ` Christoph Hellwig
2008-07-23 19:51 ` Christoph Hellwig
2008-07-24 6:16 ` Dave Chinner
2008-07-24 6:20 ` Christoph Hellwig
2008-07-28 22:57 ` Christoph Hellwig
2008-07-29 1:02 ` Niv Sardi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080723080606.GD3417@lst.de \
--to=hch@lst.de \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.