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;
Message-ID: <bb77f456-8804-b63a-7868-19e0cd9e697f@redhat.com>
Date:   Wed, 2 Aug 2023 16:16:12 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH 11/20] locking/osq: Export osq_(lock|unlock)
Content-Language: en-US
To:     Kent Overstreet <kent.overstreet@linux.dev>,
        linux-bcachefs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Boqun Feng <boqun.feng@gmail.com>
References: <20230712211115.2174650-1-kent.overstreet@linux.dev>
 <20230712211115.2174650-12-kent.overstreet@linux.dev>
From:   Waiman Long <longman@redhat.com>
In-Reply-To: <20230712211115.2174650-12-kent.overstreet@linux.dev>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/12/23 17:11, Kent Overstreet wrote:
> These are used by bcachefs's six locks.
>
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Waiman Long <longman@redhat.com>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> ---
>   kernel/locking/osq_lock.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c
> index d5610ad52b..b752ec5cc6 100644
> --- a/kernel/locking/osq_lock.c
> +++ b/kernel/locking/osq_lock.c
> @@ -203,6 +203,7 @@ bool osq_lock(struct optimistic_spin_queue *lock)
>   
>   	return false;
>   }
> +EXPORT_SYMBOL_GPL(osq_lock);
>   
>   void osq_unlock(struct optimistic_spin_queue *lock)
>   {
> @@ -230,3 +231,4 @@ void osq_unlock(struct optimistic_spin_queue *lock)
>   	if (next)
>   		WRITE_ONCE(next->locked, 1);
>   }
> +EXPORT_SYMBOL_GPL(osq_unlock);

Have you considered extending the current rw_semaphore to support a SIX 
lock semantics? There are a number of instances in the kernel that a 
up_read() is followed by a down_write(). Basically, the code try to 
upgrade the lock from read to write. I have been thinking about adding a 
upgrade_read() API to do that. However, the concern that I had was that 
another writer may come in and make modification before the reader can 
be upgraded to have exclusive write access and will make the task to 
repeat what has been done in the read lock part. By adding a read with 
intent to upgrade to write, we can have that guarantee.

With that said, I would prefer to keep osq_{lock/unlock} for internal 
use by some higher level locking primitives - mutex, rwsem and rt_mutex.

Cheers,
Longman