Re: 9pfs: scope of rename_lock?

[Greg Kurz <groug@kaod.org>]

On Sun, 16 May 2021 19:06:44 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> Hi Greg,
> 
> while reviewing the 9p code base for further optimizations, I stumbled over 
> the 'rename_lock' introduced by 02cb7f3a2 and wondered about what exactly it 
> shall protect?
> 
> As far as I understand it, the original intention at introduction 
> (aforementioned 02cb7f3a2) was to protect
> 
> 	1. fidp->path variable
> 
> 	and
> 
> 	2.  *ANY* filesystem path from being renamed during the *entire* duration
> 	    of some concurrent 9p operation.
> 
> So because of (2.) it was introduced as a global lock. But (2.) is a dead end 
> approach anyway, isn't it?
> 

I agree that this looks terrible.

> Therefore my question: rename_lock is currently a global lock. Wouldn't it 
> make more sense to transform it from a global lock from struct V9fsState -> 
> struct V9fsFidState and just let it protect that fidp->path variable locally 
> there?
> 

Nothing comes to the top of my mind right now... Maybe Aneesh (Cc'd) can shed
some light on:

commit 02cb7f3a256517cbf3136caff2863fbafc57b540
Author: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Date:   Tue May 24 15:10:56 2011 +0530

    hw/9pfs: Use read-write lock for protecting fid path.
    
    On rename we take the write lock and this ensure path
    doesn't change as we operate on them.
    
    Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>


Why are we serializing all operations on all fid paths with a single
global lock ?

> Best regards,
> Christian Schoenebeck
> 
>