* [PATCH 0/4] XFS: Case-insensitive support - ASCII only
@ 2008-05-13 7:57 Barry Naujok
2008-05-13 7:57 ` [PATCH 1/4] XFS: Name operation vector for hash and compare Barry Naujok
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Barry Naujok @ 2008-05-13 7:57 UTC (permalink / raw)
To: xfs; +Cc: linux-fsdevel
This patch seems to resolve all issues I had with CI support with the
dentry cache.
My solution for negative dentries was to not call d_add() when the
inode was not found. I had to fix up the create so it would call
d_rehash if the entry was unhashed.
I've also applied the other previous suggestions and cleanups.
The only outstanding problem is case-preserving rename on a CI
filesystem. I will write a patch for passing flags down to the
fs-specific lookup once the nameidata patch mentioned by
Christoph has been removed.
--
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/4] XFS: Name operation vector for hash and compare
2008-05-13 7:57 [PATCH 0/4] XFS: Case-insensitive support - ASCII only Barry Naujok
@ 2008-05-13 7:57 ` Barry Naujok
2008-05-13 8:31 ` Christoph Hellwig
2008-05-13 7:57 ` [PATCH 2/4] XFS: add op_flags field and helpers to xfs_da_args Barry Naujok
` (2 subsequent siblings)
3 siblings, 1 reply; 22+ messages in thread
From: Barry Naujok @ 2008-05-13 7:57 UTC (permalink / raw)
To: xfs; +Cc: linux-fsdevel
[-- Attachment #1: nameops.patch --]
[-- Type: text/plain, Size: 20184 bytes --]
Adds two pieces of functionality for the basis of case-insensitive
support in XFS:
1. A comparison result enumerated type: xfs_dacmp. It represents an
exact match, case-insensitive match or no match at all. This patch
only implements different and exact results.
2. xfs_nameops vector for specifying how to perform the hash generation
of filenames and comparision methods. In this patch the hash vector
points to the existing xfs_da_hashname function and the comparison
method does a length compare, and if the same, does a memcmp and
return the xfs_dacmp result.
All filename functions that use the hash (create, lookup remove, rename,
etc) now use the xfs_nameops.hashname function and all directory lookup
functions also use the xfs_nameops.compname function.
The lookup functions also handle case-insensitive results even though
the default comparison function cannot return that. And important
aspect of the lookup functions is that an exact match always has
precedence over a case-insensitive. So while a case-insensitive match
is found, we have to keep looking just in case there is an exact
match. In the meantime, the info for the first case-insensitive match
is retained if no exact match is found.
Signed-off-by: Barry Naujok <bnaujok@sgi.com>
---
fs/xfs/xfs_da_btree.c | 22 +++++++++++++++++
fs/xfs/xfs_da_btree.h | 22 +++++++++++++++++
fs/xfs/xfs_dir2.c | 12 +++++----
fs/xfs/xfs_dir2_block.c | 33 ++++++++++++++++++-------
fs/xfs/xfs_dir2_data.c | 5 +++
fs/xfs/xfs_dir2_leaf.c | 60 +++++++++++++++++++++++++++++++++-------------
fs/xfs/xfs_dir2_node.c | 25 +++++++++++--------
fs/xfs/xfs_dir2_sf.c | 62 +++++++++++++++++++++++++++---------------------
fs/xfs/xfs_mount.h | 2 +
9 files changed, 175 insertions(+), 68 deletions(-)
Index: kern_ci/fs/xfs/xfs_da_btree.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_da_btree.c
+++ kern_ci/fs/xfs/xfs_da_btree.c
@@ -1530,6 +1530,28 @@ xfs_da_hashname(const uchar_t *name, int
}
}
+enum xfs_dacmp
+xfs_da_compname(
+ struct xfs_da_args *args,
+ const char *name,
+ int len)
+{
+ return (args->namelen == len && memcmp(args->name, name, len) == 0) ?
+ XFS_CMP_EXACT : XFS_CMP_DIFFERENT;
+}
+
+static xfs_dahash_t
+xfs_default_hashname(
+ struct xfs_name *name)
+{
+ return xfs_da_hashname(name->name, name->len);
+}
+
+const struct xfs_nameops xfs_default_nameops = {
+ .hashname = xfs_default_hashname,
+ .compname = xfs_da_compname
+};
+
/*
* Add a block to the btree ahead of the file.
* Return the new block number to the caller.
Index: kern_ci/fs/xfs/xfs_da_btree.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_da_btree.h
+++ kern_ci/fs/xfs/xfs_da_btree.h
@@ -99,6 +99,15 @@ typedef struct xfs_da_node_entry xfs_da_
*========================================================================*/
/*
+ * Search comparison results
+ */
+enum xfs_dacmp {
+ XFS_CMP_DIFFERENT, /* names are completely different */
+ XFS_CMP_EXACT, /* names are exactly the same */
+ XFS_CMP_CASE /* names are same but differ in case */
+};
+
+/*
* Structure to ease passing around component names.
*/
typedef struct xfs_da_args {
@@ -127,6 +136,7 @@ typedef struct xfs_da_args {
unsigned char rename; /* T/F: this is an atomic rename op */
unsigned char addname; /* T/F: this is an add operation */
unsigned char oknoent; /* T/F: ok to return ENOENT, else die */
+ enum xfs_dacmp cmpresult; /* name compare result for lookups */
} xfs_da_args_t;
/*
@@ -201,6 +211,14 @@ typedef struct xfs_da_state {
(uint)(XFS_DA_LOGOFF(BASE, ADDR)), \
(uint)(XFS_DA_LOGOFF(BASE, ADDR)+(SIZE)-1)
+/*
+ * Name ops for directory and/or attr name operations
+ */
+struct xfs_nameops {
+ xfs_dahash_t (*hashname)(struct xfs_name *);
+ enum xfs_dacmp (*compname)(struct xfs_da_args *, const char *, int);
+};
+
#ifdef __KERNEL__
/*========================================================================
@@ -249,6 +267,10 @@ int xfs_da_shrink_inode(xfs_da_args_t *a
xfs_dabuf_t *dead_buf);
uint xfs_da_hashname(const uchar_t *name_string, int name_length);
+enum xfs_dacmp xfs_da_compname(struct xfs_da_args *args,
+ const char *name, int len);
+
+
xfs_da_state_t *xfs_da_state_alloc(void);
void xfs_da_state_free(xfs_da_state_t *state);
Index: kern_ci/fs/xfs/xfs_dir2.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2.c
+++ kern_ci/fs/xfs/xfs_dir2.c
@@ -67,6 +67,7 @@ xfs_dir_mount(
(mp->m_dirblksize - (uint)sizeof(xfs_da_node_hdr_t)) /
(uint)sizeof(xfs_da_node_entry_t);
mp->m_dir_magicpct = (mp->m_dirblksize * 37) / 100;
+ mp->m_dirnameops = &xfs_default_nameops;
}
/*
@@ -166,7 +167,7 @@ xfs_dir_createname(
args.name = name->name;
args.namelen = name->len;
- args.hashval = xfs_da_hashname(name->name, name->len);
+ args.hashval = dp->i_mount->m_dirnameops->hashname(name);
args.inumber = inum;
args.dp = dp;
args.firstblock = first;
@@ -212,11 +213,12 @@ xfs_dir_lookup(
args.name = name->name;
args.namelen = name->len;
- args.hashval = xfs_da_hashname(name->name, name->len);
+ args.hashval = dp->i_mount->m_dirnameops->hashname(name);
args.dp = dp;
args.whichfork = XFS_DATA_FORK;
args.trans = tp;
args.oknoent = 1;
+ args.cmpresult = XFS_CMP_DIFFERENT;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
rval = xfs_dir2_sf_lookup(&args);
@@ -259,7 +261,7 @@ xfs_dir_removename(
args.name = name->name;
args.namelen = name->len;
- args.hashval = xfs_da_hashname(name->name, name->len);
+ args.hashval = dp->i_mount->m_dirnameops->hashname(name);
args.inumber = ino;
args.dp = dp;
args.firstblock = first;
@@ -342,7 +344,7 @@ xfs_dir_replace(
args.name = name->name;
args.namelen = name->len;
- args.hashval = xfs_da_hashname(name->name, name->len);
+ args.hashval = dp->i_mount->m_dirnameops->hashname(name);
args.inumber = inum;
args.dp = dp;
args.firstblock = first;
@@ -390,7 +392,7 @@ xfs_dir_canenter(
args.name = name->name;
args.namelen = name->len;
- args.hashval = xfs_da_hashname(name->name, name->len);
+ args.hashval = dp->i_mount->m_dirnameops->hashname(name);
args.dp = dp;
args.whichfork = XFS_DATA_FORK;
args.trans = tp;
Index: kern_ci/fs/xfs/xfs_dir2_block.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_block.c
+++ kern_ci/fs/xfs/xfs_dir2_block.c
@@ -643,6 +643,7 @@ xfs_dir2_block_lookup_int(
int mid; /* binary search current idx */
xfs_mount_t *mp; /* filesystem mount point */
xfs_trans_t *tp; /* transaction pointer */
+ enum xfs_dacmp cmp; /* comparison result */
dp = args->dp;
tp = args->trans;
@@ -697,20 +698,31 @@ xfs_dir2_block_lookup_int(
dep = (xfs_dir2_data_entry_t *)
((char *)block + xfs_dir2_dataptr_to_off(mp, addr));
/*
- * Compare, if it's right give back buffer & entry number.
+ * Compare name and if it's an exact match, return the index
+ * and buffer. If it's the first case-insensitive match, store
+ * the index and buffer and continue looking for an exact match.
*/
- if (dep->namelen == args->namelen &&
- dep->name[0] == args->name[0] &&
- memcmp(dep->name, args->name, args->namelen) == 0) {
+ cmp = mp->m_dirnameops->compname(args, dep->name, dep->namelen);
+ if (cmp != XFS_CMP_DIFFERENT && cmp != args->cmpresult) {
+ args->cmpresult = cmp;
*bpp = bp;
*entno = mid;
- return 0;
+ if (cmp == XFS_CMP_EXACT)
+ return 0;
}
- } while (++mid < be32_to_cpu(btp->count) && be32_to_cpu(blp[mid].hashval) == hash);
+ } while (++mid < be32_to_cpu(btp->count) &&
+ be32_to_cpu(blp[mid].hashval) == hash);
+
+ ASSERT(args->oknoent);
+ /*
+ * Here, we can only be doing a lookup (not a rename or replace).
+ * If a case-insensitive match was found earlier, return success.
+ */
+ if (args->cmpresult == XFS_CMP_CASE)
+ return 0;
/*
* No match, release the buffer and return ENOENT.
*/
- ASSERT(args->oknoent);
xfs_da_brelse(tp, bp);
return XFS_ERROR(ENOENT);
}
@@ -1033,6 +1045,7 @@ xfs_dir2_sf_to_block(
xfs_dir2_sf_t *sfp; /* shortform structure */
__be16 *tagp; /* end of data entry */
xfs_trans_t *tp; /* transaction pointer */
+ struct xfs_name name;
xfs_dir2_trace_args("sf_to_block", args);
dp = args->dp;
@@ -1187,8 +1200,10 @@ xfs_dir2_sf_to_block(
tagp = xfs_dir2_data_entry_tag_p(dep);
*tagp = cpu_to_be16((char *)dep - (char *)block);
xfs_dir2_data_log_entry(tp, bp, dep);
- blp[2 + i].hashval = cpu_to_be32(xfs_da_hashname(
- (char *)sfep->name, sfep->namelen));
+ name.name = sfep->name;
+ name.len = sfep->namelen;
+ blp[2 + i].hashval = cpu_to_be32(mp->m_dirnameops->
+ hashname(&name));
blp[2 + i].address = cpu_to_be32(xfs_dir2_byte_to_dataptr(mp,
(char *)dep - (char *)block));
offset = (int)((char *)(tagp + 1) - (char *)block);
Index: kern_ci/fs/xfs/xfs_dir2_data.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_data.c
+++ kern_ci/fs/xfs/xfs_dir2_data.c
@@ -65,6 +65,7 @@ xfs_dir2_data_check(
xfs_mount_t *mp; /* filesystem mount point */
char *p; /* current data position */
int stale; /* count of stale leaves */
+ struct xfs_name name;
mp = dp->i_mount;
d = bp->data;
@@ -140,7 +141,9 @@ xfs_dir2_data_check(
addr = xfs_dir2_db_off_to_dataptr(mp, mp->m_dirdatablk,
(xfs_dir2_data_aoff_t)
((char *)dep - (char *)d));
- hash = xfs_da_hashname((char *)dep->name, dep->namelen);
+ name.name = dep->name;
+ name.len = dep->namelen;
+ hash = mp->m_dirnameops->hashname(&name);
for (i = 0; i < be32_to_cpu(btp->count); i++) {
if (be32_to_cpu(lep[i].address) == addr &&
be32_to_cpu(lep[i].hashval) == hash)
Index: kern_ci/fs/xfs/xfs_dir2_leaf.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_leaf.c
+++ kern_ci/fs/xfs/xfs_dir2_leaf.c
@@ -1331,6 +1331,8 @@ xfs_dir2_leaf_lookup_int(
xfs_mount_t *mp; /* filesystem mount point */
xfs_dir2_db_t newdb; /* new data block number */
xfs_trans_t *tp; /* transaction pointer */
+ xfs_dabuf_t *cbp; /* case match data buffer */
+ enum xfs_dacmp cmp; /* name compare result */
dp = args->dp;
tp = args->trans;
@@ -1354,9 +1356,11 @@ xfs_dir2_leaf_lookup_int(
* Loop over all the entries with the right hash value
* looking to match the name.
*/
+ cbp = NULL;
for (lep = &leaf->ents[index], dbp = NULL, curdb = -1;
- index < be16_to_cpu(leaf->hdr.count) && be32_to_cpu(lep->hashval) == args->hashval;
- lep++, index++) {
+ index < be16_to_cpu(leaf->hdr.count) &&
+ be32_to_cpu(lep->hashval) == args->hashval;
+ lep++, index++) {
/*
* Skip over stale leaf entries.
*/
@@ -1371,12 +1375,12 @@ xfs_dir2_leaf_lookup_int(
* need to pitch the old one and read the new one.
*/
if (newdb != curdb) {
- if (dbp)
+ if (dbp != cbp)
xfs_da_brelse(tp, dbp);
- if ((error =
- xfs_da_read_buf(tp, dp,
- xfs_dir2_db_to_da(mp, newdb), -1, &dbp,
- XFS_DATA_FORK))) {
+ error = xfs_da_read_buf(tp, dp,
+ xfs_dir2_db_to_da(mp, newdb),
+ -1, &dbp, XFS_DATA_FORK);
+ if (error) {
xfs_da_brelse(tp, lbp);
return error;
}
@@ -1386,24 +1390,46 @@ xfs_dir2_leaf_lookup_int(
/*
* Point to the data entry.
*/
- dep = (xfs_dir2_data_entry_t *)
- ((char *)dbp->data +
- xfs_dir2_dataptr_to_off(mp, be32_to_cpu(lep->address)));
+ dep = (xfs_dir2_data_entry_t *)((char *)dbp->data +
+ xfs_dir2_dataptr_to_off(mp, be32_to_cpu(lep->address)));
/*
- * If it matches then return it.
+ * Compare name and if it's an exact match, return the index
+ * and buffer. If it's the first case-insensitive match, store
+ * the index and buffer and continue looking for an exact match.
*/
- if (dep->namelen == args->namelen &&
- dep->name[0] == args->name[0] &&
- memcmp(dep->name, args->name, args->namelen) == 0) {
- *dbpp = dbp;
+ cmp = mp->m_dirnameops->compname(args, dep->name, dep->namelen);
+ if (cmp != XFS_CMP_DIFFERENT && cmp != args->cmpresult) {
+ args->cmpresult = cmp;
*indexp = index;
- return 0;
+ /*
+ * case exact match: release the stored CI buffer if it
+ * exists and return the current buffer.
+ */
+ if (cmp == XFS_CMP_EXACT) {
+ if (cbp && cbp != dbp)
+ xfs_da_brelse(tp, cbp);
+ *dbpp = dbp;
+ return 0;
+ }
+ cbp = dbp;
}
}
+ ASSERT(args->oknoent);
+ /*
+ * Here, we can only be doing a lookup (not a rename or replace).
+ * If a case-insensitive match was found earlier, release the current
+ * buffer and return the stored CI matching buffer.
+ */
+ if (args->cmpresult == XFS_CMP_CASE) {
+ if (cbp != dbp)
+ xfs_da_brelse(tp, dbp);
+ *dbpp = cbp;
+ return 0;
+ }
/*
* No match found, return ENOENT.
*/
- ASSERT(args->oknoent);
+ ASSERT(cbp == NULL);
if (dbp)
xfs_da_brelse(tp, dbp);
xfs_da_brelse(tp, lbp);
Index: kern_ci/fs/xfs/xfs_dir2_node.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_node.c
+++ kern_ci/fs/xfs/xfs_dir2_node.c
@@ -556,6 +556,7 @@ xfs_dir2_leafn_lookup_for_entry(
xfs_mount_t *mp; /* filesystem mount point */
xfs_dir2_db_t newdb; /* new data block number */
xfs_trans_t *tp; /* transaction pointer */
+ enum xfs_dacmp cmp; /* comparison result */
dp = args->dp;
tp = args->trans;
@@ -620,17 +621,21 @@ xfs_dir2_leafn_lookup_for_entry(
dep = (xfs_dir2_data_entry_t *)((char *)curbp->data +
xfs_dir2_dataptr_to_off(mp, be32_to_cpu(lep->address)));
/*
- * Compare the entry, return it if it matches.
- */
- if (dep->namelen == args->namelen && memcmp(dep->name,
- args->name, args->namelen) == 0) {
+ * Compare the entry and if it's an exact match, return
+ * EEXIST immediately. If it's the first case-insensitive
+ * match, store the inode number and continue looking.
+ */
+ cmp = mp->m_dirnameops->compname(args, dep->name, dep->namelen);
+ if (cmp != XFS_CMP_DIFFERENT && cmp != args->cmpresult) {
+ args->cmpresult = cmp;
args->inumber = be64_to_cpu(dep->inumber);
di = (int)((char *)dep - (char *)curbp->data);
error = EEXIST;
- goto out;
+ if (cmp == XFS_CMP_EXACT)
+ goto out;
}
}
- /* Didn't find a match. */
+ /* Didn't find an exact match. */
error = ENOENT;
di = -1;
ASSERT(index == be16_to_cpu(leaf->hdr.count) || args->oknoent);
@@ -1813,6 +1818,8 @@ xfs_dir2_node_lookup(
error = xfs_da_node_lookup_int(state, &rval);
if (error)
rval = error;
+ else if (rval == ENOENT && args->cmpresult == XFS_CMP_CASE)
+ rval = EEXIST; /* a case-insensitive match was found */
/*
* Release the btree blocks and leaf block.
*/
@@ -1856,9 +1863,8 @@ xfs_dir2_node_removename(
* Look up the entry we're deleting, set up the cursor.
*/
error = xfs_da_node_lookup_int(state, &rval);
- if (error) {
+ if (error)
rval = error;
- }
/*
* Didn't find it, upper layer screwed up.
*/
@@ -1875,9 +1881,8 @@ xfs_dir2_node_removename(
*/
error = xfs_dir2_leafn_remove(args, blk->bp, blk->index,
&state->extrablk, &rval);
- if (error) {
+ if (error)
return error;
- }
/*
* Fix the hash values up the btree.
*/
Index: kern_ci/fs/xfs/xfs_dir2_sf.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_sf.c
+++ kern_ci/fs/xfs/xfs_dir2_sf.c
@@ -814,6 +814,7 @@ xfs_dir2_sf_lookup(
int i; /* entry index */
xfs_dir2_sf_entry_t *sfep; /* shortform directory entry */
xfs_dir2_sf_t *sfp; /* shortform structure */
+ enum xfs_dacmp cmp; /* comparison result */
xfs_dir2_trace_args("sf_lookup", args);
xfs_dir2_sf_check(args);
@@ -836,6 +837,7 @@ xfs_dir2_sf_lookup(
*/
if (args->namelen == 1 && args->name[0] == '.') {
args->inumber = dp->i_ino;
+ args->cmpresult = XFS_CMP_EXACT;
return XFS_ERROR(EEXIST);
}
/*
@@ -844,27 +846,39 @@ xfs_dir2_sf_lookup(
if (args->namelen == 2 &&
args->name[0] == '.' && args->name[1] == '.') {
args->inumber = xfs_dir2_sf_get_inumber(sfp, &sfp->hdr.parent);
+ args->cmpresult = XFS_CMP_EXACT;
return XFS_ERROR(EEXIST);
}
/*
* Loop over all the entries trying to match ours.
*/
- for (i = 0, sfep = xfs_dir2_sf_firstentry(sfp);
- i < sfp->hdr.count;
- i++, sfep = xfs_dir2_sf_nextentry(sfp, sfep)) {
- if (sfep->namelen == args->namelen &&
- sfep->name[0] == args->name[0] &&
- memcmp(args->name, sfep->name, args->namelen) == 0) {
- args->inumber =
- xfs_dir2_sf_get_inumber(sfp,
- xfs_dir2_sf_inumberp(sfep));
- return XFS_ERROR(EEXIST);
+ for (i = 0, sfep = xfs_dir2_sf_firstentry(sfp); i < sfp->hdr.count;
+ i++, sfep = xfs_dir2_sf_nextentry(sfp, sfep)) {
+ /*
+ * Compare name and if it's an exact match, return the inode
+ * number. If it's the first case-insensitive match, store the
+ * inode number and continue looking for an exact match.
+ */
+ cmp = dp->i_mount->m_dirnameops->compname(args, sfep->name,
+ sfep->namelen);
+ if (cmp != XFS_CMP_DIFFERENT && cmp != args->cmpresult) {
+ args->cmpresult = cmp;
+ args->inumber = xfs_dir2_sf_get_inumber(sfp,
+ xfs_dir2_sf_inumberp(sfep));
+ if (cmp == XFS_CMP_EXACT)
+ return XFS_ERROR(EEXIST);
}
}
+ ASSERT(args->oknoent);
+ /*
+ * Here, we can only be doing a lookup (not a rename or replace).
+ * If a case-insensitive match was found earlier, return "found".
+ */
+ if (args->cmpresult == XFS_CMP_CASE)
+ return XFS_ERROR(EEXIST);
/*
* Didn't find it.
*/
- ASSERT(args->oknoent);
return XFS_ERROR(ENOENT);
}
@@ -904,24 +918,21 @@ xfs_dir2_sf_removename(
* Loop over the old directory entries.
* Find the one we're deleting.
*/
- for (i = 0, sfep = xfs_dir2_sf_firstentry(sfp);
- i < sfp->hdr.count;
- i++, sfep = xfs_dir2_sf_nextentry(sfp, sfep)) {
- if (sfep->namelen == args->namelen &&
- sfep->name[0] == args->name[0] &&
- memcmp(sfep->name, args->name, args->namelen) == 0) {
+ for (i = 0, sfep = xfs_dir2_sf_firstentry(sfp); i < sfp->hdr.count;
+ i++, sfep = xfs_dir2_sf_nextentry(sfp, sfep)) {
+ if (xfs_da_compname(args, sfep->name, sfep->namelen) ==
+ XFS_CMP_EXACT) {
ASSERT(xfs_dir2_sf_get_inumber(sfp,
- xfs_dir2_sf_inumberp(sfep)) ==
- args->inumber);
+ xfs_dir2_sf_inumberp(sfep)) ==
+ args->inumber);
break;
}
}
/*
* Didn't find it.
*/
- if (i == sfp->hdr.count) {
+ if (i == sfp->hdr.count)
return XFS_ERROR(ENOENT);
- }
/*
* Calculate sizes.
*/
@@ -1042,11 +1053,10 @@ xfs_dir2_sf_replace(
*/
else {
for (i = 0, sfep = xfs_dir2_sf_firstentry(sfp);
- i < sfp->hdr.count;
- i++, sfep = xfs_dir2_sf_nextentry(sfp, sfep)) {
- if (sfep->namelen == args->namelen &&
- sfep->name[0] == args->name[0] &&
- memcmp(args->name, sfep->name, args->namelen) == 0) {
+ i < sfp->hdr.count;
+ i++, sfep = xfs_dir2_sf_nextentry(sfp, sfep)) {
+ if (xfs_da_compname(args, sfep->name, sfep->namelen) ==
+ XFS_CMP_EXACT) {
#if XFS_BIG_INUMS || defined(DEBUG)
ino = xfs_dir2_sf_get_inumber(sfp,
xfs_dir2_sf_inumberp(sfep));
Index: kern_ci/fs/xfs/xfs_mount.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_mount.h
+++ kern_ci/fs/xfs/xfs_mount.h
@@ -61,6 +61,7 @@ struct xfs_bmap_free;
struct xfs_extdelta;
struct xfs_swapext;
struct xfs_mru_cache;
+struct xfs_nameops;
/*
* Prototypes and functions for the Data Migration subsystem.
@@ -313,6 +314,7 @@ typedef struct xfs_mount {
__uint8_t m_inode_quiesce;/* call quiesce on new inodes.
field governed by m_ilock */
__uint8_t m_sectbb_log; /* sectlog - BBSHIFT */
+ const struct xfs_nameops *m_dirnameops; /* vector of dir name ops */
int m_dirblksize; /* directory block sz--bytes */
int m_dirblkfsbs; /* directory block sz--fsbs */
xfs_dablk_t m_dirdatablk; /* blockno of dir data v2 */
--
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/4] XFS: add op_flags field and helpers to xfs_da_args
2008-05-13 7:57 [PATCH 0/4] XFS: Case-insensitive support - ASCII only Barry Naujok
2008-05-13 7:57 ` [PATCH 1/4] XFS: Name operation vector for hash and compare Barry Naujok
@ 2008-05-13 7:57 ` Barry Naujok
2008-05-13 8:34 ` Christoph Hellwig
2008-05-13 7:57 ` [PATCH 3/4] XFS: Return case-insensitive match for dentry cache Barry Naujok
2008-05-13 7:57 ` [PATCH 4/4] XFS: ASCII case-insensitive support Barry Naujok
3 siblings, 1 reply; 22+ messages in thread
From: Barry Naujok @ 2008-05-13 7:57 UTC (permalink / raw)
To: xfs; +Cc: linux-fsdevel
[-- Attachment #1: additional_da_args_flag.patch --]
[-- Type: text/plain, Size: 21529 bytes --]
The end of the xfs_da_args structure has 4 unsigned char fields for
true/false information on directory and attr operations using the
xfs_da_args structure.
The following converts these 4 into a op_flags field that uses the
first 4 bits for these fields and allows expansion for future
operation information (eg. case-insensitive lookup request).
There is also a bit of EOL whitespace cleanup too.
Signed-off-by: Barry Naujok <bnaujok@sgi.com>
---
fs/xfs/xfs_attr.c | 11 ++++------
fs/xfs/xfs_attr_leaf.c | 52 ++++++++++++++++++++++++------------------------
fs/xfs/xfs_da_btree.c | 2 -
fs/xfs/xfs_da_btree.h | 33 ++++++++++++++++++++++++++----
fs/xfs/xfs_dir2.c | 12 +++++------
fs/xfs/xfs_dir2_block.c | 10 ++++-----
fs/xfs/xfs_dir2_leaf.c | 6 ++---
fs/xfs/xfs_dir2_node.c | 15 +++++++------
fs/xfs/xfs_dir2_sf.c | 8 +++----
fs/xfs/xfs_dir2_trace.c | 19 +++++++++--------
fs/xfs/xfsidbg.c | 13 ++++++------
11 files changed, 105 insertions(+), 76 deletions(-)
Index: kern_ci/fs/xfs/xfs_attr.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_attr.c
+++ kern_ci/fs/xfs/xfs_attr.c
@@ -241,8 +241,7 @@ xfs_attr_set_int(xfs_inode_t *dp, struct
args.firstblock = &firstblock;
args.flist = &flist;
args.whichfork = XFS_ATTR_FORK;
- args.addname = 1;
- args.oknoent = 1;
+ args.op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
/*
* Determine space new attribute will use, and if it would be
@@ -974,7 +973,7 @@ xfs_attr_leaf_addname(xfs_da_args_t *arg
xfs_da_brelse(args->trans, bp);
return(retval);
}
- args->rename = 1; /* an atomic rename */
+ args->op_flags |= XFS_DA_OP_RENAME; /* an atomic rename */
args->blkno2 = args->blkno; /* set 2nd entry info*/
args->index2 = args->index;
args->rmtblkno2 = args->rmtblkno;
@@ -1054,7 +1053,7 @@ xfs_attr_leaf_addname(xfs_da_args_t *arg
* so that one disappears and one appears atomically. Then we
* must remove the "old" attribute/value pair.
*/
- if (args->rename) {
+ if (xfs_da_isrename_op(args)) {
/*
* In a separate transaction, set the incomplete flag on the
* "old" attr and clear the incomplete flag on the "new" attr.
@@ -1307,7 +1306,7 @@ restart:
} else if (retval == EEXIST) {
if (args->flags & ATTR_CREATE)
goto out;
- args->rename = 1; /* atomic rename op */
+ args->op_flags |= XFS_DA_OP_RENAME; /* atomic rename op */
args->blkno2 = args->blkno; /* set 2nd entry info*/
args->index2 = args->index;
args->rmtblkno2 = args->rmtblkno;
@@ -1425,7 +1424,7 @@ restart:
* so that one disappears and one appears atomically. Then we
* must remove the "old" attribute/value pair.
*/
- if (args->rename) {
+ if (xfs_da_isrename_op(args)) {
/*
* In a separate transaction, set the incomplete flag on the
* "old" attr and clear the incomplete flag on the "new" attr.
Index: kern_ci/fs/xfs/xfs_attr_leaf.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_attr_leaf.c
+++ kern_ci/fs/xfs/xfs_attr_leaf.c
@@ -150,7 +150,7 @@ xfs_attr_shortform_bytesfit(xfs_inode_t
int offset;
int minforkoff; /* lower limit on valid forkoff locations */
int maxforkoff; /* upper limit on valid forkoff locations */
- int dsize;
+ int dsize;
xfs_mount_t *mp = dp->i_mount;
offset = (XFS_LITINO(mp) - bytes) >> 3; /* rounded down */
@@ -171,39 +171,39 @@ xfs_attr_shortform_bytesfit(xfs_inode_t
}
dsize = dp->i_df.if_bytes;
-
+
switch (dp->i_d.di_format) {
case XFS_DINODE_FMT_EXTENTS:
- /*
- * If there is no attr fork and the data fork is extents,
- * determine if creating the default attr fork will result
- * in the extents form migrating to btree. If so, the
- * minimum offset only needs to be the space required for
+ /*
+ * If there is no attr fork and the data fork is extents,
+ * determine if creating the default attr fork will result
+ * in the extents form migrating to btree. If so, the
+ * minimum offset only needs to be the space required for
* the btree root.
- */
+ */
if (!dp->i_d.di_forkoff && dp->i_df.if_bytes > mp->m_attroffset)
dsize = XFS_BMDR_SPACE_CALC(MINDBTPTRS);
break;
-
+
case XFS_DINODE_FMT_BTREE:
/*
* If have data btree then keep forkoff if we have one,
- * otherwise we are adding a new attr, so then we set
- * minforkoff to where the btree root can finish so we have
+ * otherwise we are adding a new attr, so then we set
+ * minforkoff to where the btree root can finish so we have
* plenty of room for attrs
*/
if (dp->i_d.di_forkoff) {
- if (offset < dp->i_d.di_forkoff)
+ if (offset < dp->i_d.di_forkoff)
return 0;
- else
+ else
return dp->i_d.di_forkoff;
} else
dsize = XFS_BMAP_BROOT_SPACE(dp->i_df.if_broot);
break;
}
-
- /*
- * A data fork btree root must have space for at least
+
+ /*
+ * A data fork btree root must have space for at least
* MINDBTPTRS key/ptr pairs if the data fork is small or empty.
*/
minforkoff = MAX(dsize, XFS_BMDR_SPACE_CALC(MINDBTPTRS));
@@ -369,9 +369,10 @@ xfs_attr_shortform_remove(xfs_da_args_t
* Fix up the start offset of the attribute fork
*/
totsize -= size;
- if (totsize == sizeof(xfs_attr_sf_hdr_t) && !args->addname &&
- (mp->m_flags & XFS_MOUNT_ATTR2) &&
- (dp->i_d.di_format != XFS_DINODE_FMT_BTREE)) {
+ if (totsize == sizeof(xfs_attr_sf_hdr_t) &&
+ !xfs_da_isaddname_op(args) &&
+ (mp->m_flags & XFS_MOUNT_ATTR2) &&
+ (dp->i_d.di_format != XFS_DINODE_FMT_BTREE)) {
/*
* Last attribute now removed, revert to original
* inode format making all literal area available
@@ -389,9 +390,10 @@ xfs_attr_shortform_remove(xfs_da_args_t
xfs_idata_realloc(dp, -size, XFS_ATTR_FORK);
dp->i_d.di_forkoff = xfs_attr_shortform_bytesfit(dp, totsize);
ASSERT(dp->i_d.di_forkoff);
- ASSERT(totsize > sizeof(xfs_attr_sf_hdr_t) || args->addname ||
- !(mp->m_flags & XFS_MOUNT_ATTR2) ||
- dp->i_d.di_format == XFS_DINODE_FMT_BTREE);
+ ASSERT(totsize > sizeof(xfs_attr_sf_hdr_t) ||
+ xfs_da_isaddname_op(args) ||
+ !(mp->m_flags & XFS_MOUNT_ATTR2) ||
+ dp->i_d.di_format == XFS_DINODE_FMT_BTREE);
dp->i_afp->if_ext_max =
XFS_IFORK_ASIZE(dp) / (uint)sizeof(xfs_bmbt_rec_t);
dp->i_df.if_ext_max =
@@ -531,7 +533,7 @@ xfs_attr_shortform_to_leaf(xfs_da_args_t
nargs.total = args->total;
nargs.whichfork = XFS_ATTR_FORK;
nargs.trans = args->trans;
- nargs.oknoent = 1;
+ nargs.op_flags = XFS_DA_OP_OKNOENT;
sfe = &sf->list[0];
for (i = 0; i < sf->hdr.count; i++) {
@@ -853,7 +855,7 @@ xfs_attr_leaf_to_shortform(xfs_dabuf_t *
nargs.total = args->total;
nargs.whichfork = XFS_ATTR_FORK;
nargs.trans = args->trans;
- nargs.oknoent = 1;
+ nargs.op_flags = XFS_DA_OP_OKNOENT;
entry = &leaf->entries[0];
for (i = 0; i < be16_to_cpu(leaf->hdr.count); entry++, i++) {
if (entry->flags & XFS_ATTR_INCOMPLETE)
@@ -1155,7 +1157,7 @@ xfs_attr_leaf_add_work(xfs_dabuf_t *bp,
entry->hashval = cpu_to_be32(args->hashval);
entry->flags = tmp ? XFS_ATTR_LOCAL : 0;
entry->flags |= XFS_ATTR_NSP_ARGS_TO_ONDISK(args->flags);
- if (args->rename) {
+ if (xfs_da_isrename_op(args)) {
entry->flags |= XFS_ATTR_INCOMPLETE;
if ((args->blkno2 == args->blkno) &&
(args->index2 <= args->index)) {
Index: kern_ci/fs/xfs/xfs_da_btree.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_da_btree.c
+++ kern_ci/fs/xfs/xfs_da_btree.c
@@ -1431,7 +1431,7 @@ xfs_da_path_shift(xfs_da_state_t *state,
}
if (level < 0) {
*result = XFS_ERROR(ENOENT); /* we're out of our tree */
- ASSERT(args->oknoent);
+ ASSERT(xfs_da_isoknoent_op(args));
return(0);
}
Index: kern_ci/fs/xfs/xfs_da_btree.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_da_btree.h
+++ kern_ci/fs/xfs/xfs_da_btree.h
@@ -132,14 +132,39 @@ typedef struct xfs_da_args {
int index2; /* index of 2nd attr in blk */
xfs_dablk_t rmtblkno2; /* remote attr value starting blkno */
int rmtblkcnt2; /* remote attr value block count */
- unsigned char justcheck; /* T/F: check for ok with no space */
- unsigned char rename; /* T/F: this is an atomic rename op */
- unsigned char addname; /* T/F: this is an add operation */
- unsigned char oknoent; /* T/F: ok to return ENOENT, else die */
+ int op_flags; /* operation flags */
enum xfs_dacmp cmpresult; /* name compare result for lookups */
} xfs_da_args_t;
/*
+ * Operation flags:
+ */
+#define XFS_DA_OP_JUSTCHECK 0x0001 /* check for ok with no space */
+#define XFS_DA_OP_RENAME 0x0002 /* this is an atomic rename op */
+#define XFS_DA_OP_ADDNAME 0x0004 /* this is an add operation */
+#define XFS_DA_OP_OKNOENT 0x0008 /* lookup/add op, ENOENT ok, else die */
+
+static inline int xfs_da_isjustcheck_op(struct xfs_da_args *args)
+{
+ return args->op_flags & XFS_DA_OP_JUSTCHECK;
+}
+
+static inline int xfs_da_isrename_op(struct xfs_da_args *args)
+{
+ return args->op_flags & XFS_DA_OP_RENAME;
+}
+
+static inline int xfs_da_isaddname_op(struct xfs_da_args *args)
+{
+ return args->op_flags & XFS_DA_OP_ADDNAME;
+}
+
+static inline int xfs_da_isoknoent_op(struct xfs_da_args *args)
+{
+ return args->op_flags & XFS_DA_OP_OKNOENT;
+}
+
+/*
* Structure to describe buffer(s) for a block.
* This is needed in the directory version 2 format case, when
* multiple non-contiguous fsblocks might be needed to cover one
Index: kern_ci/fs/xfs/xfs_dir2.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2.c
+++ kern_ci/fs/xfs/xfs_dir2.c
@@ -175,8 +175,7 @@ xfs_dir_createname(
args.total = total;
args.whichfork = XFS_DATA_FORK;
args.trans = tp;
- args.justcheck = 0;
- args.addname = args.oknoent = 1;
+ args.op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
rval = xfs_dir2_sf_addname(&args);
@@ -217,7 +216,7 @@ xfs_dir_lookup(
args.dp = dp;
args.whichfork = XFS_DATA_FORK;
args.trans = tp;
- args.oknoent = 1;
+ args.op_flags = XFS_DA_OP_OKNOENT;
args.cmpresult = XFS_CMP_DIFFERENT;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
@@ -269,7 +268,7 @@ xfs_dir_removename(
args.total = total;
args.whichfork = XFS_DATA_FORK;
args.trans = tp;
- args.justcheck = args.addname = args.oknoent = 0;
+ args.op_flags = 0;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
rval = xfs_dir2_sf_removename(&args);
@@ -352,7 +351,7 @@ xfs_dir_replace(
args.total = total;
args.whichfork = XFS_DATA_FORK;
args.trans = tp;
- args.justcheck = args.addname = args.oknoent = 0;
+ args.op_flags = 0;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
rval = xfs_dir2_sf_replace(&args);
@@ -396,7 +395,8 @@ xfs_dir_canenter(
args.dp = dp;
args.whichfork = XFS_DATA_FORK;
args.trans = tp;
- args.justcheck = args.addname = args.oknoent = 1;
+ args.op_flags = XFS_DA_OP_JUSTCHECK | XFS_DA_OP_ADDNAME |
+ XFS_DA_OP_OKNOENT;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
rval = xfs_dir2_sf_addname(&args);
Index: kern_ci/fs/xfs/xfs_dir2_block.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_block.c
+++ kern_ci/fs/xfs/xfs_dir2_block.c
@@ -215,7 +215,7 @@ xfs_dir2_block_addname(
/*
* If this isn't a real add, we're done with the buffer.
*/
- if (args->justcheck)
+ if (xfs_da_isjustcheck_op(args))
xfs_da_brelse(tp, bp);
/*
* If we don't have space for the new entry & leaf ...
@@ -225,7 +225,7 @@ xfs_dir2_block_addname(
* Not trying to actually do anything, or don't have
* a space reservation: return no-space.
*/
- if (args->justcheck || args->total == 0)
+ if (xfs_da_isjustcheck_op(args) || args->total == 0)
return XFS_ERROR(ENOSPC);
/*
* Convert to the next larger format.
@@ -240,7 +240,7 @@ xfs_dir2_block_addname(
/*
* Just checking, and it would work, so say so.
*/
- if (args->justcheck)
+ if (xfs_da_isjustcheck_op(args))
return 0;
needlog = needscan = 0;
/*
@@ -674,7 +674,7 @@ xfs_dir2_block_lookup_int(
else
high = mid - 1;
if (low > high) {
- ASSERT(args->oknoent);
+ ASSERT(xfs_da_isoknoent_op(args));
xfs_da_brelse(tp, bp);
return XFS_ERROR(ENOENT);
}
@@ -713,7 +713,7 @@ xfs_dir2_block_lookup_int(
} while (++mid < be32_to_cpu(btp->count) &&
be32_to_cpu(blp[mid].hashval) == hash);
- ASSERT(args->oknoent);
+ ASSERT(xfs_da_isoknoent_op(args));
/*
* Here, we can only be doing a lookup (not a rename or replace).
* If a case-insensitive match was found earlier, return success.
Index: kern_ci/fs/xfs/xfs_dir2_leaf.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_leaf.c
+++ kern_ci/fs/xfs/xfs_dir2_leaf.c
@@ -276,7 +276,7 @@ xfs_dir2_leaf_addname(
/*
* Just checking or no space reservation, give up.
*/
- if (args->justcheck || args->total == 0) {
+ if (xfs_da_isjustcheck_op(args) || args->total == 0) {
xfs_da_brelse(tp, lbp);
return XFS_ERROR(ENOSPC);
}
@@ -301,7 +301,7 @@ xfs_dir2_leaf_addname(
* If just checking, then it will fit unless we needed to allocate
* a new data block.
*/
- if (args->justcheck) {
+ if (xfs_da_isjustcheck_op(args)) {
xfs_da_brelse(tp, lbp);
return use_block == -1 ? XFS_ERROR(ENOSPC) : 0;
}
@@ -1414,7 +1414,7 @@ xfs_dir2_leaf_lookup_int(
cbp = dbp;
}
}
- ASSERT(args->oknoent);
+ ASSERT(xfs_da_isoknoent_op(args));
/*
* Here, we can only be doing a lookup (not a rename or replace).
* If a case-insensitive match was found earlier, release the current
Index: kern_ci/fs/xfs/xfs_dir2_node.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_node.c
+++ kern_ci/fs/xfs/xfs_dir2_node.c
@@ -226,7 +226,7 @@ xfs_dir2_leafn_add(
ASSERT(index == be16_to_cpu(leaf->hdr.count) ||
be32_to_cpu(leaf->ents[index].hashval) >= args->hashval);
- if (args->justcheck)
+ if (xfs_da_isjustcheck_op(args))
return 0;
/*
@@ -515,7 +515,7 @@ xfs_dir2_leafn_lookup_for_addname(
/* Didn't find any space */
fi = -1;
out:
- ASSERT(args->oknoent);
+ ASSERT(xfs_da_isoknoent_op(args));
if (curbp) {
/* Giving back a free block. */
state->extravalid = 1;
@@ -638,7 +638,8 @@ xfs_dir2_leafn_lookup_for_entry(
/* Didn't find an exact match. */
error = ENOENT;
di = -1;
- ASSERT(index == be16_to_cpu(leaf->hdr.count) || args->oknoent);
+ ASSERT(index == be16_to_cpu(leaf->hdr.count) ||
+ xfs_da_isoknoent_op(args));
out:
if (curbp) {
/* Giving back a data block. */
@@ -669,7 +670,7 @@ xfs_dir2_leafn_lookup_int(
int *indexp, /* out: leaf entry index */
xfs_da_state_t *state) /* state to fill in */
{
- if (args->addname)
+ if (xfs_da_isaddname_op(args))
return xfs_dir2_leafn_lookup_for_addname(bp, args, indexp,
state);
return xfs_dir2_leafn_lookup_for_entry(bp, args, indexp, state);
@@ -1383,7 +1384,7 @@ xfs_dir2_node_addname(
/*
* It worked, fix the hash values up the btree.
*/
- if (!args->justcheck)
+ if (!xfs_da_isjustcheck_op(args))
xfs_da_fixhashpath(state, &state->path);
} else {
/*
@@ -1566,7 +1567,7 @@ xfs_dir2_node_addname_int(
/*
* Not allowed to allocate, return failure.
*/
- if (args->justcheck || args->total == 0) {
+ if (xfs_da_isjustcheck_op(args) || args->total == 0) {
/*
* Drop the freespace buffer unless it came from our
* caller.
@@ -1712,7 +1713,7 @@ xfs_dir2_node_addname_int(
/*
* If just checking, we succeeded.
*/
- if (args->justcheck) {
+ if (xfs_da_isjustcheck_op(args)) {
if ((fblk == NULL || fblk->bp == NULL) && fbp != NULL)
xfs_da_buf_done(fbp);
return 0;
Index: kern_ci/fs/xfs/xfs_dir2_sf.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_sf.c
+++ kern_ci/fs/xfs/xfs_dir2_sf.c
@@ -332,7 +332,7 @@ xfs_dir2_sf_addname(
/*
* Just checking or no space reservation, it doesn't fit.
*/
- if (args->justcheck || args->total == 0)
+ if (xfs_da_isjustcheck_op(args) || args->total == 0)
return XFS_ERROR(ENOSPC);
/*
* Convert to block form then add the name.
@@ -345,7 +345,7 @@ xfs_dir2_sf_addname(
/*
* Just checking, it fits.
*/
- if (args->justcheck)
+ if (xfs_da_isjustcheck_op(args))
return 0;
/*
* Do it the easy way - just add it at the end.
@@ -869,7 +869,7 @@ xfs_dir2_sf_lookup(
return XFS_ERROR(EEXIST);
}
}
- ASSERT(args->oknoent);
+ ASSERT(xfs_da_isoknoent_op(args));
/*
* Here, we can only be doing a lookup (not a rename or replace).
* If a case-insensitive match was found earlier, return "found".
@@ -1071,7 +1071,7 @@ xfs_dir2_sf_replace(
* Didn't find it.
*/
if (i == sfp->hdr.count) {
- ASSERT(args->oknoent);
+ ASSERT(xfs_da_isoknoent_op(args));
#if XFS_BIG_INUMS
if (i8elevated)
xfs_dir2_sf_toino4(args);
Index: kern_ci/fs/xfs/xfs_dir2_trace.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_trace.c
+++ kern_ci/fs/xfs/xfs_dir2_trace.c
@@ -85,7 +85,7 @@ xfs_dir2_trace_args(
(void *)((unsigned long)(args->inumber >> 32)),
(void *)((unsigned long)(args->inumber & 0xFFFFFFFF)),
(void *)args->dp, (void *)args->trans,
- (void *)(unsigned long)args->justcheck, NULL, NULL);
+ (void *)(unsigned long)xfs_da_isjustcheck_op(args), NULL, NULL);
}
void
@@ -100,7 +100,7 @@ xfs_dir2_trace_args_b(
(void *)((unsigned long)(args->inumber >> 32)),
(void *)((unsigned long)(args->inumber & 0xFFFFFFFF)),
(void *)args->dp, (void *)args->trans,
- (void *)(unsigned long)args->justcheck,
+ (void *)(unsigned long)xfs_da_isjustcheck_op(args),
(void *)(bp ? bp->bps[0] : NULL), NULL);
}
@@ -117,7 +117,7 @@ xfs_dir2_trace_args_bb(
(void *)((unsigned long)(args->inumber >> 32)),
(void *)((unsigned long)(args->inumber & 0xFFFFFFFF)),
(void *)args->dp, (void *)args->trans,
- (void *)(unsigned long)args->justcheck,
+ (void *)(unsigned long)xfs_da_isjustcheck_op(args),
(void *)(lbp ? lbp->bps[0] : NULL),
(void *)(dbp ? dbp->bps[0] : NULL));
}
@@ -157,8 +157,8 @@ xfs_dir2_trace_args_db(
(void *)((unsigned long)(args->inumber >> 32)),
(void *)((unsigned long)(args->inumber & 0xFFFFFFFF)),
(void *)args->dp, (void *)args->trans,
- (void *)(unsigned long)args->justcheck, (void *)(long)db,
- (void *)dbp);
+ (void *)(unsigned long)xfs_da_isjustcheck_op(args),
+ (void *)(long)db, (void *)dbp);
}
void
@@ -173,7 +173,7 @@ xfs_dir2_trace_args_i(
(void *)((unsigned long)(args->inumber >> 32)),
(void *)((unsigned long)(args->inumber & 0xFFFFFFFF)),
(void *)args->dp, (void *)args->trans,
- (void *)(unsigned long)args->justcheck,
+ (void *)(unsigned long)xfs_da_isjustcheck_op(args),
(void *)((unsigned long)(i >> 32)),
(void *)((unsigned long)(i & 0xFFFFFFFF)));
}
@@ -190,7 +190,8 @@ xfs_dir2_trace_args_s(
(void *)((unsigned long)(args->inumber >> 32)),
(void *)((unsigned long)(args->inumber & 0xFFFFFFFF)),
(void *)args->dp, (void *)args->trans,
- (void *)(unsigned long)args->justcheck, (void *)(long)s, NULL);
+ (void *)(unsigned long)xfs_da_isjustcheck_op(args),
+ (void *)(long)s, NULL);
}
void
@@ -208,7 +209,7 @@ xfs_dir2_trace_args_sb(
(void *)((unsigned long)(args->inumber >> 32)),
(void *)((unsigned long)(args->inumber & 0xFFFFFFFF)),
(void *)args->dp, (void *)args->trans,
- (void *)(unsigned long)args->justcheck, (void *)(long)s,
- (void *)dbp);
+ (void *)(unsigned long)xfs_da_isjustcheck_op(args),
+ (void *)(long)s, (void *)dbp);
}
#endif /* XFS_DIR2_TRACE */
Index: kern_ci/fs/xfs/xfsidbg.c
===================================================================
--- kern_ci.orig/fs/xfs/xfsidbg.c
+++ kern_ci/fs/xfs/xfsidbg.c
@@ -308,7 +308,7 @@ static int kdbm_xfs_xalatrace(
int nextarg = 1;
long offset = 0;
int diag;
-
+
if (argc != 1)
return KDB_ARGCOUNT;
@@ -5237,7 +5237,8 @@ xfsidbg_xdaargs(xfs_da_args_t *n)
kdb_printf("0x%x", n->flags & i);
kdb_printf(">\n");
kdb_printf(" rename %d justcheck %d addname %d oknoent %d\n",
- n->rename, n->justcheck, n->addname, n->oknoent);
+ xfs_da_isrename_op(n) != 0, xfs_da_isjustcheck_op(n) != 0,
+ xfs_da_isaddname_op(n) != 0, xfs_da_isoknoent_op(n) != 0);
kdb_printf(" leaf: blkno %d index %d rmtblkno %d rmtblkcnt %d\n",
n->blkno, n->index, n->rmtblkno, n->rmtblkcnt);
kdb_printf(" leaf2: blkno %d index %d rmtblkno %d rmtblkcnt %d\n",
@@ -5955,7 +5956,7 @@ xfsidbg_xlog_granttrace(xlog_t *log)
xfsidbg_print_trans_type((unsigned long)ktep->val[12]);
qprintf("]\n");
qprintf(" t_ocnt = %lu, t_cnt = %lu, t_curr_res = %lu, "
- "t_unit_res = %lu\n",
+ "t_unit_res = %lu\n",
t_ocnt, t_cnt, (unsigned long)ktep->val[14],
(unsigned long)ktep->val[15]);
qprintf(" tic:0x%p resQ:0x%p wrQ:0x%p ",
@@ -6449,16 +6450,16 @@ xfsidbg_xnode(xfs_inode_t *ip)
#endif
#ifdef XFS_BMBT_TRACE
qprintf(" bmbt trace 0x%p\n", ip->i_btrace);
-#endif
+#endif
#ifdef XFS_RW_TRACE
qprintf(" rw trace 0x%p\n", ip->i_rwtrace);
-#endif
+#endif
#ifdef XFS_ILOCK_TRACE
qprintf(" ilock trace 0x%p\n", ip->i_lock_trace);
#endif
#ifdef XFS_DIR2_TRACE
qprintf(" dir trace 0x%p\n", ip->i_dir_trace);
-#endif
+#endif
kdb_printf("\n");
xfs_xnode_fork("data", &ip->i_df);
xfs_xnode_fork("attr", ip->i_afp);
--
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-13 7:57 [PATCH 0/4] XFS: Case-insensitive support - ASCII only Barry Naujok
2008-05-13 7:57 ` [PATCH 1/4] XFS: Name operation vector for hash and compare Barry Naujok
2008-05-13 7:57 ` [PATCH 2/4] XFS: add op_flags field and helpers to xfs_da_args Barry Naujok
@ 2008-05-13 7:57 ` Barry Naujok
2008-05-13 8:57 ` Christoph Hellwig
2008-05-13 7:57 ` [PATCH 4/4] XFS: ASCII case-insensitive support Barry Naujok
3 siblings, 1 reply; 22+ messages in thread
From: Barry Naujok @ 2008-05-13 7:57 UTC (permalink / raw)
To: xfs; +Cc: linux-fsdevel
[-- Attachment #1: return_name.patch --]
[-- Type: text/plain, Size: 19881 bytes --]
This implements the code to store the actual filename found
during a lookup in the dentry cache and to avoid multiple entries
in the dcache pointing to the same inode.
To avoid polluting the dcache, we implement a new directory inode
operations for lookup. xfs_vn_ci_lookup() interacts directly with
the dcache and the code was derived from ntfs_lookup() in
fs/ntfs/namei.c.
The "actual name" is only allocated and returned for a case-
insensitive match and not an actual match.
Another unusual interaction with the dcache is not storing
negative dentries like other filesystems doing a d_add(dentry, NULL)
when an ENOENT is returned. During the VFS lookup, if a dentry
returned has no inode, dput is called and ENOENT is returned.
By not doing a d_add, this actually removes it completely from
the dcache to be reused. create/rename have to be modified to
support unhashed dentries being passed in.
Signed-off-by: Barry Naujok <bnaujok@sgi.com>
---
fs/dcache.c | 104 ++++++++++++++++++++++++++++++++++++++++++
fs/xfs/linux-2.6/xfs_export.c | 2
fs/xfs/linux-2.6/xfs_iops.c | 61 ++++++++++++++++++++++++
fs/xfs/linux-2.6/xfs_iops.h | 1
fs/xfs/xfs_da_btree.h | 6 ++
fs/xfs/xfs_dir2.c | 40 +++++++++++++++-
fs/xfs/xfs_dir2.h | 6 ++
fs/xfs/xfs_dir2_block.c | 9 ++-
fs/xfs/xfs_dir2_leaf.c | 5 +-
fs/xfs/xfs_dir2_node.c | 16 ++++--
fs/xfs/xfs_dir2_sf.c | 12 ++--
fs/xfs/xfs_vnodeops.c | 18 +++++--
fs/xfs/xfs_vnodeops.h | 2
include/linux/dcache.h | 1
14 files changed, 256 insertions(+), 27 deletions(-)
Index: kern_ci/fs/dcache.c
===================================================================
--- kern_ci.orig/fs/dcache.c
+++ kern_ci/fs/dcache.c
@@ -1191,6 +1191,109 @@ struct dentry *d_splice_alias(struct ino
return new;
}
+/**
+ * d_add_ci - lookup or allocate new dentry with case-exact name
+ * @inode: the inode case-insensitive lookup has found
+ * @dentry: the negative dentry that was passed to the parent's lookup func
+ * @name: the case-exact name to be associated with the returned dentry
+ *
+ * This is to avoid filling the dcache with case-insensitive names to the
+ * same inode, only the actual correct case is stored in the dcache for
+ * case-insensitive filesystems.
+ *
+ * For a case-insensitive lookup match and if the the case-exact dentry
+ * already exists in in the dcache, use it and return it.
+ *
+ * If no entry exists with the exact case name, allocate new dentry with
+ * the exact case, and return the spliced entry.
+ */
+
+struct dentry *d_add_ci(struct inode *inode, struct dentry *dentry,
+ struct qstr *name)
+{
+ int error;
+ struct dentry *found;
+ struct dentry *new;
+
+ /* Does a dentry matching the name exist already? */
+ found = d_hash_and_lookup(dentry->d_parent, name);
+ /* If not, create it now and return */
+ if (!found) {
+ new = d_alloc(dentry->d_parent, name);
+ if (!new) {
+ error = -ENOMEM;
+ goto err_out;
+ }
+ found = d_splice_alias(inode, new);
+ if (found) {
+ dput(new);
+ return found;
+ }
+ return new;
+ }
+ /* Matching dentry exists, check if it is negative. */
+ if (found->d_inode) {
+ if (unlikely(found->d_inode != inode)) {
+ /* This can't happen because bad inodes are unhashed. */
+ BUG_ON(!is_bad_inode(inode));
+ BUG_ON(!is_bad_inode(found->d_inode));
+ }
+ /*
+ * Already have the inode and the dentry attached, decrement
+ * the reference count to balance the iget() done
+ * earlier on. We found the dentry using d_lookup() so it
+ * cannot be disconnected and thus we do not need to worry
+ * about any NFS/disconnectedness issues here.
+ */
+ iput(inode);
+ return found;
+ }
+ /*
+ * Negative dentry: instantiate it unless the inode is a directory and
+ * has a 'disconnected' dentry (i.e. IS_ROOT and DCACHE_DISCONNECTED),
+ * in which case d_move() that in place of the found dentry.
+ */
+ if (!S_ISDIR(inode->i_mode)) {
+ /* Not a directory; everything is easy. */
+ d_instantiate(found, inode);
+ return found;
+ }
+ spin_lock(&dcache_lock);
+ if (list_empty(&inode->i_dentry)) {
+ /*
+ * Directory without a 'disconnected' dentry; we need to do
+ * d_instantiate() by hand because it takes dcache_lock which
+ * we already hold.
+ */
+ list_add(&found->d_alias, &inode->i_dentry);
+ found->d_inode = inode;
+ spin_unlock(&dcache_lock);
+ security_d_instantiate(found, inode);
+ return found;
+ }
+ /*
+ * Directory with a 'disconnected' dentry; get a reference to the
+ * 'disconnected' dentry.
+ */
+ new = list_entry(inode->i_dentry.next, struct dentry, d_alias);
+ dget_locked(new);
+ spin_unlock(&dcache_lock);
+ /* Do security vodoo. */
+ security_d_instantiate(found, inode);
+ /* Move new in place of found. */
+ d_move(new, found);
+ /* Balance the iget() we did above. */
+ iput(inode);
+ /* Throw away found. */
+ dput(found);
+ /* Use new as the actual dentry. */
+ return new;
+
+err_out:
+ iput(inode);
+ return ERR_PTR(error);
+}
+
/**
* d_lookup - search for a dentry
@@ -2178,6 +2281,7 @@ EXPORT_SYMBOL(d_path);
EXPORT_SYMBOL(d_prune_aliases);
EXPORT_SYMBOL(d_rehash);
EXPORT_SYMBOL(d_splice_alias);
+EXPORT_SYMBOL(d_add_ci);
EXPORT_SYMBOL(d_validate);
EXPORT_SYMBOL(dget_locked);
EXPORT_SYMBOL(dput);
Index: kern_ci/fs/xfs/linux-2.6/xfs_export.c
===================================================================
--- kern_ci.orig/fs/xfs/linux-2.6/xfs_export.c
+++ kern_ci/fs/xfs/linux-2.6/xfs_export.c
@@ -215,7 +215,7 @@ xfs_fs_get_parent(
struct xfs_inode *cip;
struct dentry *parent;
- error = xfs_lookup(XFS_I(child->d_inode), &xfs_name_dotdot, &cip);
+ error = xfs_lookup(XFS_I(child->d_inode), &xfs_name_dotdot, &cip, NULL);
if (unlikely(error))
return ERR_PTR(-error);
Index: kern_ci/fs/xfs/linux-2.6/xfs_iops.c
===================================================================
--- kern_ci.orig/fs/xfs/linux-2.6/xfs_iops.c
+++ kern_ci/fs/xfs/linux-2.6/xfs_iops.c
@@ -389,7 +389,7 @@ xfs_vn_lookup(
return ERR_PTR(-ENAMETOOLONG);
xfs_dentry_to_name(&name, dentry);
- error = xfs_lookup(XFS_I(dir), &name, &cip);
+ error = xfs_lookup(XFS_I(dir), &name, &cip, NULL);
if (unlikely(error)) {
if (unlikely(error != ENOENT))
return ERR_PTR(-error);
@@ -400,6 +400,46 @@ xfs_vn_lookup(
return d_splice_alias(cip->i_vnode, dentry);
}
+STATIC struct dentry *
+xfs_vn_ci_lookup(
+ struct inode *dir,
+ struct dentry *dentry,
+ struct nameidata *nd)
+{
+ struct xfs_inode *ip;
+ struct xfs_name xname;
+ struct qstr dname;
+ int ci_match = 0;
+ int error;
+
+ if (dentry->d_name.len >= MAXNAMELEN)
+ return ERR_PTR(-ENAMETOOLONG);
+
+ xfs_dentry_to_name(&xname, dentry);
+ error = xfs_lookup(XFS_I(dir), &xname, &ip, &ci_match);
+ if (unlikely(error)) {
+ if (unlikely(error != ENOENT))
+ return ERR_PTR(-error);
+ /*
+ * don't d_add dentry, __link_path_walk will dput the
+ * dentry if its inode is NULL which means the negative
+ * dentry will be destroyed rather than kept around.
+ */
+ return NULL;
+ }
+
+ /* if exact match, just splice and exit */
+ if (!ci_match)
+ return d_splice_alias(ip->i_vnode, dentry);
+
+ /* else case-insensitive match... */
+ dname.name = xname.name;
+ dname.len = xname.len;
+ dentry = d_add_ci(ip->i_vnode, dentry, &dname);
+ kmem_free(xname.name, xname.len);
+ return dentry;
+}
+
STATIC int
xfs_vn_link(
struct dentry *old_dentry,
@@ -911,6 +951,25 @@ const struct inode_operations xfs_dir_in
.removexattr = xfs_vn_removexattr,
};
+const struct inode_operations xfs_dir_ci_inode_operations = {
+ .create = xfs_vn_create,
+ .lookup = xfs_vn_ci_lookup,
+ .link = xfs_vn_link,
+ .unlink = xfs_vn_unlink,
+ .symlink = xfs_vn_symlink,
+ .mkdir = xfs_vn_mkdir,
+ .rmdir = xfs_vn_rmdir,
+ .mknod = xfs_vn_mknod,
+ .rename = xfs_vn_rename,
+ .permission = xfs_vn_permission,
+ .getattr = xfs_vn_getattr,
+ .setattr = xfs_vn_setattr,
+ .setxattr = xfs_vn_setxattr,
+ .getxattr = xfs_vn_getxattr,
+ .listxattr = xfs_vn_listxattr,
+ .removexattr = xfs_vn_removexattr,
+};
+
const struct inode_operations xfs_symlink_inode_operations = {
.readlink = generic_readlink,
.follow_link = xfs_vn_follow_link,
Index: kern_ci/fs/xfs/linux-2.6/xfs_iops.h
===================================================================
--- kern_ci.orig/fs/xfs/linux-2.6/xfs_iops.h
+++ kern_ci/fs/xfs/linux-2.6/xfs_iops.h
@@ -20,6 +20,7 @@
extern const struct inode_operations xfs_inode_operations;
extern const struct inode_operations xfs_dir_inode_operations;
+extern const struct inode_operations xfs_dir_ci_inode_operations;
extern const struct inode_operations xfs_symlink_inode_operations;
extern const struct file_operations xfs_file_operations;
Index: kern_ci/fs/xfs/xfs_da_btree.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_da_btree.h
+++ kern_ci/fs/xfs/xfs_da_btree.h
@@ -143,6 +143,7 @@ typedef struct xfs_da_args {
#define XFS_DA_OP_RENAME 0x0002 /* this is an atomic rename op */
#define XFS_DA_OP_ADDNAME 0x0004 /* this is an add operation */
#define XFS_DA_OP_OKNOENT 0x0008 /* lookup/add op, ENOENT ok, else die */
+#define XFS_DA_OP_CILOOKUP 0x0010 /* lookup to return CI name if found */
static inline int xfs_da_isjustcheck_op(struct xfs_da_args *args)
{
@@ -164,6 +165,11 @@ static inline int xfs_da_isoknoent_op(st
return args->op_flags & XFS_DA_OP_OKNOENT;
}
+static inline int xfs_da_iscilookup_op(struct xfs_da_args *args)
+{
+ return args->op_flags & XFS_DA_OP_CILOOKUP;
+}
+
/*
* Structure to describe buffer(s) for a block.
* This is needed in the directory version 2 format case, when
Index: kern_ci/fs/xfs/xfs_dir2.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2.c
+++ kern_ci/fs/xfs/xfs_dir2.c
@@ -193,14 +193,42 @@ xfs_dir_createname(
}
/*
+ * If doing a CI lookup and case-insensitive match, dup actual name into
+ * args.value. Return EEXIST for success (ie. name found) or an error.
+ */
+int
+xfs_dir_cilookup_result(
+ struct xfs_da_args *args,
+ const char *name,
+ int len)
+{
+ if (args->cmpresult == XFS_CMP_DIFFERENT)
+ return ENOENT;
+ if (args->cmpresult != XFS_CMP_CASE || !xfs_da_iscilookup_op(args))
+ return EEXIST;
+
+ args->value = kmem_alloc(len, KM_MAYFAIL);
+ if (!args->value)
+ return ENOMEM;
+
+ memcpy(args->value, name, len);
+ args->valuelen = len;
+ return EEXIST;
+}
+
+/*
* Lookup a name in a directory, give back the inode number.
+ * If ci_match is not NULL, sets whether a CI match occurred of not, and
+ * if so, return the actual name in name.
*/
+
int
xfs_dir_lookup(
xfs_trans_t *tp,
xfs_inode_t *dp,
struct xfs_name *name,
- xfs_ino_t *inum) /* out: inode number */
+ xfs_ino_t *inum, /* out: inode number */
+ int *ci_match) /* out: CI match occurred */
{
xfs_da_args_t args;
int rval;
@@ -217,6 +245,8 @@ xfs_dir_lookup(
args.whichfork = XFS_DATA_FORK;
args.trans = tp;
args.op_flags = XFS_DA_OP_OKNOENT;
+ if (ci_match)
+ args.op_flags |= XFS_DA_OP_CILOOKUP;
args.cmpresult = XFS_CMP_DIFFERENT;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
@@ -233,8 +263,14 @@ xfs_dir_lookup(
rval = xfs_dir2_node_lookup(&args);
if (rval == EEXIST)
rval = 0;
- if (rval == 0)
+ if (!rval) {
*inum = args.inumber;
+ if (ci_match) {
+ *ci_match = args.cmpresult == XFS_CMP_CASE;
+ name->name = args.value;
+ name->len = args.valuelen;
+ }
+ }
return rval;
}
Index: kern_ci/fs/xfs/xfs_dir2.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2.h
+++ kern_ci/fs/xfs/xfs_dir2.h
@@ -74,7 +74,8 @@ extern int xfs_dir_createname(struct xfs
xfs_fsblock_t *first,
struct xfs_bmap_free *flist, xfs_extlen_t tot);
extern int xfs_dir_lookup(struct xfs_trans *tp, struct xfs_inode *dp,
- struct xfs_name *name, xfs_ino_t *inum);
+ struct xfs_name *name, xfs_ino_t *inum,
+ int *ci_match);
extern int xfs_dir_removename(struct xfs_trans *tp, struct xfs_inode *dp,
struct xfs_name *name, xfs_ino_t ino,
xfs_fsblock_t *first,
@@ -99,4 +100,7 @@ extern int xfs_dir2_isleaf(struct xfs_tr
extern int xfs_dir2_shrink_inode(struct xfs_da_args *args, xfs_dir2_db_t db,
struct xfs_dabuf *bp);
+extern int xfs_dir_cilookup_result(struct xfs_da_args *args, const char *name,
+ int len);
+
#endif /* __XFS_DIR2_H__ */
Index: kern_ci/fs/xfs/xfs_dir2_block.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_block.c
+++ kern_ci/fs/xfs/xfs_dir2_block.c
@@ -610,14 +610,15 @@ xfs_dir2_block_lookup(
/*
* Get the offset from the leaf entry, to point to the data.
*/
- dep = (xfs_dir2_data_entry_t *)
- ((char *)block + xfs_dir2_dataptr_to_off(mp, be32_to_cpu(blp[ent].address)));
+ dep = (xfs_dir2_data_entry_t *)((char *)block +
+ xfs_dir2_dataptr_to_off(mp, be32_to_cpu(blp[ent].address)));
/*
- * Fill in inode number, release the block.
+ * Fill in inode number, CI name if appropriate, release the block.
*/
args->inumber = be64_to_cpu(dep->inumber);
+ error = xfs_dir_cilookup_result(args, dep->name, dep->namelen);
xfs_da_brelse(args->trans, bp);
- return XFS_ERROR(EEXIST);
+ return XFS_ERROR(error);
}
/*
Index: kern_ci/fs/xfs/xfs_dir2_leaf.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_leaf.c
+++ kern_ci/fs/xfs/xfs_dir2_leaf.c
@@ -1298,12 +1298,13 @@ xfs_dir2_leaf_lookup(
((char *)dbp->data +
xfs_dir2_dataptr_to_off(dp->i_mount, be32_to_cpu(lep->address)));
/*
- * Return the found inode number.
+ * Return the found inode number & CI name if appropriate
*/
args->inumber = be64_to_cpu(dep->inumber);
+ error = xfs_dir_cilookup_result(args, dep->name, dep->namelen);
xfs_da_brelse(tp, dbp);
xfs_da_brelse(tp, lbp);
- return XFS_ERROR(EEXIST);
+ return XFS_ERROR(error);
}
/*
Index: kern_ci/fs/xfs/xfs_dir2_node.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_node.c
+++ kern_ci/fs/xfs/xfs_dir2_node.c
@@ -549,7 +549,7 @@ xfs_dir2_leafn_lookup_for_entry(
xfs_dir2_data_entry_t *dep; /* data block entry */
xfs_inode_t *dp; /* incore directory inode */
int error; /* error return value */
- int di; /* data entry index */
+ int di = -1; /* data entry index */
int index; /* leaf entry index */
xfs_dir2_leaf_t *leaf; /* leaf structure */
xfs_dir2_leaf_entry_t *lep; /* leaf entry */
@@ -577,6 +577,7 @@ xfs_dir2_leafn_lookup_for_entry(
if (state->extravalid) {
curbp = state->extrablk.bp;
curdb = state->extrablk.blkno;
+ di = state->extrablk.index;
}
/*
* Loop over leaf entries with the right hash value.
@@ -637,7 +638,6 @@ xfs_dir2_leafn_lookup_for_entry(
}
/* Didn't find an exact match. */
error = ENOENT;
- di = -1;
ASSERT(index == be16_to_cpu(leaf->hdr.count) ||
xfs_da_isoknoent_op(args));
out:
@@ -652,7 +652,7 @@ out:
state->extravalid = 0;
}
/*
- * Return the index, that will be the insertion point.
+ * Return the index, that will be the deletion point for remove/replace.
*/
*indexp = index;
return XFS_ERROR(error);
@@ -1819,8 +1819,14 @@ xfs_dir2_node_lookup(
error = xfs_da_node_lookup_int(state, &rval);
if (error)
rval = error;
- else if (rval == ENOENT && args->cmpresult == XFS_CMP_CASE)
- rval = EEXIST; /* a case-insensitive match was found */
+ else if (rval == ENOENT && args->cmpresult == XFS_CMP_CASE) {
+ /* If a CI match, dup the actual name and return EEXIST */
+ xfs_dir2_data_entry_t *dep;
+
+ dep = (xfs_dir2_data_entry_t *)((char *)state->extrablk.bp->
+ data + state->extrablk.index);
+ rval = xfs_dir_cilookup_result(args, dep->name, dep->namelen);
+ }
/*
* Release the btree blocks and leaf block.
*/
Index: kern_ci/fs/xfs/xfs_dir2_sf.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_sf.c
+++ kern_ci/fs/xfs/xfs_dir2_sf.c
@@ -815,6 +815,7 @@ xfs_dir2_sf_lookup(
xfs_dir2_sf_entry_t *sfep; /* shortform directory entry */
xfs_dir2_sf_t *sfp; /* shortform structure */
enum xfs_dacmp cmp; /* comparison result */
+ xfs_dir2_sf_entry_t *ci_sfep; /* case-insens. entry */
xfs_dir2_trace_args("sf_lookup", args);
xfs_dir2_sf_check(args);
@@ -852,6 +853,7 @@ xfs_dir2_sf_lookup(
/*
* Loop over all the entries trying to match ours.
*/
+ ci_sfep = NULL;
for (i = 0, sfep = xfs_dir2_sf_firstentry(sfp); i < sfp->hdr.count;
i++, sfep = xfs_dir2_sf_nextentry(sfp, sfep)) {
/*
@@ -867,15 +869,13 @@ xfs_dir2_sf_lookup(
xfs_dir2_sf_inumberp(sfep));
if (cmp == XFS_CMP_EXACT)
return XFS_ERROR(EEXIST);
+ ci_sfep = sfep;
}
}
ASSERT(xfs_da_isoknoent_op(args));
- /*
- * Here, we can only be doing a lookup (not a rename or replace).
- * If a case-insensitive match was found earlier, return "found".
- */
- if (args->cmpresult == XFS_CMP_CASE)
- return XFS_ERROR(EEXIST);
+ if (ci_sfep)
+ return XFS_ERROR(xfs_dir_cilookup_result(args,
+ ci_sfep->name, ci_sfep->namelen));
/*
* Didn't find it.
*/
Index: kern_ci/fs/xfs/xfs_vnodeops.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_vnodeops.c
+++ kern_ci/fs/xfs/xfs_vnodeops.c
@@ -1629,12 +1629,19 @@ xfs_inactive(
return VN_INACTIVE_CACHE;
}
-
+/*
+ * Lookups up an inode from "name". If ci_match is not NULL, then name->name
+ * will be replaced. If a CI match is found, name->name will point to a the
+ * actual name (caller must free) and ci_match is set to 1.
+ * The caller of xfs_lookup must call xfs_name_free(name->name) if
+ * ci_match in non-NULL. If no CI match is found, name->name will be NULL.
+ */
int
xfs_lookup(
xfs_inode_t *dp,
struct xfs_name *name,
- xfs_inode_t **ipp)
+ xfs_inode_t **ipp,
+ int *ci_match)
{
xfs_ino_t inum;
int error;
@@ -1646,15 +1653,18 @@ xfs_lookup(
return XFS_ERROR(EIO);
lock_mode = xfs_ilock_map_shared(dp);
- error = xfs_dir_lookup(NULL, dp, name, &inum);
+ error = xfs_dir_lookup(NULL, dp, name, &inum, ci_match);
xfs_iunlock_map_shared(dp, lock_mode);
if (error)
goto out;
error = xfs_iget(dp->i_mount, NULL, inum, 0, 0, ipp, 0);
- if (error)
+ if (error) {
+ if (ci_match && *ci_match)
+ kmem_free(name->name, name->len);
goto out;
+ }
xfs_itrace_ref(*ipp);
return 0;
Index: kern_ci/fs/xfs/xfs_vnodeops.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_vnodeops.h
+++ kern_ci/fs/xfs/xfs_vnodeops.h
@@ -23,7 +23,7 @@ int xfs_fsync(struct xfs_inode *ip, int
int xfs_release(struct xfs_inode *ip);
int xfs_inactive(struct xfs_inode *ip);
int xfs_lookup(struct xfs_inode *dp, struct xfs_name *name,
- struct xfs_inode **ipp);
+ struct xfs_inode **ipp, int *ci_match);
int xfs_create(struct xfs_inode *dp, struct xfs_name *name, mode_t mode,
xfs_dev_t rdev, struct xfs_inode **ipp, struct cred *credp);
int xfs_remove(struct xfs_inode *dp, struct xfs_name *name,
Index: kern_ci/include/linux/dcache.h
===================================================================
--- kern_ci.orig/include/linux/dcache.h
+++ kern_ci/include/linux/dcache.h
@@ -231,6 +231,7 @@ extern void d_delete(struct dentry *);
extern struct dentry * d_alloc(struct dentry *, const struct qstr *);
extern struct dentry * d_alloc_anon(struct inode *);
extern struct dentry * d_splice_alias(struct inode *, struct dentry *);
+extern struct dentry * d_add_ci(struct inode *, struct dentry *, struct qstr *);
extern void shrink_dcache_sb(struct super_block *);
extern void shrink_dcache_parent(struct dentry *);
extern void shrink_dcache_for_umount(struct super_block *);
--
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/4] XFS: ASCII case-insensitive support
2008-05-13 7:57 [PATCH 0/4] XFS: Case-insensitive support - ASCII only Barry Naujok
` (2 preceding siblings ...)
2008-05-13 7:57 ` [PATCH 3/4] XFS: Return case-insensitive match for dentry cache Barry Naujok
@ 2008-05-13 7:57 ` Barry Naujok
2008-05-13 8:58 ` Christoph Hellwig
3 siblings, 1 reply; 22+ messages in thread
From: Barry Naujok @ 2008-05-13 7:57 UTC (permalink / raw)
To: xfs; +Cc: linux-fsdevel
[-- Attachment #1: ascii_ci.patch --]
[-- Type: text/plain, Size: 6012 bytes --]
Implement ASCII case-insensitive support. It's primary purpose
is for supporting existing filesystems that already use this
case-insensitive mode migrated from IRIX. But, if you only need
ASCII-only case-insensitive support (ie. English only) and will
never use another language, then this mode is perfectly adequate.
ASCII-CI is implemented by generating hashes based on lower-case
letters and doing lower-case compares. It implements a new
xfs_nameops vector for doing the hashes and comparisons for
all filename operations.
To create a filesystem with this CI mode, use:
# mkfs.xfs -n version=ci <device>
Signed-off-by: Barry Naujok <bnaujok@sgi.com>
---
fs/xfs/linux-2.6/xfs_linux.h | 1
fs/xfs/linux-2.6/xfs_super.c | 5 +++-
fs/xfs/xfs_dir2.c | 52 ++++++++++++++++++++++++++++++++++++++++++-
fs/xfs/xfs_fs.h | 1
fs/xfs/xfs_fsops.c | 4 ++-
fs/xfs/xfs_sb.h | 10 +++++++-
6 files changed, 69 insertions(+), 4 deletions(-)
Index: kern_ci/fs/xfs/linux-2.6/xfs_linux.h
===================================================================
--- kern_ci.orig/fs/xfs/linux-2.6/xfs_linux.h
+++ kern_ci/fs/xfs/linux-2.6/xfs_linux.h
@@ -76,6 +76,7 @@
#include <linux/log2.h>
#include <linux/spinlock.h>
#include <linux/random.h>
+#include <linux/ctype.h>
#include <asm/page.h>
#include <asm/div64.h>
Index: kern_ci/fs/xfs/linux-2.6/xfs_super.c
===================================================================
--- kern_ci.orig/fs/xfs/linux-2.6/xfs_super.c
+++ kern_ci/fs/xfs/linux-2.6/xfs_super.c
@@ -565,7 +565,10 @@ xfs_set_inodeops(
inode->i_mapping->a_ops = &xfs_address_space_operations;
break;
case S_IFDIR:
- inode->i_op = &xfs_dir_inode_operations;
+ if (xfs_sb_version_hasoldci(&XFS_M(inode->i_sb)->m_sb))
+ inode->i_op = &xfs_dir_ci_inode_operations;
+ else
+ inode->i_op = &xfs_dir_inode_operations;
inode->i_fop = &xfs_dir_file_operations;
break;
case S_IFLNK:
Index: kern_ci/fs/xfs/xfs_dir2.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2.c
+++ kern_ci/fs/xfs/xfs_dir2.c
@@ -48,6 +48,53 @@ struct xfs_name xfs_name_dotdot = {"..",
extern const struct xfs_nameops xfs_default_nameops;
+/*
+ * V1/OLDCI case-insensitive support for directories that was used in IRIX.
+ *
+ * This is ASCII only case support, ie. A-Z.
+ */
+STATIC xfs_dahash_t
+xfs_ascii_ci_hashname(
+ struct xfs_name *name)
+{
+ xfs_dahash_t hash;
+ int i;
+
+ for (i = 0, hash = 0; i < name->len; i++)
+ hash = tolower(name->name[i]) ^ rol32(hash, 7);
+
+ return hash;
+}
+
+STATIC enum xfs_dacmp
+xfs_ascii_ci_compname(
+ struct xfs_da_args *args,
+ const char *name,
+ int len)
+{
+ enum xfs_dacmp result;
+ int i;
+
+ if (args->namelen != len)
+ return XFS_CMP_DIFFERENT;
+
+ result = XFS_CMP_EXACT;
+ for (i = 0; i < len; i++) {
+ if (args->name[i] == name[i])
+ continue;
+ if (tolower(args->name[i]) != tolower(name[i]))
+ return XFS_CMP_DIFFERENT;
+ result = XFS_CMP_CASE;
+ }
+
+ return result;
+}
+
+static struct xfs_nameops xfs_ascii_ci_nameops = {
+ .hashname = xfs_ascii_ci_hashname,
+ .compname = xfs_ascii_ci_compname,
+};
+
void
xfs_dir_mount(
xfs_mount_t *mp)
@@ -67,7 +114,10 @@ xfs_dir_mount(
(mp->m_dirblksize - (uint)sizeof(xfs_da_node_hdr_t)) /
(uint)sizeof(xfs_da_node_entry_t);
mp->m_dir_magicpct = (mp->m_dirblksize * 37) / 100;
- mp->m_dirnameops = &xfs_default_nameops;
+ if (xfs_sb_version_hasoldci(&mp->m_sb))
+ mp->m_dirnameops = &xfs_ascii_ci_nameops;
+ else
+ mp->m_dirnameops = &xfs_default_nameops;
}
/*
Index: kern_ci/fs/xfs/xfs_fs.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_fs.h
+++ kern_ci/fs/xfs/xfs_fs.h
@@ -239,6 +239,7 @@ typedef struct xfs_fsop_resblks {
#define XFS_FSOP_GEOM_FLAGS_LOGV2 0x0100 /* log format version 2 */
#define XFS_FSOP_GEOM_FLAGS_SECTOR 0x0200 /* sector sizes >1BB */
#define XFS_FSOP_GEOM_FLAGS_ATTR2 0x0400 /* inline attributes rework */
+#define XFS_FSOP_GEOM_FLAGS_DIRV2CI 0x1000 /* ASCII only CI names */
#define XFS_FSOP_GEOM_FLAGS_LAZYSB 0x4000 /* lazy superblock counters */
Index: kern_ci/fs/xfs/xfs_fsops.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_fsops.c
+++ kern_ci/fs/xfs/xfs_fsops.c
@@ -95,6 +95,8 @@ xfs_fs_geometry(
XFS_FSOP_GEOM_FLAGS_DIRV2 : 0) |
(xfs_sb_version_hassector(&mp->m_sb) ?
XFS_FSOP_GEOM_FLAGS_SECTOR : 0) |
+ (xfs_sb_version_hasoldci(&mp->m_sb) ?
+ XFS_FSOP_GEOM_FLAGS_DIRV2CI : 0) |
(xfs_sb_version_haslazysbcount(&mp->m_sb) ?
XFS_FSOP_GEOM_FLAGS_LAZYSB : 0) |
(xfs_sb_version_hasattr2(&mp->m_sb) ?
@@ -625,7 +627,7 @@ xfs_fs_goingdown(
xfs_force_shutdown(mp, SHUTDOWN_FORCE_UMOUNT);
thaw_bdev(sb->s_bdev, sb);
}
-
+
break;
}
case XFS_FSOP_GOING_FLAGS_LOGFLUSH:
Index: kern_ci/fs/xfs/xfs_sb.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_sb.h
+++ kern_ci/fs/xfs/xfs_sb.h
@@ -46,10 +46,12 @@ struct xfs_mount;
#define XFS_SB_VERSION_SECTORBIT 0x0800
#define XFS_SB_VERSION_EXTFLGBIT 0x1000
#define XFS_SB_VERSION_DIRV2BIT 0x2000
+#define XFS_SB_VERSION_BORGBIT 0x4000 /* ASCII only case-insens. */
#define XFS_SB_VERSION_MOREBITSBIT 0x8000
#define XFS_SB_VERSION_OKSASHFBITS \
(XFS_SB_VERSION_EXTFLGBIT | \
- XFS_SB_VERSION_DIRV2BIT)
+ XFS_SB_VERSION_DIRV2BIT | \
+ XFS_SB_VERSION_BORGBIT)
#define XFS_SB_VERSION_OKREALFBITS \
(XFS_SB_VERSION_ATTRBIT | \
XFS_SB_VERSION_NLINKBIT | \
@@ -437,6 +439,12 @@ static inline int xfs_sb_version_hassect
((sbp)->sb_versionnum & XFS_SB_VERSION_SECTORBIT);
}
+static inline int xfs_sb_version_hasoldci(xfs_sb_t *sbp)
+{
+ return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4) && \
+ (sbp->sb_versionnum & XFS_SB_VERSION_BORGBIT);
+}
+
static inline int xfs_sb_version_hasmorebits(xfs_sb_t *sbp)
{
return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4) && \
--
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] XFS: Name operation vector for hash and compare
2008-05-13 7:57 ` [PATCH 1/4] XFS: Name operation vector for hash and compare Barry Naujok
@ 2008-05-13 8:31 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2008-05-13 8:31 UTC (permalink / raw)
To: Barry Naujok; +Cc: xfs, linux-fsdevel
Looks good to me.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] XFS: add op_flags field and helpers to xfs_da_args
2008-05-13 7:57 ` [PATCH 2/4] XFS: add op_flags field and helpers to xfs_da_args Barry Naujok
@ 2008-05-13 8:34 ` Christoph Hellwig
2008-05-14 6:12 ` Barry Naujok
0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2008-05-13 8:34 UTC (permalink / raw)
To: Barry Naujok; +Cc: xfs, linux-fsdevel
On Tue, May 13, 2008 at 05:57:51PM +1000, Barry Naujok wrote:
> The end of the xfs_da_args structure has 4 unsigned char fields for
> true/false information on directory and attr operations using the
> xfs_da_args structure.
>
> The following converts these 4 into a op_flags field that uses the
> first 4 bits for these fields and allows expansion for future
> operation information (eg. case-insensitive lookup request).
>
> There is also a bit of EOL whitespace cleanup too.
Looks generally good to me. A few stylistic comments:
- I don't think the xfs_da_is*_op wrappers help readability, we'd
be better off without those.
- op_flags seems like a rather odd name to me, what about
lookup_flags instead?
And the hinks below are an awfull lot of random reformatting that don't
belong into this patch. As they're sensible what about just commiting
the beforehand?
> dsize = dp->i_df.if_bytes;
> -
> +
> switch (dp->i_d.di_format) {
> case XFS_DINODE_FMT_EXTENTS:
> - /*
> - * If there is no attr fork and the data fork is extents,
> - * determine if creating the default attr fork will result
> - * in the extents form migrating to btree. If so, the
> - * minimum offset only needs to be the space required for
> + /*
> + * If there is no attr fork and the data fork is extents,
> + * determine if creating the default attr fork will result
> + * in the extents form migrating to btree. If so, the
> + * minimum offset only needs to be the space required for
> * the btree root.
> - */
> + */
> if (!dp->i_d.di_forkoff && dp->i_df.if_bytes > mp->m_attroffset)
> dsize = XFS_BMDR_SPACE_CALC(MINDBTPTRS);
> break;
> -
> +
> case XFS_DINODE_FMT_BTREE:
> /*
> * If have data btree then keep forkoff if we have one,
> - * otherwise we are adding a new attr, so then we set
> - * minforkoff to where the btree root can finish so we have
> + * otherwise we are adding a new attr, so then we set
> + * minforkoff to where the btree root can finish so we have
> * plenty of room for attrs
> */
> if (dp->i_d.di_forkoff) {
> - if (offset < dp->i_d.di_forkoff)
> + if (offset < dp->i_d.di_forkoff)
> return 0;
> - else
> + else
> return dp->i_d.di_forkoff;
> } else
> dsize = XFS_BMAP_BROOT_SPACE(dp->i_df.if_broot);
> break;
> }
> -
> - /*
> - * A data fork btree root must have space for at least
> +
> + /*
> + * A data fork btree root must have space for at least
> * MINDBTPTRS key/ptr pairs if the data fork is small or empty.
> */
> minforkoff = MAX(dsize, XFS_BMDR_SPACE_CALC(MINDBTPTRS));
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-13 7:57 ` [PATCH 3/4] XFS: Return case-insensitive match for dentry cache Barry Naujok
@ 2008-05-13 8:57 ` Christoph Hellwig
2008-05-14 6:15 ` Barry Naujok
2008-05-14 7:55 ` Barry Naujok
0 siblings, 2 replies; 22+ messages in thread
From: Christoph Hellwig @ 2008-05-13 8:57 UTC (permalink / raw)
To: Barry Naujok; +Cc: xfs, linux-fsdevel, aia21
First please Cc Anton on this as he wrote the original version of
what's now d_add_ci, I suspect he might have some useful comments.
On Tue, May 13, 2008 at 05:57:52PM +1000, Barry Naujok wrote:
> Another unusual interaction with the dcache is not storing
> negative dentries like other filesystems doing a d_add(dentry, NULL)
> when an ENOENT is returned. During the VFS lookup, if a dentry
> returned has no inode, dput is called and ENOENT is returned.
> By not doing a d_add, this actually removes it completely from
> the dcache to be reused.
That is a way to implement this correctly, but I suspect not creating
negative dentries will degrade performance quite badly on some
workloads. Then again CI is useful only for samba serving where the
namecache on the client side should mitigate that effect.
We'd probably be better off long-term implementing Anton's earlier
suggestion to have a routine that purges all ci aliased negative
dentries on a successfull lookup.
> create/rename have to be modified to
> support unhashed dentries being passed in.
> + if (ci_sfep)
> + return XFS_ERROR(xfs_dir_cilookup_result(args,
> + ci_sfep->name, ci_sfep->namelen));
Putting a function call inside XFS_ERROR is quite unreadable. Should be
easy to fix as there's already an error variable in scope.
> @@ -1646,15 +1653,18 @@ xfs_lookup(
> return XFS_ERROR(EIO);
>
> lock_mode = xfs_ilock_map_shared(dp);
> - error = xfs_dir_lookup(NULL, dp, name, &inum);
> + error = xfs_dir_lookup(NULL, dp, name, &inum, ci_match);
> xfs_iunlock_map_shared(dp, lock_mode);
>
> if (error)
> goto out;
>
> error = xfs_iget(dp->i_mount, NULL, inum, 0, 0, ipp, 0);
> - if (error)
> + if (error) {
> + if (ci_match && *ci_match)
> + kmem_free(name->name, name->len);
> goto out;
normal style would be to add a out_free_name label for this one to move
the error handling code into one place at the end of the function.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4/4] XFS: ASCII case-insensitive support
2008-05-13 7:57 ` [PATCH 4/4] XFS: ASCII case-insensitive support Barry Naujok
@ 2008-05-13 8:58 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2008-05-13 8:58 UTC (permalink / raw)
To: Barry Naujok; +Cc: xfs, linux-fsdevel
Looks good.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] XFS: add op_flags field and helpers to xfs_da_args
2008-05-13 8:34 ` Christoph Hellwig
@ 2008-05-14 6:12 ` Barry Naujok
0 siblings, 0 replies; 22+ messages in thread
From: Barry Naujok @ 2008-05-14 6:12 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs, linux-fsdevel
On Tue, 13 May 2008 18:34:58 +1000, Christoph Hellwig <hch@infradead.org>
wrote:
> On Tue, May 13, 2008 at 05:57:51PM +1000, Barry Naujok wrote:
>> The end of the xfs_da_args structure has 4 unsigned char fields for
>> true/false information on directory and attr operations using the
>> xfs_da_args structure.
>>
>> The following converts these 4 into a op_flags field that uses the
>> first 4 bits for these fields and allows expansion for future
>> operation information (eg. case-insensitive lookup request).
>>
>> There is also a bit of EOL whitespace cleanup too.
>
> Looks generally good to me. A few stylistic comments:
>
> - I don't think the xfs_da_is*_op wrappers help readability, we'd
> be better off without those.
I have taken the wrappers out.
> - op_flags seems like a rather odd name to me, what about
> lookup_flags instead?
Because these flags are used for all sorts of dir and attr operations.
> And the hinks below are an awfull lot of random reformatting that don't
> belong into this patch. As they're sensible what about just commiting
> the beforehand?
Yanked the whitespace cleanups from the follow up patches.
>> dsize = dp->i_df.if_bytes;
>> -
>> +
>> switch (dp->i_d.di_format) {
>> case XFS_DINODE_FMT_EXTENTS:
>> - /*
>> - * If there is no attr fork and the data fork is extents,
>> - * determine if creating the default attr fork will result
>> - * in the extents form migrating to btree. If so, the
>> - * minimum offset only needs to be the space required for
>> + /*
>> + * If there is no attr fork and the data fork is extents,
>> + * determine if creating the default attr fork will result
>> + * in the extents form migrating to btree. If so, the
>> + * minimum offset only needs to be the space required for
>> * the btree root.
>> - */
>> + */
>> if (!dp->i_d.di_forkoff && dp->i_df.if_bytes > mp->m_attroffset)
>> dsize = XFS_BMDR_SPACE_CALC(MINDBTPTRS);
>> break;
>> -
>> +
>> case XFS_DINODE_FMT_BTREE:
>> /*
>> * If have data btree then keep forkoff if we have one,
>> - * otherwise we are adding a new attr, so then we set
>> - * minforkoff to where the btree root can finish so we have
>> + * otherwise we are adding a new attr, so then we set
>> + * minforkoff to where the btree root can finish so we have
>> * plenty of room for attrs
>> */
>> if (dp->i_d.di_forkoff) {
>> - if (offset < dp->i_d.di_forkoff)
>> + if (offset < dp->i_d.di_forkoff)
>> return 0;
>> - else
>> + else
>> return dp->i_d.di_forkoff;
>> } else
>> dsize = XFS_BMAP_BROOT_SPACE(dp->i_df.if_broot);
>> break;
>> }
>> -
>> - /*
>> - * A data fork btree root must have space for at least
>> +
>> + /*
>> + * A data fork btree root must have space for at least
>> * MINDBTPTRS key/ptr pairs if the data fork is small or empty.
>> */
>> minforkoff = MAX(dsize, XFS_BMDR_SPACE_CALC(MINDBTPTRS));
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-13 8:57 ` Christoph Hellwig
@ 2008-05-14 6:15 ` Barry Naujok
2008-05-14 7:55 ` Barry Naujok
1 sibling, 0 replies; 22+ messages in thread
From: Barry Naujok @ 2008-05-14 6:15 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs, linux-fsdevel
On Tue, 13 May 2008 18:57:24 +1000, Christoph Hellwig <hch@infradead.org>
wrote:
> On Tue, May 13, 2008 at 05:57:52PM +1000, Barry Naujok wrote:
>
>> + if (ci_sfep)
>> + return XFS_ERROR(xfs_dir_cilookup_result(args,
>> + ci_sfep->name, ci_sfep->namelen));
>
> Putting a function call inside XFS_ERROR is quite unreadable. Should be
> easy to fix as there's already an error variable in scope.
Done (even though there was no "error" variable in this function).
>> @@ -1646,15 +1653,18 @@ xfs_lookup(
>> return XFS_ERROR(EIO);
>>
>> lock_mode = xfs_ilock_map_shared(dp);
>> - error = xfs_dir_lookup(NULL, dp, name, &inum);
>> + error = xfs_dir_lookup(NULL, dp, name, &inum, ci_match);
>> xfs_iunlock_map_shared(dp, lock_mode);
>>
>> if (error)
>> goto out;
>>
>> error = xfs_iget(dp->i_mount, NULL, inum, 0, 0, ipp, 0);
>> - if (error)
>> + if (error) {
>> + if (ci_match && *ci_match)
>> + kmem_free(name->name, name->len);
>> goto out;
>
> normal style would be to add a out_free_name label for this one to move
> the error handling code into one place at the end of the function.
Done.
I've also changed "int *ci_match" to "struct xfs_name *ci_name" to make
it's use clearer and not overload the use of the "name" parameter.
Barry.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-14 7:52 [PATCH 0/4] XFS: ASCII case-insensitivity support Barry Naujok
@ 2008-05-14 7:52 ` Barry Naujok
0 siblings, 0 replies; 22+ messages in thread
From: Barry Naujok @ 2008-05-14 7:52 UTC (permalink / raw)
To: xfs; +Cc: linux-fsdevel
[-- Attachment #1: return_name.patch --]
[-- Type: text/plain, Size: 19869 bytes --]
This implements the code to store the actual filename found
during a lookup in the dentry cache and to avoid multiple entries
in the dcache pointing to the same inode.
To avoid polluting the dcache, we implement a new directory inode
operations for lookup. xfs_vn_ci_lookup() interacts directly with
the dcache and the code was derived from ntfs_lookup() in
fs/ntfs/namei.c.
The "actual name" is only allocated and returned for a case-
insensitive match and not an actual match.
Another unusual interaction with the dcache is not storing
negative dentries like other filesystems doing a d_add(dentry, NULL)
when an ENOENT is returned. During the VFS lookup, if a dentry
returned has no inode, dput is called and ENOENT is returned.
By not doing a d_add, this actually removes it completely from
the dcache to be reused. create/rename have to be modified to
support unhashed dentries being passed in.
Signed-off-by: Barry Naujok <bnaujok@sgi.com>
---
fs/dcache.c | 104 ++++++++++++++++++++++++++++++++++++++++++
fs/xfs/linux-2.6/xfs_export.c | 2
fs/xfs/linux-2.6/xfs_iops.c | 63 +++++++++++++++++++++++++
fs/xfs/linux-2.6/xfs_iops.h | 1
fs/xfs/xfs_da_btree.h | 1
fs/xfs/xfs_dir2.c | 40 +++++++++++++++-
fs/xfs/xfs_dir2.h | 6 ++
fs/xfs/xfs_dir2_block.c | 9 ++-
fs/xfs/xfs_dir2_leaf.c | 5 +-
fs/xfs/xfs_dir2_node.c | 16 ++++--
fs/xfs/xfs_dir2_sf.c | 16 +++---
fs/xfs/xfs_vnodeops.c | 17 +++++-
fs/xfs/xfs_vnodeops.h | 2
include/linux/dcache.h | 1
14 files changed, 255 insertions(+), 28 deletions(-)
Index: kern_ci/fs/dcache.c
===================================================================
--- kern_ci.orig/fs/dcache.c
+++ kern_ci/fs/dcache.c
@@ -1191,6 +1191,109 @@ struct dentry *d_splice_alias(struct ino
return new;
}
+/**
+ * d_add_ci - lookup or allocate new dentry with case-exact name
+ * @inode: the inode case-insensitive lookup has found
+ * @dentry: the negative dentry that was passed to the parent's lookup func
+ * @name: the case-exact name to be associated with the returned dentry
+ *
+ * This is to avoid filling the dcache with case-insensitive names to the
+ * same inode, only the actual correct case is stored in the dcache for
+ * case-insensitive filesystems.
+ *
+ * For a case-insensitive lookup match and if the the case-exact dentry
+ * already exists in in the dcache, use it and return it.
+ *
+ * If no entry exists with the exact case name, allocate new dentry with
+ * the exact case, and return the spliced entry.
+ */
+
+struct dentry *d_add_ci(struct inode *inode, struct dentry *dentry,
+ struct qstr *name)
+{
+ int error;
+ struct dentry *found;
+ struct dentry *new;
+
+ /* Does a dentry matching the name exist already? */
+ found = d_hash_and_lookup(dentry->d_parent, name);
+ /* If not, create it now and return */
+ if (!found) {
+ new = d_alloc(dentry->d_parent, name);
+ if (!new) {
+ error = -ENOMEM;
+ goto err_out;
+ }
+ found = d_splice_alias(inode, new);
+ if (found) {
+ dput(new);
+ return found;
+ }
+ return new;
+ }
+ /* Matching dentry exists, check if it is negative. */
+ if (found->d_inode) {
+ if (unlikely(found->d_inode != inode)) {
+ /* This can't happen because bad inodes are unhashed. */
+ BUG_ON(!is_bad_inode(inode));
+ BUG_ON(!is_bad_inode(found->d_inode));
+ }
+ /*
+ * Already have the inode and the dentry attached, decrement
+ * the reference count to balance the iget() done
+ * earlier on. We found the dentry using d_lookup() so it
+ * cannot be disconnected and thus we do not need to worry
+ * about any NFS/disconnectedness issues here.
+ */
+ iput(inode);
+ return found;
+ }
+ /*
+ * Negative dentry: instantiate it unless the inode is a directory and
+ * has a 'disconnected' dentry (i.e. IS_ROOT and DCACHE_DISCONNECTED),
+ * in which case d_move() that in place of the found dentry.
+ */
+ if (!S_ISDIR(inode->i_mode)) {
+ /* Not a directory; everything is easy. */
+ d_instantiate(found, inode);
+ return found;
+ }
+ spin_lock(&dcache_lock);
+ if (list_empty(&inode->i_dentry)) {
+ /*
+ * Directory without a 'disconnected' dentry; we need to do
+ * d_instantiate() by hand because it takes dcache_lock which
+ * we already hold.
+ */
+ list_add(&found->d_alias, &inode->i_dentry);
+ found->d_inode = inode;
+ spin_unlock(&dcache_lock);
+ security_d_instantiate(found, inode);
+ return found;
+ }
+ /*
+ * Directory with a 'disconnected' dentry; get a reference to the
+ * 'disconnected' dentry.
+ */
+ new = list_entry(inode->i_dentry.next, struct dentry, d_alias);
+ dget_locked(new);
+ spin_unlock(&dcache_lock);
+ /* Do security vodoo. */
+ security_d_instantiate(found, inode);
+ /* Move new in place of found. */
+ d_move(new, found);
+ /* Balance the iget() we did above. */
+ iput(inode);
+ /* Throw away found. */
+ dput(found);
+ /* Use new as the actual dentry. */
+ return new;
+
+err_out:
+ iput(inode);
+ return ERR_PTR(error);
+}
+
/**
* d_lookup - search for a dentry
@@ -2178,6 +2281,7 @@ EXPORT_SYMBOL(d_path);
EXPORT_SYMBOL(d_prune_aliases);
EXPORT_SYMBOL(d_rehash);
EXPORT_SYMBOL(d_splice_alias);
+EXPORT_SYMBOL(d_add_ci);
EXPORT_SYMBOL(d_validate);
EXPORT_SYMBOL(dget_locked);
EXPORT_SYMBOL(dput);
Index: kern_ci/fs/xfs/linux-2.6/xfs_export.c
===================================================================
--- kern_ci.orig/fs/xfs/linux-2.6/xfs_export.c
+++ kern_ci/fs/xfs/linux-2.6/xfs_export.c
@@ -215,7 +215,7 @@ xfs_fs_get_parent(
struct xfs_inode *cip;
struct dentry *parent;
- error = xfs_lookup(XFS_I(child->d_inode), &xfs_name_dotdot, &cip);
+ error = xfs_lookup(XFS_I(child->d_inode), &xfs_name_dotdot, &cip, NULL);
if (unlikely(error))
return ERR_PTR(-error);
Index: kern_ci/fs/xfs/linux-2.6/xfs_iops.c
===================================================================
--- kern_ci.orig/fs/xfs/linux-2.6/xfs_iops.c
+++ kern_ci/fs/xfs/linux-2.6/xfs_iops.c
@@ -345,6 +345,8 @@ xfs_vn_mknod(
if (S_ISDIR(mode))
xfs_validate_fields(inode);
d_instantiate(dentry, inode);
+ if (d_unhashed(dentry))
+ d_rehash(dentry);
xfs_validate_fields(dir);
return -error;
@@ -389,7 +391,7 @@ xfs_vn_lookup(
return ERR_PTR(-ENAMETOOLONG);
xfs_dentry_to_name(&name, dentry);
- error = xfs_lookup(XFS_I(dir), &name, &cip);
+ error = xfs_lookup(XFS_I(dir), &name, &cip, NULL);
if (unlikely(error)) {
if (unlikely(error != ENOENT))
return ERR_PTR(-error);
@@ -400,6 +402,46 @@ xfs_vn_lookup(
return d_splice_alias(cip->i_vnode, dentry);
}
+STATIC struct dentry *
+xfs_vn_ci_lookup(
+ struct inode *dir,
+ struct dentry *dentry,
+ struct nameidata *nd)
+{
+ struct xfs_inode *ip;
+ struct xfs_name xname;
+ struct xfs_name ci_name;
+ struct qstr dname;
+ int error;
+
+ if (dentry->d_name.len >= MAXNAMELEN)
+ return ERR_PTR(-ENAMETOOLONG);
+
+ xfs_dentry_to_name(&xname, dentry);
+ error = xfs_lookup(XFS_I(dir), &xname, &ip, &ci_name);
+ if (unlikely(error)) {
+ if (unlikely(error != ENOENT))
+ return ERR_PTR(-error);
+ /*
+ * don't d_add dentry, __link_path_walk will dput the
+ * dentry if its inode is NULL which means the negative
+ * dentry will be destroyed rather than kept around.
+ */
+ return NULL;
+ }
+
+ /* if exact match, just splice and exit */
+ if (!ci_name->name)
+ return d_splice_alias(ip->i_vnode, dentry);
+
+ /* else case-insensitive match... */
+ dname.name = ci_name.name;
+ dname.len = ci_name.len;
+ dentry = d_add_ci(ip->i_vnode, dentry, &dname);
+ kmem_free(ci_name.name);
+ return dentry;
+}
+
STATIC int
xfs_vn_link(
struct dentry *old_dentry,
@@ -911,6 +953,25 @@ const struct inode_operations xfs_dir_in
.removexattr = xfs_vn_removexattr,
};
+const struct inode_operations xfs_dir_ci_inode_operations = {
+ .create = xfs_vn_create,
+ .lookup = xfs_vn_ci_lookup,
+ .link = xfs_vn_link,
+ .unlink = xfs_vn_unlink,
+ .symlink = xfs_vn_symlink,
+ .mkdir = xfs_vn_mkdir,
+ .rmdir = xfs_vn_rmdir,
+ .mknod = xfs_vn_mknod,
+ .rename = xfs_vn_rename,
+ .permission = xfs_vn_permission,
+ .getattr = xfs_vn_getattr,
+ .setattr = xfs_vn_setattr,
+ .setxattr = xfs_vn_setxattr,
+ .getxattr = xfs_vn_getxattr,
+ .listxattr = xfs_vn_listxattr,
+ .removexattr = xfs_vn_removexattr,
+};
+
const struct inode_operations xfs_symlink_inode_operations = {
.readlink = generic_readlink,
.follow_link = xfs_vn_follow_link,
Index: kern_ci/fs/xfs/linux-2.6/xfs_iops.h
===================================================================
--- kern_ci.orig/fs/xfs/linux-2.6/xfs_iops.h
+++ kern_ci/fs/xfs/linux-2.6/xfs_iops.h
@@ -20,6 +20,7 @@
extern const struct inode_operations xfs_inode_operations;
extern const struct inode_operations xfs_dir_inode_operations;
+extern const struct inode_operations xfs_dir_ci_inode_operations;
extern const struct inode_operations xfs_symlink_inode_operations;
extern const struct file_operations xfs_file_operations;
Index: kern_ci/fs/xfs/xfs_da_btree.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_da_btree.h
+++ kern_ci/fs/xfs/xfs_da_btree.h
@@ -143,6 +143,7 @@ typedef struct xfs_da_args {
#define XFS_DA_OP_RENAME 0x0002 /* this is an atomic rename op */
#define XFS_DA_OP_ADDNAME 0x0004 /* this is an add operation */
#define XFS_DA_OP_OKNOENT 0x0008 /* lookup/add op, ENOENT ok, else die */
+#define XFS_DA_OP_CILOOKUP 0x0010 /* lookup to return CI name if found */
/*
* Structure to describe buffer(s) for a block.
Index: kern_ci/fs/xfs/xfs_dir2.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2.c
+++ kern_ci/fs/xfs/xfs_dir2.c
@@ -193,14 +193,43 @@ xfs_dir_createname(
}
/*
+ * If doing a CI lookup and case-insensitive match, dup actual name into
+ * args.value. Return EEXIST for success (ie. name found) or an error.
+ */
+int
+xfs_dir_cilookup_result(
+ struct xfs_da_args *args,
+ const char *name,
+ int len)
+{
+ if (args->cmpresult == XFS_CMP_DIFFERENT)
+ return ENOENT;
+ if (args->cmpresult != XFS_CMP_CASE ||
+ !(args->op_flags & XFS_DA_OP_CILOOKUP))
+ return EEXIST;
+
+ args->value = kmem_alloc(len, KM_MAYFAIL);
+ if (!args->value)
+ return ENOMEM;
+
+ memcpy(args->value, name, len);
+ args->valuelen = len;
+ return EEXIST;
+}
+
+/*
* Lookup a name in a directory, give back the inode number.
+ * If ci_name is not NULL, returns the actual name in ci_name if it differs
+ * to name, or ci_name->name is set to NULL for an exact match.
*/
+
int
xfs_dir_lookup(
xfs_trans_t *tp,
xfs_inode_t *dp,
struct xfs_name *name,
- xfs_ino_t *inum) /* out: inode number */
+ xfs_ino_t *inum, /* out: inode number */
+ struct xfs_name *ci_name) /* out: actual name if CI match */
{
xfs_da_args_t args;
int rval;
@@ -217,6 +246,8 @@ xfs_dir_lookup(
args.whichfork = XFS_DATA_FORK;
args.trans = tp;
args.op_flags = XFS_DA_OP_OKNOENT;
+ if (ci_name)
+ args.op_flags |= XFS_DA_OP_CILOOKUP;
args.cmpresult = XFS_CMP_DIFFERENT;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
@@ -233,8 +264,13 @@ xfs_dir_lookup(
rval = xfs_dir2_node_lookup(&args);
if (rval == EEXIST)
rval = 0;
- if (rval == 0)
+ if (!rval) {
*inum = args.inumber;
+ if (ci_name) {
+ ci_name->name = args.value;
+ ci_name->len = args.valuelen;
+ }
+ }
return rval;
}
Index: kern_ci/fs/xfs/xfs_dir2.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2.h
+++ kern_ci/fs/xfs/xfs_dir2.h
@@ -74,7 +74,8 @@ extern int xfs_dir_createname(struct xfs
xfs_fsblock_t *first,
struct xfs_bmap_free *flist, xfs_extlen_t tot);
extern int xfs_dir_lookup(struct xfs_trans *tp, struct xfs_inode *dp,
- struct xfs_name *name, xfs_ino_t *inum);
+ struct xfs_name *name, xfs_ino_t *inum,
+ struct xfs_name *ci_name);
extern int xfs_dir_removename(struct xfs_trans *tp, struct xfs_inode *dp,
struct xfs_name *name, xfs_ino_t ino,
xfs_fsblock_t *first,
@@ -99,4 +100,7 @@ extern int xfs_dir2_isleaf(struct xfs_tr
extern int xfs_dir2_shrink_inode(struct xfs_da_args *args, xfs_dir2_db_t db,
struct xfs_dabuf *bp);
+extern int xfs_dir_cilookup_result(struct xfs_da_args *args, const char *name,
+ int len);
+
#endif /* __XFS_DIR2_H__ */
Index: kern_ci/fs/xfs/xfs_dir2_block.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_block.c
+++ kern_ci/fs/xfs/xfs_dir2_block.c
@@ -610,14 +610,15 @@ xfs_dir2_block_lookup(
/*
* Get the offset from the leaf entry, to point to the data.
*/
- dep = (xfs_dir2_data_entry_t *)
- ((char *)block + xfs_dir2_dataptr_to_off(mp, be32_to_cpu(blp[ent].address)));
+ dep = (xfs_dir2_data_entry_t *)((char *)block +
+ xfs_dir2_dataptr_to_off(mp, be32_to_cpu(blp[ent].address)));
/*
- * Fill in inode number, release the block.
+ * Fill in inode number, CI name if appropriate, release the block.
*/
args->inumber = be64_to_cpu(dep->inumber);
+ error = xfs_dir_cilookup_result(args, dep->name, dep->namelen);
xfs_da_brelse(args->trans, bp);
- return XFS_ERROR(EEXIST);
+ return XFS_ERROR(error);
}
/*
Index: kern_ci/fs/xfs/xfs_dir2_leaf.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_leaf.c
+++ kern_ci/fs/xfs/xfs_dir2_leaf.c
@@ -1299,12 +1299,13 @@ xfs_dir2_leaf_lookup(
((char *)dbp->data +
xfs_dir2_dataptr_to_off(dp->i_mount, be32_to_cpu(lep->address)));
/*
- * Return the found inode number.
+ * Return the found inode number & CI name if appropriate
*/
args->inumber = be64_to_cpu(dep->inumber);
+ error = xfs_dir_cilookup_result(args, dep->name, dep->namelen);
xfs_da_brelse(tp, dbp);
xfs_da_brelse(tp, lbp);
- return XFS_ERROR(EEXIST);
+ return XFS_ERROR(error);
}
/*
Index: kern_ci/fs/xfs/xfs_dir2_node.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_node.c
+++ kern_ci/fs/xfs/xfs_dir2_node.c
@@ -549,7 +549,7 @@ xfs_dir2_leafn_lookup_for_entry(
xfs_dir2_data_entry_t *dep; /* data block entry */
xfs_inode_t *dp; /* incore directory inode */
int error; /* error return value */
- int di; /* data entry index */
+ int di = -1; /* data entry index */
int index; /* leaf entry index */
xfs_dir2_leaf_t *leaf; /* leaf structure */
xfs_dir2_leaf_entry_t *lep; /* leaf entry */
@@ -577,6 +577,7 @@ xfs_dir2_leafn_lookup_for_entry(
if (state->extravalid) {
curbp = state->extrablk.bp;
curdb = state->extrablk.blkno;
+ di = state->extrablk.index;
}
/*
* Loop over leaf entries with the right hash value.
@@ -637,7 +638,6 @@ xfs_dir2_leafn_lookup_for_entry(
}
/* Didn't find an exact match. */
error = ENOENT;
- di = -1;
ASSERT(index == be16_to_cpu(leaf->hdr.count) ||
(args->op_flags & XFS_DA_OP_OKNOENT));
out:
@@ -652,7 +652,7 @@ out:
state->extravalid = 0;
}
/*
- * Return the index, that will be the insertion point.
+ * Return the index, that will be the deletion point for remove/replace.
*/
*indexp = index;
return XFS_ERROR(error);
@@ -1820,8 +1820,14 @@ xfs_dir2_node_lookup(
error = xfs_da_node_lookup_int(state, &rval);
if (error)
rval = error;
- else if (rval == ENOENT && args->cmpresult == XFS_CMP_CASE)
- rval = EEXIST; /* a case-insensitive match was found */
+ else if (rval == ENOENT && args->cmpresult == XFS_CMP_CASE) {
+ /* If a CI match, dup the actual name and return EEXIST */
+ xfs_dir2_data_entry_t *dep;
+
+ dep = (xfs_dir2_data_entry_t *)((char *)state->extrablk.bp->
+ data + state->extrablk.index);
+ rval = xfs_dir_cilookup_result(args, dep->name, dep->namelen);
+ }
/*
* Release the btree blocks and leaf block.
*/
Index: kern_ci/fs/xfs/xfs_dir2_sf.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2_sf.c
+++ kern_ci/fs/xfs/xfs_dir2_sf.c
@@ -812,9 +812,11 @@ xfs_dir2_sf_lookup(
{
xfs_inode_t *dp; /* incore directory inode */
int i; /* entry index */
+ int error;
xfs_dir2_sf_entry_t *sfep; /* shortform directory entry */
xfs_dir2_sf_t *sfp; /* shortform structure */
enum xfs_dacmp cmp; /* comparison result */
+ xfs_dir2_sf_entry_t *ci_sfep; /* case-insens. entry */
xfs_dir2_trace_args("sf_lookup", args);
xfs_dir2_sf_check(args);
@@ -852,6 +854,7 @@ xfs_dir2_sf_lookup(
/*
* Loop over all the entries trying to match ours.
*/
+ ci_sfep = NULL;
for (i = 0, sfep = xfs_dir2_sf_firstentry(sfp); i < sfp->hdr.count;
i++, sfep = xfs_dir2_sf_nextentry(sfp, sfep)) {
/*
@@ -872,14 +875,13 @@ xfs_dir2_sf_lookup(
ASSERT(args->op_flags & XFS_DA_OP_OKNOENT);
/*
* Here, we can only be doing a lookup (not a rename or replace).
- * If a case-insensitive match was found earlier, return "found".
+ * If a case-insensitive match was not found, return ENOENT.
*/
- if (args->cmpresult == XFS_CMP_CASE)
- return XFS_ERROR(EEXIST);
- /*
- * Didn't find it.
- */
- return XFS_ERROR(ENOENT);
+ if (!ci_sfep)
+ return XFS_ERROR(ENOENT);
+ /* otherwise process the CI match as required by the caller */
+ error = xfs_dir_cilookup_result(args, ci_sfep->name, ci_sfep->namelen);
+ return XFS_ERROR(error);
}
/*
Index: kern_ci/fs/xfs/xfs_vnodeops.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_vnodeops.c
+++ kern_ci/fs/xfs/xfs_vnodeops.c
@@ -1601,12 +1601,18 @@ xfs_inactive(
return VN_INACTIVE_CACHE;
}
-
+/*
+ * Lookups up an inode from "name". If ci_name is not NULL, then a CI match
+ * is allowed, otherwise it has to be an exact match. If a CI match is found,
+ * ci_name->name will point to a the actual name (caller must free) or
+ * will be set to NULL if an exact match is found.
+ */
int
xfs_lookup(
xfs_inode_t *dp,
struct xfs_name *name,
- xfs_inode_t **ipp)
+ xfs_inode_t **ipp,
+ struct xfs_name *ci_name)
{
xfs_ino_t inum;
int error;
@@ -1618,15 +1624,18 @@ xfs_lookup(
return XFS_ERROR(EIO);
lock_mode = xfs_ilock_map_shared(dp);
- error = xfs_dir_lookup(NULL, dp, name, &inum);
+ error = xfs_dir_lookup(NULL, dp, name, &inum, ci_name);
xfs_iunlock_map_shared(dp, lock_mode);
if (error)
goto out;
error = xfs_iget(dp->i_mount, NULL, inum, 0, 0, ipp, 0);
- if (error)
+ if (error) {
+ if (ci_name)
+ kmem_free(ci_name->name);
goto out;
+ }
xfs_itrace_ref(*ipp);
return 0;
Index: kern_ci/fs/xfs/xfs_vnodeops.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_vnodeops.h
+++ kern_ci/fs/xfs/xfs_vnodeops.h
@@ -22,7 +22,7 @@ int xfs_fsync(struct xfs_inode *ip);
int xfs_release(struct xfs_inode *ip);
int xfs_inactive(struct xfs_inode *ip);
int xfs_lookup(struct xfs_inode *dp, struct xfs_name *name,
- struct xfs_inode **ipp);
+ struct xfs_inode **ipp, struct xfs_name *ci_name);
int xfs_create(struct xfs_inode *dp, struct xfs_name *name, mode_t mode,
xfs_dev_t rdev, struct xfs_inode **ipp, struct cred *credp);
int xfs_remove(struct xfs_inode *dp, struct xfs_name *name,
Index: kern_ci/include/linux/dcache.h
===================================================================
--- kern_ci.orig/include/linux/dcache.h
+++ kern_ci/include/linux/dcache.h
@@ -231,6 +231,7 @@ extern void d_delete(struct dentry *);
extern struct dentry * d_alloc(struct dentry *, const struct qstr *);
extern struct dentry * d_alloc_anon(struct inode *);
extern struct dentry * d_splice_alias(struct inode *, struct dentry *);
+extern struct dentry * d_add_ci(struct inode *, struct dentry *, struct qstr *);
extern void shrink_dcache_sb(struct super_block *);
extern void shrink_dcache_parent(struct dentry *);
extern void shrink_dcache_for_umount(struct super_block *);
--
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-13 8:57 ` Christoph Hellwig
2008-05-14 6:15 ` Barry Naujok
@ 2008-05-14 7:55 ` Barry Naujok
2008-05-15 4:57 ` Christoph Hellwig
1 sibling, 1 reply; 22+ messages in thread
From: Barry Naujok @ 2008-05-14 7:55 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs, linux-fsdevel, aia21
On Tue, 13 May 2008 18:57:24 +1000, Christoph Hellwig <hch@infradead.org>
wrote:
> First please Cc Anton on this as he wrote the original version of
> what's now d_add_ci, I suspect he might have some useful comments.
>
>
> On Tue, May 13, 2008 at 05:57:52PM +1000, Barry Naujok wrote:
>> Another unusual interaction with the dcache is not storing
>> negative dentries like other filesystems doing a d_add(dentry, NULL)
>> when an ENOENT is returned. During the VFS lookup, if a dentry
>> returned has no inode, dput is called and ENOENT is returned.
>> By not doing a d_add, this actually removes it completely from
>> the dcache to be reused.
>
> That is a way to implement this correctly, but I suspect not creating
> negative dentries will degrade performance quite badly on some
> workloads. Then again CI is useful only for samba serving where the
> namecache on the client side should mitigate that effect.
Not quite sure if this is the right test, but I did 1000 creates on
a brand new filesystem with and without ci on my SATA drive, both
sustained almost 600 creates per second.
I believe creates would be the worst case scenario for not adding
negative dentries?
> We'd probably be better off long-term implementing Anton's earlier
> suggestion to have a routine that purges all ci aliased negative
> dentries on a successfull lookup.
Barry.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-14 7:55 ` Barry Naujok
@ 2008-05-15 4:57 ` Christoph Hellwig
2008-05-15 5:14 ` Barry Naujok
0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2008-05-15 4:57 UTC (permalink / raw)
To: Barry Naujok; +Cc: Christoph Hellwig, xfs, linux-fsdevel, aia21
On Wed, May 14, 2008 at 05:55:45PM +1000, Barry Naujok wrote:
> Not quite sure if this is the right test, but I did 1000 creates on
> a brand new filesystem with and without ci on my SATA drive, both
> sustained almost 600 creates per second.
>
> I believe creates would be the worst case scenario for not adding
> negative dentries?
No, negative dentries shouldn't have any effect on that. negative
entries help to optimize away lookups. E.g. thing of the PATH variable
and say your shell is not in the first directory listed there. Having
a negative dentry for it means that you don't have to do a lookup in
the first directories everytime someone wants to use the shell.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-15 4:57 ` Christoph Hellwig
@ 2008-05-15 5:14 ` Barry Naujok
2008-05-15 13:43 ` Anton Altaparmakov
0 siblings, 1 reply; 22+ messages in thread
From: Barry Naujok @ 2008-05-15 5:14 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs, linux-fsdevel, aia21
On Thu, 15 May 2008 14:57:00 +1000, Christoph Hellwig <hch@infradead.org>
wrote:
> On Wed, May 14, 2008 at 05:55:45PM +1000, Barry Naujok wrote:
>> Not quite sure if this is the right test, but I did 1000 creates on
>> a brand new filesystem with and without ci on my SATA drive, both
>> sustained almost 600 creates per second.
>>
>> I believe creates would be the worst case scenario for not adding
>> negative dentries?
>
> No, negative dentries shouldn't have any effect on that. negative
> entries help to optimize away lookups. E.g. thing of the PATH variable
> and say your shell is not in the first directory listed there. Having
> a negative dentry for it means that you don't have to do a lookup in
> the first directories everytime someone wants to use the shell.
Ah, that makes more sense. I did a test of a million lookups to a
non-existant file in a short-form directory (dual 1.6G opteron):
CI = 4.6s
non-CI = 3.7s
And a directory with 10000 files:
CI = 10.3s
non-CI = 3.9s
Barry.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-15 5:14 ` Barry Naujok
@ 2008-05-15 13:43 ` Anton Altaparmakov
2008-05-15 14:11 ` Christoph Hellwig
2008-05-20 2:24 ` Barry Naujok
0 siblings, 2 replies; 22+ messages in thread
From: Anton Altaparmakov @ 2008-05-15 13:43 UTC (permalink / raw)
To: Barry Naujok; +Cc: Christoph Hellwig, xfs, linux-fsdevel
Hi,
On 15 May 2008, at 06:14, Barry Naujok wrote:
> On Thu, 15 May 2008 14:57:00 +1000, Christoph Hellwig <hch@infradead.org
> > wrote:
>> On Wed, May 14, 2008 at 05:55:45PM +1000, Barry Naujok wrote:
>>> Not quite sure if this is the right test, but I did 1000 creates on
>>> a brand new filesystem with and without ci on my SATA drive, both
>>> sustained almost 600 creates per second.
>>>
>>> I believe creates would be the worst case scenario for not adding
>>> negative dentries?
>>
>> No, negative dentries shouldn't have any effect on that. negative
>> entries help to optimize away lookups. E.g. thing of the PATH
>> variable
>> and say your shell is not in the first directory listed there.
>> Having
>> a negative dentry for it means that you don't have to do a lookup in
>> the first directories everytime someone wants to use the shell.
>
> Ah, that makes more sense. I did a test of a million lookups to a
> non-existant file in a short-form directory (dual 1.6G opteron):
>
> CI = 4.6s
> non-CI = 3.7s
>
> And a directory with 10000 files:
>
> CI = 10.3s
> non-CI = 3.9s
Yes, and you can get the performance back if you allow negative
dentries to be created. You just have to make sure that every time a
directory entry is created in directory X, all negative dentries which
are children of directory X are thrown away.
Failure to do so will result in lookups returning ENOENT even though a
file now exists that matches case insensitively. This happens because
the VFS will find the negative dentry and return ENOENT without
calling the file system lookup method thus the file system does not
get a chance to discover the new matching directory entry...
Best regards,
Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-15 13:43 ` Anton Altaparmakov
@ 2008-05-15 14:11 ` Christoph Hellwig
2008-05-16 0:30 ` Barry Naujok
2008-05-16 7:25 ` Anton Altaparmakov
2008-05-20 2:24 ` Barry Naujok
1 sibling, 2 replies; 22+ messages in thread
From: Christoph Hellwig @ 2008-05-15 14:11 UTC (permalink / raw)
To: Anton Altaparmakov; +Cc: Barry Naujok, Christoph Hellwig, xfs, linux-fsdevel
On Thu, May 15, 2008 at 02:43:44PM +0100, Anton Altaparmakov wrote:
> Yes, and you can get the performance back if you allow negative dentries to
> be created. You just have to make sure that every time a directory entry
> is created in directory X, all negative dentries which are children of
> directory X are thrown away.
We might even be able to optimize this a little by calling d_compare on
each alias to see if it hashes down to the same one down in the fs.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-15 14:11 ` Christoph Hellwig
@ 2008-05-16 0:30 ` Barry Naujok
2008-05-16 7:25 ` Anton Altaparmakov
1 sibling, 0 replies; 22+ messages in thread
From: Barry Naujok @ 2008-05-16 0:30 UTC (permalink / raw)
To: Christoph Hellwig, Anton Altaparmakov; +Cc: xfs, linux-fsdevel
On Fri, 16 May 2008 00:11:21 +1000, Christoph Hellwig <hch@infradead.org>
wrote:
> On Thu, May 15, 2008 at 02:43:44PM +0100, Anton Altaparmakov wrote:
>> Yes, and you can get the performance back if you allow negative
>> dentries to
>> be created. You just have to make sure that every time a directory
>> entry
>> is created in directory X, all negative dentries which are children of
>> directory X are thrown away.
>
> We might even be able to optimize this a little by calling d_compare on
> each alias to see if it hashes down to the same one down in the fs.
So, I gather from this comment is that the CI FS implementation overrides
d_compare but not d_hash, and the CI purge function will behave differently
if d_compare is overridden or not? If overridden, use it as per your
suggestion and if not (due to oversight or unexpected problems), purge
all negative children?
Barry.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-15 14:11 ` Christoph Hellwig
2008-05-16 0:30 ` Barry Naujok
@ 2008-05-16 7:25 ` Anton Altaparmakov
1 sibling, 0 replies; 22+ messages in thread
From: Anton Altaparmakov @ 2008-05-16 7:25 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Barry Naujok, xfs, linux-fsdevel
Hi,
On 15 May 2008, at 15:11, Christoph Hellwig wrote:
> On Thu, May 15, 2008 at 02:43:44PM +0100, Anton Altaparmakov wrote:
>> Yes, and you can get the performance back if you allow negative
>> dentries to
>> be created. You just have to make sure that every time a directory
>> entry
>> is created in directory X, all negative dentries which are children
>> of
>> directory X are thrown away.
>
> We might even be able to optimize this a little by calling d_compare
> on
> each alias to see if it hashes down to the same one down in the fs.
Perhaps, although I am not convinced that wouldn't be worse than just
throwing them all away. For example think of a very active directory
with thousands of negative dentries in it. A create comes in and we
either throw away thousands of entries or we perform a case
insensitive comparison thousands of times and discard only a few
entries. I am concerned that the "thousands of case insensitive
comparisons" would actually be very costly and far outweigh the cost
of throwing all negative dentries away and letting them be created
again if they are requested again.
At lest in NTFS the case insensitive comparison is very expensive as
it involves converting both the UTF8 strings into little endian 2-byte
fixed width Unicode, and then for each Unicode character of each of
the strings being compared, individually performing a look up in the
128kiB large Unicode upcase table and then the two upcased characters
are compared and if they match we move to the next character.
Doing that a thousand times would be way more expensive then simply
throwing all negative dentries away I would think.
In the end it probably depends on the usage scenario as to what will
be more efficient and perhaps on the file system as well so it may be
worth allowing the file system to decide whether to try and do
comparisons or to simply throw all the negative dentries away.
Best regards,
Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-15 13:43 ` Anton Altaparmakov
2008-05-15 14:11 ` Christoph Hellwig
@ 2008-05-20 2:24 ` Barry Naujok
2008-05-20 18:23 ` Christoph Hellwig
1 sibling, 1 reply; 22+ messages in thread
From: Barry Naujok @ 2008-05-20 2:24 UTC (permalink / raw)
To: Anton Altaparmakov; +Cc: Christoph Hellwig, xfs, linux-fsdevel
On Thu, 15 May 2008 23:43:44 +1000, Anton Altaparmakov <aia21@cam.ac.uk>
wrote:
> Hi,
>
> On 15 May 2008, at 06:14, Barry Naujok wrote:
>> On Thu, 15 May 2008 14:57:00 +1000, Christoph Hellwig <hch@infradead.org
>> > wrote:
>>> On Wed, May 14, 2008 at 05:55:45PM +1000, Barry Naujok wrote:
>>>> Not quite sure if this is the right test, but I did 1000 creates on
>>>> a brand new filesystem with and without ci on my SATA drive, both
>>>> sustained almost 600 creates per second.
>>>>
>>>> I believe creates would be the worst case scenario for not adding
>>>> negative dentries?
>>>
>>> No, negative dentries shouldn't have any effect on that. negative
>>> entries help to optimize away lookups. E.g. thing of the PATH variable
>>> and say your shell is not in the first directory listed there. Having
>>> a negative dentry for it means that you don't have to do a lookup in
>>> the first directories everytime someone wants to use the shell.
>>
>> Ah, that makes more sense. I did a test of a million lookups to a
>> non-existant file in a short-form directory (dual 1.6G opteron):
>>
>> CI = 4.6s
>> non-CI = 3.7s
>>
>> And a directory with 10000 files:
>>
>> CI = 10.3s
>> non-CI = 3.9s
>
>
> Yes, and you can get the performance back if you allow negative dentries
> to be created. You just have to make sure that every time a directory
> entry is created in directory X, all negative dentries which are
> children of directory X are thrown away.
>
> Failure to do so will result in lookups returning ENOENT even though a
> file now exists that matches case insensitively. This happens because
> the VFS will find the negative dentry and return ENOENT without calling
> the file system lookup method thus the file system does not get a chance
> to discover the new matching directory entry...
Ok, with the following snippet of code, it now takes 2.9s for shortform CI
loop and 2.7s with 10000 files. Don't know why it's quicker, but, anyway,
no slowdown with CI anymore.
+/**
+ * d_drop_neg_children - drop negative child dentries
+ * @parent: parent dentry
+ *
+ * Searches the children of the @parent dentry for negative dentries and
+ * drops them as they are found.
+ *
+ * This is primarily useful for case-insensitive filesystems to drop these
+ * entries when a new entry is created in the parent. The new entry must
+ * be instantiated before calling this function.
+ */
+
+void d_drop_neg_children(struct dentry *parent)
+{
+ struct dentry *dentry;
+
+ spin_lock(&dcache_lock);
+ list_for_each_entry(dentry, &parent->d_subdirs, d_u.d_child) {
+ if (!dentry->d_inode) {
+ spin_lock(&dentry->d_lock);
+ __d_drop(dentry);
+ spin_unlock(&dentry->d_lock);
+ cond_resched_lock(&dcache_lock);
+ }
+ }
+ spin_unlock(&dcache_lock);
+
+}
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-20 2:24 ` Barry Naujok
@ 2008-05-20 18:23 ` Christoph Hellwig
2008-05-20 20:50 ` Sunil Mushran
0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2008-05-20 18:23 UTC (permalink / raw)
To: Barry Naujok; +Cc: Anton Altaparmakov, Christoph Hellwig, xfs, linux-fsdevel
On Tue, May 20, 2008 at 12:24:57PM +1000, Barry Naujok wrote:
> +/**
> + * d_drop_neg_children - drop negative child dentries
> + * @parent: parent dentry
> + *
> + * Searches the children of the @parent dentry for negative dentries and
> + * drops them as they are found.
> + *
> + * This is primarily useful for case-insensitive filesystems to drop these
> + * entries when a new entry is created in the parent. The new entry must
> + * be instantiated before calling this function.
> + */
> +
> +void d_drop_neg_children(struct dentry *parent)
please spell out the negative :) also no empty line between the
kerneldoc sand the actual function please.
> +{
> + struct dentry *dentry;
> +
> + spin_lock(&dcache_lock);
> + list_for_each_entry(dentry, &parent->d_subdirs, d_u.d_child) {
> + if (!dentry->d_inode) {
> + spin_lock(&dentry->d_lock);
> + __d_drop(dentry);
> + spin_unlock(&dentry->d_lock);
> + cond_resched_lock(&dcache_lock);
The cond_resched_lock here is not safe here, because the pointer you
are going to dereference in list_for_each_entry might not be valid
anymore. This should look more like:
void d_drop_negative_children(struct dentry *parent)
{
struct dentry *dentry;
again:
spin_lock(&dcache_lock);
list_for_each_entry(dentry, &parent->d_subdirs, d_u.d_child) {
if !dentry->d_inode)
continue;
spin_lock(&dentry->d_lock);
__d_drop(dentry);
spin_unlock(&dentry->d_lock);
if (need_resched()) {
spin_unlock(&dcache_lock);
cond_resched();
goto again;
}
}
spin_unlock(&dcache_lock);
}
Btw, any reason you haven't commited patches 1 and 2 yet? Also maybe
splitting 3 and 4 differently with one patch for the two new functions
in dcache.c and one for the full XFS ascii CI support might be best.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
2008-05-20 18:23 ` Christoph Hellwig
@ 2008-05-20 20:50 ` Sunil Mushran
0 siblings, 0 replies; 22+ messages in thread
From: Sunil Mushran @ 2008-05-20 20:50 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Barry Naujok, Anton Altaparmakov, xfs, linux-fsdevel
Christoph Hellwig wrote:
> The cond_resched_lock here is not safe here, because the pointer you
> are going to dereference in list_for_each_entry might not be valid
> anymore. This should look more like:
>
> void d_drop_negative_children(struct dentry *parent)
> {
> struct dentry *dentry;
>
> again:
> spin_lock(&dcache_lock);
> list_for_each_entry(dentry, &parent->d_subdirs, d_u.d_child) {
> if !(dentry->d_inode)
> continue;
>
> spin_lock(&dentry->d_lock);
> __d_drop(dentry);
> spin_unlock(&dentry->d_lock);
>
> if (need_resched()) {
> spin_unlock(&dcache_lock);
> cond_resched();
> goto again;
> }
> }
> spin_unlock(&dcache_lock);
> }
>
Yes, we have been bitten by the same issue.
Instead of need_resched(), it may be better if you do:
if (cond_resched_lock(&dcache_lock))
goto again;
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2008-05-20 20:50 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-13 7:57 [PATCH 0/4] XFS: Case-insensitive support - ASCII only Barry Naujok
2008-05-13 7:57 ` [PATCH 1/4] XFS: Name operation vector for hash and compare Barry Naujok
2008-05-13 8:31 ` Christoph Hellwig
2008-05-13 7:57 ` [PATCH 2/4] XFS: add op_flags field and helpers to xfs_da_args Barry Naujok
2008-05-13 8:34 ` Christoph Hellwig
2008-05-14 6:12 ` Barry Naujok
2008-05-13 7:57 ` [PATCH 3/4] XFS: Return case-insensitive match for dentry cache Barry Naujok
2008-05-13 8:57 ` Christoph Hellwig
2008-05-14 6:15 ` Barry Naujok
2008-05-14 7:55 ` Barry Naujok
2008-05-15 4:57 ` Christoph Hellwig
2008-05-15 5:14 ` Barry Naujok
2008-05-15 13:43 ` Anton Altaparmakov
2008-05-15 14:11 ` Christoph Hellwig
2008-05-16 0:30 ` Barry Naujok
2008-05-16 7:25 ` Anton Altaparmakov
2008-05-20 2:24 ` Barry Naujok
2008-05-20 18:23 ` Christoph Hellwig
2008-05-20 20:50 ` Sunil Mushran
2008-05-13 7:57 ` [PATCH 4/4] XFS: ASCII case-insensitive support Barry Naujok
2008-05-13 8:58 ` Christoph Hellwig
-- strict thread matches above, loose matches on Subject: below --
2008-05-14 7:52 [PATCH 0/4] XFS: ASCII case-insensitivity support Barry Naujok
2008-05-14 7:52 ` [PATCH 3/4] XFS: Return case-insensitive match for dentry cache Barry Naujok
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).