Messages in this thread | | | Date | Wed, 15 Dec 2010 10:15:42 +0200 | Subject | Re: [PATCH] cramfs: generate unique inode number for better inode cache usage | From | Pekka Enberg <> |
| |
On Wed, Dec 15, 2010 at 9:50 AM, Stefani Seibold <stefani@seibold.net> wrote: > No, it makes no sense, for following reasons: > > - Why pre-setup variables like iops, fops and aops which are in the most > cases not needed? Because the inode is still in the cache. > > - Each branch is a little bit different. > > - The default branch is completly different, because in this case the > init_special_inode() will be called, which sets the inode->fop. > > What do you think about this solution: > > static void setup_inode(struct inode *inode, struct cramfs_inode * cramfs_inode) > { > static struct timespec zerotime; > inode->i_mode = cramfs_inode->mode; > inode->i_uid = cramfs_inode->uid; > /* if the lower 2 bits are zero, the inode contains data */ > if (!(inode->i_ino & 3)) { > inode->i_size = size; > inode->i_blocks = (size - 1) / 512 + 1; > } > inode->i_gid = cramfs_inode->gid; > /* Struct copy intentional */ > inode->i_mtime = inode->i_atime = inode->i_ctime = zerotime; > /* inode->i_nlink is left 1 - arguably wrong for directories, > but it's the best we can do without reading the directory > contents. 1 yields the right result in GNU find, even > without -noleaf option. */ > unlock_new_inode(inode); > } > > static struct inode *get_cramfs_inode(struct super_block *sb, > struct cramfs_inode * cramfs_inode, unsigned int offset) > { > struct inode *inode; > > switch (cram_inode->mode & S_IFMT) { > case S_IFREG: > inode = iget_locked(sb, cramino(cramfs_inode, offset)); > if (inode && (inode->i_state & I_NEW)) { > inode->i_fop = &generic_ro_fops; > inode->i_data.a_ops = &cramfs_aops; > setup_inode(inode, cramfs_inode); > } > break; > case S_IFDIR: > inode = iget_locked(sb, cramino(cramfs_inode, offset)); > if (inode && (inode->i_state & I_NEW)) { > inode->i_op = &cramfs_dir_inode_operations; > inode->i_fop = &cramfs_directory_operations; > setup_inode(inode, cramfs_inode); > } > break; > case S_IFLNK: > inode = iget_locked(sb, cramino(cramfs_inode, offset)); > if (inode && (inode->i_state & I_NEW)) { > inode->i_op = &page_symlink_inode_operations; > inode->i_data.a_ops = &cramfs_aops; > setup_inode(inode, cramfs_inode); > } > break; > default: > inode = iget_locked(sb, CRAMINO_UNIQ(offset)); > if (inode && (inode->i_state & I_NEW)) { > init_special_inode(inode, cramfs_inode->mode, > old_decode_dev(cramfs_inode->size)); > setup_inode(inode, cramfs_inode); > } > break; > } > return inode; > }
I think Linus is after something like this:
static struct inode *get_cramfs_inode(struct super_block *sb, struct cramfs_inode * cramfs_inode, unsigned int offset) { struct inode *inode; unsigned long ino;
switch (cram_inode->mode & S_IFMT) { case S_IFREG: case S_IFDIR: case S_IFLNK: ino = cramino(cramfs_inode, offset); break; default: ino = CRAMINO_UNIQ(offset); break; }
inode = iget_locked(sb, ino); if (!inode) return ERR_PTR(-ENOMEM); if (!(inode->i_state & I_NEW)) return inode;
switch (cram_inode->mode & S_IFMT) { case S_IFREG: inode->i_fop = &generic_ro_fops; inode->i_data.a_ops = &cramfs_aops; setup_inode(inode, cramfs_inode); break; case S_IFDIR: inode->i_op = &cramfs_dir_inode_operations; inode->i_fop = &cramfs_directory_operations; setup_inode(inode, cramfs_inode); break; case S_IFLNK: inode->i_op = &page_symlink_inode_operations; inode->i_data.a_ops = &cramfs_aops; setup_inode(inode, cramfs_inode); break; default: init_special_inode(inode, cramfs_inode->mode, old_decode_dev(cramfs_inode->size)); setup_inode(inode, cramfs_inode); break; } return inode; }
You should optimize for the in-cache case. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |