On Mon 11-08-14 15:25:15, Jeff Mahoney wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 8/6/14, 2:03 PM, Jan Kara wrote:
> > xfstest run for reiserfs produces lots of warnings like:
> >
> > WARNING: CPU: 4 PID: 24572 at fs/inode.c:937
> > unlock_new_inode+0x76/0x80()
> >
> > because reiserfs uses new_inode() to allocate new inodes and that
> > doesn't set I_NEW in i_state. This seems like it could cause subtle
> > bugs because half-initialized inodes may be visible in superblock
> > inode list or inode hashes. So make sure inode has I_NEW set before
> > it's visible anywhere which also gets rid of the warning.
> >
> > Signed-off-by: Jan Kara <jack@xxxxxxx>
>
> The problem is that reiserfs_new_inode drops down to the error
> handling case without having called insert_inode_locked4. I have a
> patch that just moves the quota check after the insertion so we don't
> need to do gross things like if (!inode_unhashed(inode))
> unlock_new_inode(inode) in the error path. It's consistent with how
> ext4 does it.
OK, thanks. I've removed this patch from my tree, asked Linus not to pull
the original request and will send him a new one after some testing. Please
send me your fix when you have it ready so that I can include it in pull
request for rc2/rc3. Thanks!
Honza
>
> - -Jeff
>
>
> > --- fs/reiserfs/namei.c | 34 ++++++++++++++++++---------------- 1
> > file changed, 18 insertions(+), 16 deletions(-)
> >
> > diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c index
> > cd11358b10c7..c4f435c4e6fc 100644 --- a/fs/reiserfs/namei.c +++
> > b/fs/reiserfs/namei.c @@ -580,8 +580,8 @@ static int
> > reiserfs_add_entry(struct reiserfs_transaction_handle *th, }
> >
> > /* - * quota utility function, call if you've had to abort after
> > calling - * new_inode_init, and have not called reiserfs_new_inode
> > yet. + * Utility function to call if you've had to abort after
> > calling + * get_new_inode, and have not called reiserfs_new_inode
> > yet. * This should only be called on inodes that do not have stat
> > data * inserted into the tree yet. */ @@ -590,6 +590,7 @@ static
> > int drop_new_inode(struct inode *inode) dquot_drop(inode);
> > make_bad_inode(inode); inode->i_flags |= S_NOQUOTA; +
> > unlock_new_inode(inode); iput(inode); return 0; } @@ -600,8 +601,16
> > @@ static int drop_new_inode(struct inode *inode) * outside of a
> > transaction, so we had to pull some bits of * reiserfs_new_inode
> > out into this func. */ -static int new_inode_init(struct inode
> > *inode, struct inode *dir, umode_t mode) +static struct inode
> > *get_new_inode(struct inode *dir, umode_t mode) { + struct inode
> > *inode = new_inode_pseudo(dir->i_sb); + + if (!inode) + return
> > NULL; + /* Make sure inode is invisible until it's fully set up */
> > + inode->i_state |= I_NEW; + inode_sb_list_add(inode); + /* * Make
> > inode invalid - just in case we are going to drop it before * the
> > initialization happens @@ -614,7 +623,8 @@ static int
> > new_inode_init(struct inode *inode, struct inode *dir, umode_t
> > mode) */ inode_init_owner(inode, dir, mode);
> > dquot_initialize(inode); - return 0; + + return inode; }
> >
> > static int reiserfs_create(struct inode *dir, struct dentry
> > *dentry, umode_t mode, @@ -635,10 +645,8 @@ static int
> > reiserfs_create(struct inode *dir, struct dentry *dentry, umode_t
> > mod
> >
> > dquot_initialize(dir);
> >
> > - if (!(inode = new_inode(dir->i_sb))) { + if (!(inode =
> > get_new_inode(dir, mode))) return -ENOMEM; - } -
> > new_inode_init(inode, dir, mode);
> >
> > jbegin_count += reiserfs_cache_default_acl(dir); retval =
> > reiserfs_security_init(dir, inode, &dentry->d_name, &security); @@
> > -712,10 +720,8 @@ static int reiserfs_mknod(struct inode *dir,
> > struct dentry *dentry, umode_t mode
> >
> > dquot_initialize(dir);
> >
> > - if (!(inode = new_inode(dir->i_sb))) { + if (!(inode =
> > get_new_inode(dir, mode))) return -ENOMEM; - } -
> > new_inode_init(inode, dir, mode);
> >
> > jbegin_count += reiserfs_cache_default_acl(dir); retval =
> > reiserfs_security_init(dir, inode, &dentry->d_name, &security); @@
> > -797,10 +803,8 @@ static int reiserfs_mkdir(struct inode *dir,
> > struct dentry *dentry, umode_t mode
> > REISERFS_I(dir)->new_packing_locality = 1; #endif mode = S_IFDIR |
> > mode; - if (!(inode = new_inode(dir->i_sb))) { + if (!(inode =
> > get_new_inode(dir, mode))) return -ENOMEM; - } -
> > new_inode_init(inode, dir, mode);
> >
> > jbegin_count += reiserfs_cache_default_acl(dir); retval =
> > reiserfs_security_init(dir, inode, &dentry->d_name, &security); @@
> > -1097,10 +1101,8 @@ static int reiserfs_symlink(struct inode
> > *parent_dir,
> >
> > dquot_initialize(parent_dir);
> >
> > - if (!(inode = new_inode(parent_dir->i_sb))) { + if (!(inode =
> > get_new_inode(parent_dir, mode))) return -ENOMEM; - } -
> > new_inode_init(inode, parent_dir, mode);
> >
> > retval = reiserfs_security_init(parent_dir, inode,
> > &dentry->d_name, &security);
> >
>
>
> - --
> Jeff Mahoney
> SUSE Labs
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG/MacGPG2 v2.0.19 (Darwin)
>
> iQIcBAEBAgAGBQJT6RibAAoJEB57S2MheeWyQN8P/A9BE9Xbp0tSwBi1TklqE3Hm
> xKK1IHgdVPODpysKKbr6V2llVOl+Fis/1kT7SKyVZ5LraVhrfQU/37HzO0535ITR
> n1bBiQV/PgejEtkSqKDMispk0kGaVCCgg2o6scHg3Qr8w9qF2mmrTUUOQPcMsaEa
> YKmd7sN+3+RbvP7mHderAM4Yjb+EXzjlgfqyOZYg+jY8Id+b5Yn5TekMQJMvvsPh
> tWiGaDF9cYsVZL2fdNUvOXdUMycYx2xDTzcCNBOEHh9FHS3kIVrCtrwfXQJEys5a
> nB1Es94tAW2M3Au/8L3kugcy8iR7gxqGFctCtfmtUpm7SiusNDIfXyNpbRtgdbF5
> QFlqmNF2mLDYKNr93SbMLbF+fnl1NzkoFmrsu17bWiiQaxmqNDsnZeNUANWSrXsE
> QXS2Ardd24+BOW2aiSQXXwSkaHeQy3h/H9gX+TqbpUBdsRJO9bggm7IDFNn7n4ow
> mM0jRlu5tzyUHQRjzkwqqAqjBTB9l+rEV5zwikNE9QzWjovlKZ+NltW3mkMtYKKX
> P/wQs49zKT8g+InLiLFXanYrZkMpQ/HgQGw3rRMd5o5P2I0kih3Flaa2Gwlf4P7n
> 6xcv+STczVUg/htRUIMppQVIkriOD92QVmKAzmIsPBXIPTpJNVjrknc0e8MDDfql
> 8j0moTvJ8CloSg25k0W5
> =xfQ5
> -----END PGP SIGNATURE-----
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
