Skip to content

Commit

Permalink
[GFS2] Fix bug in directory code and tidy up
Browse files Browse the repository at this point in the history
Due to a typo, the dir leaf split operation was (for the first
split in a directory) writing the new hash vaules at the
wrong offset. This is now fixed.

Also some other tidy ups are included:

 - We use GFS2's hash function for dentries (see ops_dentry.c) so that
   we don't have to keep recalculating the hash values.
 - A lot of common code is eliminated between the various directory
   lookup routines.
 - Better error checking on directory lookup (previously different
   routines checked for different errors)
 - The leaf split operation has a couple of redundant operations
   removed from it, so it should be faster.

There is still further scope for further clean ups in the directory
code, and readdir in particular could do with slimming down a bit.

Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
  • Loading branch information
Steven Whitehouse committed Mar 20, 2006
1 parent 419c93e commit c752666
Show file tree
Hide file tree
Showing 11 changed files with 698 additions and 999 deletions.
1,390 changes: 534 additions & 856 deletions fs/gfs2/dir.c

Large diffs are not rendered by default.

39 changes: 24 additions & 15 deletions fs/gfs2/dir.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,34 @@ typedef int (*gfs2_filldir_t) (void *opaque,
uint64_t offset,
struct gfs2_inum *inum, unsigned int type);

int gfs2_filecmp(struct qstr *file1, char *file2, int len_of_file2);
int gfs2_dirent_alloc(struct gfs2_inode *dip, struct buffer_head *bh,
int name_len, struct gfs2_dirent **dent_out);

int gfs2_dir_search(struct gfs2_inode *dip, struct qstr *filename,
struct gfs2_inum *inum, unsigned int *type);
int gfs2_dir_add(struct gfs2_inode *dip, struct qstr *filename,
struct gfs2_inum *inum, unsigned int type);
int gfs2_dir_del(struct gfs2_inode *dip, struct qstr *filename);
int gfs2_dir_search(struct inode *dir, const struct qstr *filename,
struct gfs2_inum *inum, unsigned int *type);
int gfs2_dir_add(struct inode *inode, const struct qstr *filename,
const struct gfs2_inum *inum, unsigned int type);
int gfs2_dir_del(struct gfs2_inode *dip, const struct qstr *filename);
int gfs2_dir_read(struct gfs2_inode *dip, uint64_t * offset, void *opaque,
gfs2_filldir_t filldir);
int gfs2_dir_mvino(struct gfs2_inode *dip, struct qstr *filename,
struct gfs2_inum *new_inum, unsigned int new_type);
gfs2_filldir_t filldir);
int gfs2_dir_mvino(struct gfs2_inode *dip, const struct qstr *filename,
struct gfs2_inum *new_inum, unsigned int new_type);

int gfs2_dir_exhash_dealloc(struct gfs2_inode *dip);

int gfs2_diradd_alloc_required(struct gfs2_inode *dip, struct qstr *filename,
int *alloc_required);
int gfs2_diradd_alloc_required(struct inode *dir,
const struct qstr *filename);
int gfs2_dir_get_buffer(struct gfs2_inode *ip, uint64_t block, int new,
struct buffer_head **bhp);
struct buffer_head **bhp);

/* N.B. This probably ought to take inum & type as args as well */
static inline void gfs2_qstr2dirent(const struct qstr *name, u16 reclen, struct gfs2_dirent *dent)
{
dent->de_inum.no_addr = cpu_to_be64(0);
dent->de_inum.no_formal_ino = cpu_to_be64(0);
dent->de_hash = cpu_to_be32(name->hash);
dent->de_rec_len = cpu_to_be16(reclen);
dent->de_name_len = cpu_to_be16(name->len);
dent->de_type = cpu_to_be16(0);
memset(dent->__pad, 0, sizeof(dent->__pad));
memcpy((char*)(dent+1), name->name, name->len);
}

#endif /* __DIR_DOT_H__ */
63 changes: 40 additions & 23 deletions fs/gfs2/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,10 @@ struct inode *gfs2_iget(struct super_block *sb, struct gfs2_inum *inum)

void gfs2_inode_min_init(struct gfs2_inode *ip, unsigned int type)
{
spin_lock(&ip->i_spin);
if (!test_and_set_bit(GIF_MIN_INIT, &ip->i_flags)) {
ip->i_di.di_nlink = 1;
ip->i_di.di_mode = DT2IF(type);
}
spin_unlock(&ip->i_spin);
}

/**
Expand All @@ -257,10 +255,8 @@ int gfs2_inode_refresh(struct gfs2_inode *ip)
return -EIO;
}

spin_lock(&ip->i_spin);
gfs2_dinode_in(&ip->i_di, dibh->b_data);
set_bit(GIF_MIN_INIT, &ip->i_flags);
spin_unlock(&ip->i_spin);

brelse(dibh);

Expand Down Expand Up @@ -702,6 +698,16 @@ int gfs2_change_nlink(struct gfs2_inode *ip, int diff)
return 0;
}

struct inode *gfs2_lookup_simple(struct inode *dip, const char *name)
{
struct qstr qstr;
qstr.name = name;
qstr.len = strlen(name);
qstr.hash = gfs2_disk_hash(qstr.name, qstr.len);
return gfs2_lookupi(dip, &qstr, 1, NULL);
}


/**
* gfs2_lookupi - Look up a filename in a directory and return its inode
* @d_gh: An initialized holder for the directory glock
Expand All @@ -715,8 +721,9 @@ int gfs2_change_nlink(struct gfs2_inode *ip, int diff)
* Returns: errno
*/

int gfs2_lookupi(struct inode *dir, struct qstr *name, int is_root,
struct inode **inodep)
struct inode *gfs2_lookupi(struct inode *dir, struct qstr *name, int is_root,
struct nameidata *nd)

