Re: svn commit: r333859 - head/sys/kern

[Matthew Macy]

Bear in mind that prior to my change most functions would call it
without ever using it on non-debug builds.

On Mon, May 21, 2018 at 9:54 AM, Eric van Gyzen  wrote:
> On 05/19/2018 00:09, Matt Macy wrote:
>> @@ -1663,16 +1655,18 @@ static int
>>  umtxq_sleep_pi(struct umtx_q *uq, struct umtx_pi *pi, uint32_t owner,
>>  const char *wmesg, struct abs_timeout *timo, bool shared)
>>  {
>> - struct umtxq_chain *uc;
>>   struct thread *td, *td1;
>>   struct umtx_q *uq1;
>>   int error, pri;
>> +#ifdef INVARIANTS
>> + struct umtxq_chain *uc;
>>
>> + uc = umtxq_getchain(>pi_key);
>> +#endif
>>   error = 0;
>>   td = uq->uq_thread;
>>   KASSERT(td == curthread, ("inconsistent uq_thread"));
>> - uc = umtxq_getchain(>uq_key);
>> - UMTXQ_LOCKED_ASSERT(uc);
>> + UMTXQ_LOCKED_ASSERT(umtxq_getchain(>uq_key));
>
> Couldn't this line stay as it was?
>
> UMTXQ_LOCKED_ASSERT(uc);
>
> With the current code, we're calling umtxq_getchain() once more than
> necessary.  Also, the casual reader might be confused by calling it with
> two different arguments.
>
> Eric
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333859 - head/sys/kern

[Matthew Macy]

Good point. Will fix.

On Mon, May 21, 2018 at 9:54 AM, Eric van Gyzen  wrote:
> On 05/19/2018 00:09, Matt Macy wrote:
>> @@ -1663,16 +1655,18 @@ static int
>>  umtxq_sleep_pi(struct umtx_q *uq, struct umtx_pi *pi, uint32_t owner,
>>  const char *wmesg, struct abs_timeout *timo, bool shared)
>>  {
>> - struct umtxq_chain *uc;
>>   struct thread *td, *td1;
>>   struct umtx_q *uq1;
>>   int error, pri;
>> +#ifdef INVARIANTS
>> + struct umtxq_chain *uc;
>>
>> + uc = umtxq_getchain(>pi_key);
>> +#endif
>>   error = 0;
>>   td = uq->uq_thread;
>>   KASSERT(td == curthread, ("inconsistent uq_thread"));
>> - uc = umtxq_getchain(>uq_key);
>> - UMTXQ_LOCKED_ASSERT(uc);
>> + UMTXQ_LOCKED_ASSERT(umtxq_getchain(>uq_key));
>
> Couldn't this line stay as it was?
>
> UMTXQ_LOCKED_ASSERT(uc);
>
> With the current code, we're calling umtxq_getchain() once more than
> necessary.  Also, the casual reader might be confused by calling it with
> two different arguments.
>
> Eric
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333859 - head/sys/kern

[Eric van Gyzen]

On 05/19/2018 00:09, Matt Macy wrote:
> @@ -1663,16 +1655,18 @@ static int
>  umtxq_sleep_pi(struct umtx_q *uq, struct umtx_pi *pi, uint32_t owner,
>  const char *wmesg, struct abs_timeout *timo, bool shared)
>  {
> - struct umtxq_chain *uc;
>   struct thread *td, *td1;
>   struct umtx_q *uq1;
>   int error, pri;
> +#ifdef INVARIANTS
> + struct umtxq_chain *uc;
>  
> + uc = umtxq_getchain(>pi_key);
> +#endif
>   error = 0;
>   td = uq->uq_thread;
>   KASSERT(td == curthread, ("inconsistent uq_thread"));
> - uc = umtxq_getchain(>uq_key);
> - UMTXQ_LOCKED_ASSERT(uc);
> + UMTXQ_LOCKED_ASSERT(umtxq_getchain(>uq_key));

Couldn't this line stay as it was?

UMTXQ_LOCKED_ASSERT(uc);

With the current code, we're calling umtxq_getchain() once more than
necessary.  Also, the casual reader might be confused by calling it with
two different arguments.

Eric
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r333859 - head/sys/kern

[Matt Macy]

Author: mmacy
Date: Sat May 19 05:09:10 2018
New Revision: 333859
URL: https://svnweb.freebsd.org/changeset/base/333859

Log:
  umtx: don't call umtxq_getchain unless the value is needed

Modified:
  head/sys/kern/kern_umtx.c

Modified: head/sys/kern/kern_umtx.c
==
--- head/sys/kern/kern_umtx.c   Sat May 19 05:07:31 2018(r333858)
+++ head/sys/kern/kern_umtx.c   Sat May 19 05:09:10 2018(r333859)
@@ -662,11 +662,9 @@ umtxq_remove_queue(struct umtx_q *uq, int q)
 static int
 umtxq_count(struct umtx_key *key)
 {
-   struct umtxq_chain *uc;
struct umtxq_queue *uh;
 
-   uc = umtxq_getchain(key);
-   UMTXQ_LOCKED_ASSERT(uc);
+   UMTXQ_LOCKED_ASSERT(umtxq_getchain(key));
uh = umtxq_queue_lookup(key, UMTX_SHARED_QUEUE);
if (uh != NULL)
return (uh->length);
@@ -680,12 +678,10 @@ umtxq_count(struct umtx_key *key)
 static int
 umtxq_count_pi(struct umtx_key *key, struct umtx_q **first)
 {
-   struct umtxq_chain *uc;
struct umtxq_queue *uh;
 
*first = NULL;
-   uc = umtxq_getchain(key);
-   UMTXQ_LOCKED_ASSERT(uc);
+   UMTXQ_LOCKED_ASSERT(umtxq_getchain(key));
uh = umtxq_queue_lookup(key, UMTX_SHARED_QUEUE);
if (uh != NULL) {
*first = TAILQ_FIRST(>head);
@@ -727,14 +723,12 @@ umtxq_check_susp(struct thread *td)
 static int
 umtxq_signal_queue(struct umtx_key *key, int n_wake, int q)
 {
-   struct umtxq_chain *uc;
struct umtxq_queue *uh;
struct umtx_q *uq;
int ret;
 
ret = 0;
-   uc = umtxq_getchain(key);
-   UMTXQ_LOCKED_ASSERT(uc);
+   UMTXQ_LOCKED_ASSERT(umtxq_getchain(key));
uh = umtxq_queue_lookup(key, q);
if (uh != NULL) {
while ((uq = TAILQ_FIRST(>head)) != NULL) {
@@ -754,10 +748,8 @@ umtxq_signal_queue(struct umtx_key *key, int n_wake, i
 static inline void
 umtxq_signal_thread(struct umtx_q *uq)
 {
-   struct umtxq_chain *uc;
 
-   uc = umtxq_getchain(>uq_key);
-   UMTXQ_LOCKED_ASSERT(uc);
+   UMTXQ_LOCKED_ASSERT(umtxq_getchain(>uq_key));
umtxq_remove(uq);
wakeup(uq);
 }
@@ -1663,16 +1655,18 @@ static int
 umtxq_sleep_pi(struct umtx_q *uq, struct umtx_pi *pi, uint32_t owner,
 const char *wmesg, struct abs_timeout *timo, bool shared)
 {
-   struct umtxq_chain *uc;
struct thread *td, *td1;
struct umtx_q *uq1;
int error, pri;
+#ifdef INVARIANTS
+   struct umtxq_chain *uc;
 
+   uc = umtxq_getchain(>pi_key);
+#endif
error = 0;
td = uq->uq_thread;
KASSERT(td == curthread, ("inconsistent uq_thread"));
-   uc = umtxq_getchain(>uq_key);
-   UMTXQ_LOCKED_ASSERT(uc);
+   UMTXQ_LOCKED_ASSERT(umtxq_getchain(>uq_key));
KASSERT(uc->uc_busy != 0, ("umtx chain is not busy"));
umtxq_insert(uq);
mtx_lock(_lock);
@@ -1728,10 +1722,8 @@ umtxq_sleep_pi(struct umtx_q *uq, struct umtx_pi *pi, 
 static void
 umtx_pi_ref(struct umtx_pi *pi)
 {
-   struct umtxq_chain *uc;
 
-   uc = umtxq_getchain(>pi_key);
-   UMTXQ_LOCKED_ASSERT(uc);
+   UMTXQ_LOCKED_ASSERT(umtxq_getchain(>pi_key));
pi->pi_refcount++;
 }
 
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"