Re: Add operation vcache_rekey

["J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>]

On 25 Jun 2014, at 11:46, Taylor R Campbell 
<campbell+netbsd-tech-kern%mumble.net@localhost> wrote:

>   Date: Wed, 25 Jun 2014 11:24:16 +0200
>   From: "J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>
> 
>   The attached diff adds a new vcache operation
> 
>           void
>           vcache_rekey(struct mount *mp, void *old_key, size_t old_key_len,
>               void *new_key, size_t new_key_len)
> 
>   to atomically change the key of a cached vnode.  As the key is no longer
>   constant remove "const" from all key arguments.
> 
>   Comments or objections anyone?
> 
> I assume you're doing this in preparation for replacing the msdosfs
> ihash without breaking msdosfs_rename, yes?

Yes.

> I'm not sure all the const removal is sensible.  For example, it's not
> clear to me why vcache_get(mp, &ino, sizeof ino, &vp) should ever
> modify ino.  I think the only place in the API you really want to
> remove const is in the loadvnode prototype:
> 
> int   xyzfs_loadvnode(struct mount *mp, struct vnode *vp,
>           const void *key, size_t key_len, void **new_key);
>                                             ^^^^^^^^^^^^^^
> And I think vcache_rekey should take const pointers too.  (This may
> require a little judicious __UNCONST in the implementation of the API
> as an artefact of the dual use of struct vcache_key.)

This is all because vk_key member cannot be "const" as it is the
destination of memcpy().  This "const" removal then ripples up.

Using "memcpy(__UNCONST(node->vn_key.vk_key), new_key, new_key_len)"
works but I'm not sure if __UNCONST is the right way to go.

> 
> But I may be missing something here.  If I'm barking up the wrong tree
> with this, could you show how you plan to use vcache_rekey with
> msdosfs or whatever else needs it?
> 
> Also, I wonder whether any file systems will ever change the length of
> the key for a vnode.  Probably not.

--
J. Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig 
(Germany)