xfs
[Top] [All Lists]

Re: [PATCH 4/7] XFS: Return case-insensitive match for dentry cache

To: Barry Naujok <bnaujok@xxxxxxx>
Subject: Re: [PATCH 4/7] XFS: Return case-insensitive match for dentry cache
From: "Josef 'Jeff' Sipek" <jeffpc@xxxxxxxxxxxxxx>
Date: Wed, 2 Apr 2008 22:34:34 -0400
Cc: xfs@xxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx
In-reply-to: <20080402062708.654277049@chook.melbourne.sgi.com>
References: <20080402062508.017738664@chook.melbourne.sgi.com> <20080402062708.654277049@chook.melbourne.sgi.com>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.16 (2007-06-11)
On Wed, Apr 02, 2008 at 04:25:12PM +1000, Barry Naujok wrote:
...
> +     /*
> +      * Directory with a 'disconnected' dentry; get a reference to the
> +      * 'disconnected' dentry.
> +      */
> +     dentry = list_entry(inode->i_dentry.next, struct dentry, d_alias);

list_first_entry does the .next for you.

...
> --- kern_ci.orig/fs/xfs/xfs_da_btree.c
> +++ kern_ci/fs/xfs/xfs_da_btree.c
> @@ -2176,6 +2176,22 @@ xfs_da_reada_buf(
>               return rval;
>  }
>  
> +
> +kmem_zone_t  *xfs_da_name_zone;
> +
> +uchar_t *
> +xfs_da_name_alloc(void)
> +{
> +     return kmem_zone_zalloc(xfs_da_name_zone, KM_SLEEP);
> +}
> +
> +void
> +xfs_da_name_free(const uchar_t *name)

Since you don't care about the type anyway, you might want to make it void*,
and remove the cast from the lookup_ci code.

> +{
> +     kmem_zone_free(xfs_da_name_zone, (void *)name);

No need for the cast.

> --- kern_ci.orig/fs/xfs/xfs_dir2_leaf.c
> +++ kern_ci/fs/xfs/xfs_dir2_leaf.c
> @@ -1301,6 +1301,15 @@ xfs_dir2_leaf_lookup(
>        * Return the found inode number.
>        */
>       args->inumber = be64_to_cpu(dep->inumber);
> +     /*
> +      * If a case-insensitive match, allocate a buffer and copy the actual
> +      * name into the buffer. Return it via args->value.
> +      */
> +     if (args->cmpresult == XFS_CMP_CASE) {
> +             args->value = xfs_da_name_alloc();
> +             memcpy(args->value, dep->name, dep->namelen);
> +             args->valuelen = dep->namelen;

Perhaps having a static inline xfs_da_name_dup(...) would be useful...

...
> --- kern_ci.orig/fs/xfs/xfs_vnodeops.c
> +++ kern_ci/fs/xfs/xfs_vnodeops.c
> @@ -1762,24 +1762,33 @@ xfs_inactive(
>  int
>  xfs_lookup(
>       xfs_inode_t             *dp,
> -     bhv_vname_t             *dentry,
> -     xfs_inode_t             **ipp)
> +     bhv_vstr_t              *d_name,
> +     xfs_inode_t             **ipp,
> +     bhv_vstr_t              *ci_name)
>  {
>       xfs_inode_t             *ip;
>       xfs_ino_t               e_inum;
>       int                     error;
>       uint                    lock_mode;
> +     xfs_name_t              name, rname;
>  
>       xfs_itrace_entry(dp);
>  
>       if (XFS_FORCED_SHUTDOWN(dp->i_mount))
>               return XFS_ERROR(EIO);
>  
> +     name.name = (uchar_t *)d_name->name;

d_name->name is: const unsigned char*
name.name is:    const uchar_t*

Is there any reason why you use uchar_t - beyond the other parts of XFS use
it? (I guess this is the same question that I asked before - coding style.)

xfs_types.h defines uchar_t as unsigned char...

Josef 'Jeff' Sipek.

-- 
Defenestration n. (formal or joc.):
  The act of removing Windows from your computer in disgust, usually
  followed by the installation of Linux or some other Unix-like operating
  system.


<Prev in Thread] Current Thread [Next in Thread>