{
struct super_block *sb = dir->i_sb;
struct gfs2_inode *ipp;
Expand All @@ -727,30 +734,30 @@ int gfs2_lookupi(struct inode *dir, struct qstr *name, int is_root,
unsigned int type;
struct gfs2_glock *gl;
int error = 0;

*inodep = NULL;
struct inode *inode = NULL;

if (!name->len || name->len > GFS2_FNAMESIZE)
return -ENAMETOOLONG;
return ERR_PTR(-ENAMETOOLONG);

if (gfs2_filecmp(name, ".", 1) ||
(gfs2_filecmp(name, "..", 2) && dir == sb->s_root->d_inode)) {
if ((name->len == 1 && memcmp(name->name, ".", 1) == 0) ||
(name->len == 2 && memcmp(name->name, "..", 2) == 0 &&
dir == sb->s_root->d_inode)) {
gfs2_inode_hold(dip);
ipp = dip;
goto done;
}

error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0, &d_gh);
if (error)
return error;
return ERR_PTR(error);

if (!is_root) {
error = gfs2_repermission(dip->i_vnode, MAY_EXEC, NULL);
if (error)
goto out;
}

error = gfs2_dir_search(dip, name, &inum, &type);
error = gfs2_dir_search(dir, name, &inum, &type);
if (error)
goto out;

Expand All @@ -768,13 +775,16 @@ int gfs2_lookupi(struct inode *dir, struct qstr *name, int is_root,
out:
gfs2_glock_dq_uninit(&d_gh);
done:
if (error == -ENOENT)
return NULL;
if (error == 0) {
*inodep = gfs2_ip2v(ipp);
if (!*inodep)
error = -ENOMEM;
inode = gfs2_ip2v(ipp);
gfs2_inode_put(ipp);
if (!inode)
return ERR_PTR(-ENOMEM);
return inode;
}
return error;
return ERR_PTR(error);
}

static int pick_formal_ino_1(struct gfs2_sbd *sdp, uint64_t *formal_ino)
Expand Down Expand Up @@ -918,7 +928,7 @@ static int create_ok(struct gfs2_inode *dip, struct qstr *name,
if (!dip->i_di.di_nlink)
return -EPERM;

error = gfs2_dir_search(dip, name, NULL, NULL);
error = gfs2_dir_search(dip->i_vnode, name, NULL, NULL);
switch (error) {
case -ENOENT:
error = 0;
Expand Down Expand Up @@ -1116,7 +1126,9 @@ static int link_dinode(struct gfs2_inode *dip, struct qstr *name,
if (error)
goto fail;

error = gfs2_diradd_alloc_required(dip, name, &alloc_required);
error = alloc_required = gfs2_diradd_alloc_required(dip->i_vnode, name);
if (alloc_required < 0)
goto fail;
if (alloc_required) {
error = gfs2_quota_check(dip, dip->i_di.di_uid,
dip->i_di.di_gid);
Expand Down Expand Up @@ -1145,7 +1157,7 @@ static int link_dinode(struct gfs2_inode *dip, struct qstr *name,
goto fail_quota_locks;
}

error = gfs2_dir_add(dip, name, &ip->i_num, IF2DT(ip->i_di.di_mode));
error = gfs2_dir_add(dip->i_vnode, name, &ip->i_num, IF2DT(ip->i_di.di_mode));
if (error)
goto fail_end_trans;

Expand Down Expand Up @@ -1379,12 +1391,14 @@ int gfs2_rmdiri(struct gfs2_inode *dip, struct qstr *name,

dotname.len = 1;
dotname.name = ".";
dotname.hash = gfs2_disk_hash(dotname.name, dotname.len);
error = gfs2_dir_del(ip, &dotname);
if (error)
return error;

dotname.len = 2;
dotname.name = "..";
dotname.hash = gfs2_disk_hash(dotname.name, dotname.len);
error = gfs2_dir_del(ip, &dotname);
if (error)
return error;
Expand Down Expand Up @@ -1439,7 +1453,7 @@ int gfs2_unlink_ok(struct gfs2_inode *dip, struct qstr *name,
if (error)
return error;

error = gfs2_dir_search(dip, name, &inum, &type);
error = gfs2_dir_search(dip->i_vnode, name, &inum, &type);
if (error)
return error;

Expand Down Expand Up @@ -1476,6 +1490,7 @@ int gfs2_ok_to_move(struct gfs2_inode *this, struct gfs2_inode *to)
memset(&dotdot, 0, sizeof(struct qstr));
dotdot.name = "..";
dotdot.len = 2;
dotdot.hash = gfs2_disk_hash(dotdot.name, dotdot.len);

igrab(dir);

Expand All @@ -1489,9 +1504,11 @@ int gfs2_ok_to_move(struct gfs2_inode *this, struct gfs2_inode *to)
break;
}

error = gfs2_lookupi(dir, &dotdot, 1, &tmp);
if (error)
tmp = gfs2_lookupi(dir, &dotdot, 1, NULL);
if (IS_ERR(tmp)) {
error = PTR_ERR(tmp);
break;
}

iput(dir);
dir = tmp;
Expand Down
16 changes: 3 additions & 13 deletions fs/gfs2/inode.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ void gfs2_inode_destroy(struct gfs2_inode *ip);
int gfs2_inode_dealloc(struct gfs2_sbd *sdp, struct gfs2_unlinked *ul);

int gfs2_change_nlink(struct gfs2_inode *ip, int diff);
int gfs2_lookupi(struct inode *dir, struct qstr *name, int is_root,
struct inode **ipp);
struct inode *gfs2_lookupi(struct inode *dir, struct qstr *name, int is_root,
struct nameidata *nd);
struct inode *gfs2_createi(struct gfs2_holder *ghs, struct qstr *name,
unsigned int mode);
int gfs2_unlinki(struct gfs2_inode *dip, struct qstr *name,
Expand All @@ -66,17 +66,7 @@ int gfs2_setattr_simple(struct gfs2_inode *ip, struct iattr *attr);

int gfs2_repermission(struct inode *inode, int mask, struct nameidata *nd);

static inline int gfs2_lookup_simple(struct inode *dip, char *name,
struct inode **ipp)
{
struct qstr qstr;
int err;
memset(&qstr, 0, sizeof(struct qstr));
qstr.name = name;
qstr.len = strlen(name);
err = gfs2_lookupi(dip, &qstr, 1, ipp);
return err;
}
struct inode *gfs2_lookup_simple(struct inode *dip, const char *name);

#endif /* __INODE_DOT_H__ */

13 changes: 1 addition & 12 deletions fs/gfs2/ondisk.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void gfs2_inum_in(struct gfs2_inum *no, char *buf)
no->no_addr = be64_to_cpu(str->no_addr);
}

void gfs2_inum_out(struct gfs2_inum *no, char *buf)
void gfs2_inum_out(const struct gfs2_inum *no, char *buf)
{
struct gfs2_inum *str = (struct gfs2_inum *)buf;

Expand Down Expand Up @@ -342,17 +342,6 @@ void gfs2_dirent_print(struct gfs2_dirent *de, char *name)
printk(KERN_INFO " name = %s\n", buf);
}

void gfs2_leaf_in(struct gfs2_leaf *lf, char *buf)
{
struct gfs2_leaf *str = (struct gfs2_leaf *)buf;

gfs2_meta_header_in(&lf->lf_header, buf);
lf->lf_depth = be16_to_cpu(str->lf_depth);
lf->lf_entries = be16_to_cpu(str->lf_entries);
lf->lf_dirent_format = be32_to_cpu(str->lf_dirent_format);
lf->lf_next = be64_to_cpu(str->lf_next);
}

void gfs2_leaf_print(struct gfs2_leaf *lf)
{
gfs2_meta_header_print(&lf->lf_header);
Expand Down
21 changes: 13 additions & 8 deletions fs/gfs2/ops_dentry.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,25 +38,26 @@
static int gfs2_drevalidate(struct dentry *dentry, struct nameidata *nd)
{
struct dentry *parent = dget_parent(dentry);
struct gfs2_sbd *sdp = parent->d_inode->i_sb->s_fs_info;
struct gfs2_inode *dip = parent->d_inode->u.generic_ip;
struct inode *inode;
struct inode *inode = dentry->d_inode;
struct gfs2_holder d_gh;
struct gfs2_inode *ip;
struct gfs2_inum inum;
unsigned int type;
int error;

lock_kernel();

inode = dentry->d_inode;
if (inode && is_bad_inode(inode))
goto invalid;

if (sdp->sd_args.ar_localcaching)
goto valid;

error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0, &d_gh);
if (error)
goto fail;

error = gfs2_dir_search(dip, &dentry->d_name, &inum, &type);
error = gfs2_dir_search(parent->d_inode, &dentry->d_name, &inum, &type);
switch (error) {
case 0:
if (!inode)
Expand Down Expand Up @@ -84,7 +85,6 @@ static int gfs2_drevalidate(struct dentry *dentry, struct nameidata *nd)
gfs2_glock_dq_uninit(&d_gh);

valid:
unlock_kernel();
dput(parent);
return 1;

Expand All @@ -99,20 +99,25 @@ static int gfs2_drevalidate(struct dentry *dentry, struct nameidata *nd)
}
d_drop(dentry);

unlock_kernel();
dput(parent);
return 0;

fail_gunlock:
gfs2_glock_dq_uninit(&d_gh);

fail:
unlock_kernel();
dput(parent);
return 0;
}

static int gfs2_dhash(struct dentry *dentry, struct qstr *str)
{
str->hash = gfs2_disk_hash(str->name, str->len);
return 0;
}

struct dentry_operations gfs2_dops = {
.d_revalidate = gfs2_drevalidate,
.d_hash = gfs2_dhash,
};

13 changes: 9 additions & 4 deletions fs/gfs2/ops_export.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "inode.h"
#include "ops_export.h"
#include "rgrp.h"
#include "util.h"

static struct dentry *gfs2_decode_fh(struct super_block *sb,
__u32 *fh,
Expand Down Expand Up @@ -167,11 +168,15 @@ static struct dentry *gfs2_get_parent(struct dentry *child)
struct qstr dotdot = { .name = "..", .len = 2 };
struct inode *inode;
struct dentry *dentry;
int error;

error = gfs2_lookupi(child->d_inode, &dotdot, 1, &inode);
if (error)
return ERR_PTR(error);
dotdot.hash = gfs2_disk_hash(dotdot.name, dotdot.len);

inode = gfs2_lookupi(child->d_inode, &dotdot, 1, NULL);

if (!inode)
return ERR_PTR(-ENOENT);
if (IS_ERR(inode))
return ERR_PTR(PTR_ERR(inode));

dentry = d_alloc_anon(inode);
if (!dentry) {
Expand Down
Loading

0 comments on commit c752666

Please sign in to comment.