Re: WAPBL not locking enough?

[Greg Oster <oster%netbsd.org@localhost>]

On Tue, 3 May 2016 10:02:44 +0000
coypu%SDF.ORG@localhost wrote:

> Hi,
> 
> I fear that WAPBL should be locking wl_mtx before running
> wapbl_transaction_len (in wapbl_flush)
> 
> I suspect so because it performs a similar lock before
> looking at wl_bufcount in sys/kern/vfs_wapbl.c:1455-1460.
> 
> If true, how about this diff?
> 
> diff --git a/sys/kern/vfs_wapbl.c b/sys/kern/vfs_wapbl.c
> index 378b0ba..f765ce3 100644
> --- a/sys/kern/vfs_wapbl.c
> +++ b/sys/kern/vfs_wapbl.c
> @@ -1971,6 +1971,7 @@ wapbl_transaction_len(struct wapbl *wl)
>         size_t len;
>         int bph;
>  
> +       mutex_enter(&wl->wl_mtx);
>         /* Calculate number of blocks described in a blocklist header
> */ bph = (blocklen - offsetof(struct wapbl_wc_blocklist, wc_blocks)) /
>             sizeof(((struct wapbl_wc_blocklist *)0)->wc_blocks[0]);
> @@ -1981,6 +1982,7 @@ wapbl_transaction_len(struct wapbl *wl)
>         len += howmany(wl->wl_bufcount, bph) * blocklen;
>         len += howmany(wl->wl_dealloccnt, bph) * blocklen;
>         len += wapbl_transaction_inodes_len(wl);
> +       mutex_exit(&wl->wl_mtx);
>  
>         return len;
>  }
>  

It's been a while since I looked at this, but you might want to see if 
this:

 rw_enter(&wl->wl_rwlock, RW_WRITER);

takes care care of the locking that you are concerned about...  

There certainly does seem to be mutex_enter/mutex_exits missing on the
DIAGNOSTIC code around line 981.

In any event you'll want more than just my comments, and a lot more
than 'a kernel with these changes boots' for testing :)

Later...

Greg Oster