Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 5 Jul 2022 21:29:50 +0100
From:   Matthew Wilcox <willy@infradead.org>
To:     Filipe Manana <fdmanana@kernel.org>
Cc:     dai.ngo@oracle.com, linux-btrfs <linux-btrfs@vger.kernel.org>,
        gniebler@suse.com,
        "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: bug in btrfs during low memory testing.
Message-ID: <YsSfPl6IvqrM5OPU@casper.infradead.org>
References: <c09e1af7-7d1f-1bbf-5562-ead9a4d99562@oracle.com>
 <CAL3q7H7Jm034yfVYJDzugWHPamvnKU=7XSb=38ey+-L_qdd=OA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAL3q7H7Jm034yfVYJDzugWHPamvnKU=7XSb=38ey+-L_qdd=OA@mail.gmail.com>
Precedence: bulk

On Tue, Jul 05, 2022 at 09:26:47PM +0100, Filipe Manana wrote:
> In this case we can actually call xa_insert() without holding that
> spinlock, it's safe against other concurrent calls to
> btrfs_get_or_create_delayed_node(), btrfs_get_delayed_node(),
> btrfs_kill_delayed_inode_items(), etc.
> 
> However, looking at xa_insert() we have:
> 
>         xa_lock(xa);
>         err = __xa_insert(xa, index, entry, gfp);
>         xa_unlock(xa);
> 
> And xa_lock() is defined as:
> 
>         #define xa_lock(xa)             spin_lock(&(xa)->xa_lock)
> 
> So we'll always be under a spinlock even if we change btrfs to not
> take the root->inode_lock spinlock.
> 
> This seems more like a general problem outside btrfs' control.
> So CC'ing Willy to double check.

No, the XArray knows about its own spinlock.  It'll drop it if it needs
to allocate memory and the GFP flags indicate that the caller can sleep.
It doesn't know about your spinlock, so it can't do the same thing for
you ;-)