Handling negative entries in a Case Insensitive file system

[RichardSharpe]

In an issue that I inadvertently closed, I said:

I have tested both the only returning NULL approach and the approach above.

The test I use is:

1. ls some-file.txt # This should return ENOENT when it does not exist.
2. touch Some-file.txt # Create a file with a name differing only by the case of the first char
3. ls some-file.txt
If the first step created a negative entry in the dcache, step 3 will fail with ENOENT when it should not.

Returning NULL seems to cause ENOENT in step 3, while doing as above (a more complicated approach) seems to work.

I went back and tested the case of returning NULL with the above test, as my previous test was slightly different, and I found that returning NULL does work correctly. It is also the simplest way to deal with this.

I with the addition of the d_iput operation I also get rid of negative entries on delete.

After we resolve the issue of how to set dentry operations I will submit a patch to clean this all up.

[behlendorf]

@RichardSharpe while your chewing on this exact problem let me suggest a small related improvement you could make to help clean this all up. The zfs_remove() and zfs_link function both have real pathname chunks commented out with a #ifdef HAVE_PN_UTILS. This was done in the early days of the project as a simplification to avoid adding pn_alloc() and pn_free().

If you added those two functions you could remove the #ifdef HAVE_PN_UTILS wrappers, use those two new functions in zpl_lookup(), and in the process bring the code a little closer to back in line with Illumos.

[tuxoko]

@RichardSharpe
You don't need to add d_op. Just add this to the end of unlink and rmdir when success.
http://lxr.free-electrons.com/source/fs/xfs/xfs_iops.c#L340

[RichardSharpe]

Great, thanks ... that is simpler!

[RichardSharpe]

@behlendorf

I see what you mean. Let me look at this, perhaps this weekend ... however, I will fix the negative entry issues first.

[RichardSharpe]

I have created a pull request for the fixups ...

I will look at the issue of pn_alloc and pn_free next.

I am thinking I probably want to allocate one piece of memory and then set up the structure correctly.

[behlendorf]

@RichardSharpe awesome thanks. I took a quick look at the patch and it looks correct to me.

Yes, I think you can just wrap your existing kmem callers with pn_alloc/pn_free wrappers. I'd suggest adding them as static inline functions in include/linux/vfs_compat.h.

As another slight aside you might take a quick look at the related code disabled by the HAVE_DNLC macro. This macro is never defined so this code, which came from illumos, is always disabled since for Linux it makes more sense to do this in the zpl_* functions where we have access to the dentry. We should probably either remove this code entirely or convert it to say a __sunos__ macro.

[RichardSharpe]

OK, now that I have seen what Illumnos does there, it does appear to be as easy as you said.

Give me a few days.

[RichardSharpe]

Hmmm, Illumnos has this:

void
pn_alloc(struct pathname *pnp)
{
    pn_alloc_sz(pnp, MAXPATHLEN);
}

Which means that the buffer they are allocating pn_path is actually MAXPATHLEN in length.