All of lore.kernel.org
 help / color / mirror / Atom feed
From: Waiman Long <longman@redhat.com>
To: Hillf Danton <hdanton@sina.com>
Cc: "Mukesh Ojha" <quic_mojha@quicinc.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Ingo Molnar" <mingo@redhat.com>, "Will Deacon" <will@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	linux-kernel@vger.kernel.org, john.p.donnelly@oracle.com,
	"Ting11 Wang 王婷" <wangting11@xiaomi.com>
Subject: Re: [PATCH v3 4/5] locking/rwsem: Enable direct rwsem lock handoff
Date: Tue, 18 Oct 2022 22:49:02 -0400	[thread overview]
Message-ID: <086fd2bd-0d24-9f7b-8264-448f1e27c3b5@redhat.com> (raw)
In-Reply-To: <20221019022934.1166-1-hdanton@sina.com>


On 10/18/22 22:29, Hillf Danton wrote:
> On 18 Oct 2022 20:39:59 -0400 Waiman Long <longman@redhat.com>
>> On 10/18/22 19:51, Hillf Danton wrote:
>>> On 18 Oct 2022 13:37:20 -0400 Waiman Long <longman@redhat.com>
>>>> On 10/18/22 10:13, Mukesh Ojha wrote:
>>>>> On 10/18/2022 4:44 PM, Hillf Danton wrote:
>>>>>> On 17 Oct 2022 17:13:55 -0400 Waiman Long <longman@redhat.com>
>>>>>>> @@ -1067,13 +1119,33 @@ rwsem_down_read_slowpath(struct rw_semaphore
>>>>>>>                 return sem;
>>>>>>>             }
>>>>>>>             adjustment += RWSEM_FLAG_WAITERS;
>>>>>>> +    } else if ((count & RWSEM_FLAG_HANDOFF) &&
>>>>>>> +          ((count & RWSEM_LOCK_MASK) == RWSEM_READER_BIAS)) {
>>>>>> Could a couple of CPUs go read slow path in parallel?
>>>>>>
>>>> This is under wait_lock protection. So no parallel execution is possible.
>>> They individually add RWSEM_READER_BIAS to count before taking wait_lock,
>>> and the check for BIAS here does not cover the case of readers in parallel.
>>> Is this intended?
>>>
>>> Hillf
>> As I said in the patch description, the lock handoff can only be done if
>> we can be sure that there is no other active locks outstanding with the
>> handoff bit set. If at the time of the check, another reader come in and
>> adds its RWSEM_READER_BIAS, the check fail and the cpu will proceed to
>> put its waiter in the queue and begin sleeping. Hopefully, the last one
>> left will find that count has only its RWSEM_READER_BIAS and it can
>> start the handoff process.
> If handoff grants rwsem to a read waiter then the read fast path may revive.
I don't quite understand what you mean by "read fast path may revive".
> And at the time of the check, multiple readers do not break handoff IMO.

I am not saying that multiple readers will break handoff. They will just 
delay it until all their temporary RWSEM_READ_BIAS are taken off.

Cheers,
Longman


  parent reply	other threads:[~2022-10-19  2:49 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-17 21:13 [PATCH v3 0/5] lockinig/rwsem: Fix rwsem bugs & enable true lock handoff Waiman Long
2022-10-17 21:13 ` [PATCH v3 1/5] locking/rwsem: Prevent non-first waiter from spinning in down_write() slowpath Waiman Long
2022-10-24 13:17   ` Peter Zijlstra
2022-10-24 13:50     ` Waiman Long
2022-10-17 21:13 ` [PATCH v3 2/5] locking/rwsem: Limit # of null owner retries for handoff writer Waiman Long
2022-10-24 13:31   ` Peter Zijlstra
2022-10-24 15:55     ` Waiman Long
2022-10-25 11:22       ` Peter Zijlstra
2022-10-25 11:48         ` Peter Zijlstra
2022-10-25 19:55           ` Waiman Long
2022-10-25 20:14             ` Peter Zijlstra
2022-10-26  1:44               ` Waiman Long
     [not found]         ` <20221025145843.2953-1-hdanton@sina.com>
2022-10-25 19:00           ` Waiman Long
2022-10-17 21:13 ` [PATCH v3 3/5] locking/rwsem: Change waiter->hanodff_set to a handoff_state enum Waiman Long
2022-10-17 21:13 ` [PATCH v3 4/5] locking/rwsem: Enable direct rwsem lock handoff Waiman Long
2022-10-17 21:13 ` [PATCH v3 5/5] locking/rwsem: Update handoff lock events tracking Waiman Long
     [not found] ` <20221018111424.1007-1-hdanton@sina.com>
2022-10-18 14:13   ` [PATCH v3 4/5] locking/rwsem: Enable direct rwsem lock handoff Mukesh Ojha
2022-10-18 17:37     ` Waiman Long
     [not found]     ` <20221018235138.1088-1-hdanton@sina.com>
2022-10-19  0:39       ` Waiman Long
     [not found]       ` <20221019022934.1166-1-hdanton@sina.com>
2022-10-19  2:49         ` Waiman Long [this message]
     [not found]         ` <20221019070559.1220-1-hdanton@sina.com>
2022-10-19 15:02           ` Waiman Long
2022-10-24 16:18       ` Waiman Long
2022-10-20  2:46 kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=086fd2bd-0d24-9f7b-8264-448f1e27c3b5@redhat.com \
    --to=longman@redhat.com \
    --cc=boqun.feng@gmail.com \
    --cc=hdanton@sina.com \
    --cc=john.p.donnelly@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=quic_mojha@quicinc.com \
    --cc=wangting11@xiaomi.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.