Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
        Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
        Kingdom.
        Registered in England and Wales under Company Registration No. 3798903
From:   David Howells <dhowells@redhat.com>
In-Reply-To: <42b33792-50e9-77d7-4d3e-ac5ce1adeda6@huaweicloud.com>
References: <42b33792-50e9-77d7-4d3e-ac5ce1adeda6@huaweicloud.com> <20221128031929.3918348-1-houtao@huaweicloud.com>
To:     Hou Tao <houtao@huaweicloud.com>
Cc:     dhowells@redhat.com, linux-cachefs@redhat.com,
        Jeff Layton <jlayton@kernel.org>, linux-erofs@lists.ozlabs.org,
        linux-kernel@vger.kernel.org,
        "houtao1@huawei.com" <houtao1@huawei.com>
Subject: Re: [PATCH] fscache: Use wake_up_var() to wake up pending volume acquisition
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2308528.1670585211.1@warthog.procyon.org.uk>
Date:   Fri, 09 Dec 2022 11:26:51 +0000
Message-ID: <2308529.1670585211@warthog.procyon.org.uk>
Precedence: bulk

Hou Tao <houtao@huaweicloud.com> wrote:

> >  			clear_bit(FSCACHE_VOLUME_ACQUIRE_PENDING, &cursor->flags);

Maybe this should be clear_bit_unlock() instead.

And I wonder if:

	set_bit(FSCACHE_VOLUME_ACQUIRE_PENDING, &candidate->flags);

in fscache_hash_volume() needs a barrier before it.

> > -			wake_up_bit(&cursor->flags, FSCACHE_VOLUME_ACQUIRE_PENDING);
> > +			/*
> > +			 * Paired with barrier in wait_var_event(). Check
> > +			 * waitqueue_active() and wake_up_var() for details.
> > +			 */
> > +			smp_mb__after_atomic();
> > +			wake_up_var(&cursor->flags);

That doesn't seem right.

wake_up_bit() is more selective, so should be preferred to wake_up_var().

David