diff mbox series

[1/4] xen/watchdog: Fold exit paths to have a single unlock

Message ID 1557512884-32395-2-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series xen/watchdog: Code and API improvements to the domain watchdog | expand

Commit Message

Andrew Cooper May 10, 2019, 6:28 p.m. UTC 
This is mostly to simplify future logical changes, but it does come with a
modest redunction in compiled code size (-55, 345 => 290).

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
CC: George Dunlap <george.dunlap@eu.citrix.com>
CC: Edwin Török <edvin.torok@citrix.com>
CC: Christian Lindig <christian.lindig@citrix.com>
CC: Pau Ruiz Safont <pau.safont@citrix.com>
---
 xen/common/schedule.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

Comments

Wei Liu May 13, 2019, 1:14 p.m. UTC | #1 
On Fri, May 10, 2019 at 07:28:01PM +0100, Andrew Cooper wrote:
> This is mostly to simplify future logical changes, but it does come with a
> modest redunction in compiled code size (-55, 345 => 290).
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Jan Beulich May 13, 2019, 1:47 p.m. UTC | #2 
>>> On 10.05.19 at 20:28, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -1050,6 +1050,8 @@ static void domain_watchdog_timeout(void *data)
>  
>  static long domain_watchdog(struct domain *d, uint32_t id, uint32_t timeout)
>  {
> +    long rc = 0;

I'm wondering why this isn't plain int. Not a big deal, but I'm
curious anyway.

As per your own response to patch 2 I understand that the
other patches in this series don#t need looking at until you
send a v2.

Jan
Andrew Cooper May 13, 2019, 1:51 p.m. UTC | #3 
On 13/05/2019 14:47, Jan Beulich wrote:
>>>> On 10.05.19 at 20:28, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/common/schedule.c
>> +++ b/xen/common/schedule.c
>> @@ -1050,6 +1050,8 @@ static void domain_watchdog_timeout(void *data)
>>  
>>  static long domain_watchdog(struct domain *d, uint32_t id, uint32_t timeout)
>>  {
>> +    long rc = 0;
> I'm wondering why this isn't plain int. Not a big deal, but I'm
> curious anyway.

To match the return value.  This function is compiled twice AFAICT.

>
> As per your own response to patch 2 I understand that the
> other patches in this series don#t need looking at until you
> send a v2.

patch 3 is independent of the ABI changes, so fine in principle to
review now.

Patches 2 and 4 will definitely need changing.

~Andrew
Jan Beulich May 13, 2019, 2:01 p.m. UTC | #4 
>>> On 13.05.19 at 15:51, <andrew.cooper3@citrix.com> wrote:
> On 13/05/2019 14:47, Jan Beulich wrote:
>>>>> On 10.05.19 at 20:28, <andrew.cooper3@citrix.com> wrote:
>>> --- a/xen/common/schedule.c
>>> +++ b/xen/common/schedule.c
>>> @@ -1050,6 +1050,8 @@ static void domain_watchdog_timeout(void *data)
>>>  
>>>  static long domain_watchdog(struct domain *d, uint32_t id, uint32_t timeout)
>>>  {
>>> +    long rc = 0;
>> I'm wondering why this isn't plain int. Not a big deal, but I'm
>> curious anyway.
> 
> To match the return value.

But the compiler will happily sign-extend the value at the return statement.

>  This function is compiled twice AFAICT.

I have no idea how this matters.

>> As per your own response to patch 2 I understand that the
>> other patches in this series don#t need looking at until you
>> send a v2.
> 
> patch 3 is independent of the ABI changes, so fine in principle to
> review now.

Okay.

Jan
George Dunlap May 13, 2019, 3:22 p.m. UTC | #5 
On 5/10/19 7:28 PM, Andrew Cooper wrote:
> This is mostly to simplify future logical changes, but it does come with a
> modest redunction in compiled code size (-55, 345 => 290).
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: George Dunlap <george.dunlap@citrix.com>
diff mbox series

Patch

diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 66f1e26..47f5d04 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -1050,6 +1050,8 @@  static void domain_watchdog_timeout(void *data)
 
 static long domain_watchdog(struct domain *d, uint32_t id, uint32_t timeout)
 {
+    long rc = 0;
+
     if ( id > NR_DOMAIN_WATCHDOG_TIMERS )
         return -EINVAL;
 
@@ -1064,15 +1066,15 @@  static long domain_watchdog(struct domain *d, uint32_t id, uint32_t timeout)
             set_timer(&d->watchdog_timer[id], NOW() + SECONDS(timeout));
             break;
         }
-        spin_unlock(&d->watchdog_lock);
-        return id == NR_DOMAIN_WATCHDOG_TIMERS ? -ENOSPC : id + 1;
+        rc = id == NR_DOMAIN_WATCHDOG_TIMERS ? -ENOSPC : id + 1;
+        goto unlock;
     }
 
     id -= 1;
     if ( !test_bit(id, &d->watchdog_inuse_map) )
     {
-        spin_unlock(&d->watchdog_lock);
-        return -EINVAL;
+        rc = -EINVAL;
+        goto unlock;
     }
 
     if ( timeout == 0 )
@@ -1085,8 +1087,10 @@  static long domain_watchdog(struct domain *d, uint32_t id, uint32_t timeout)
         set_timer(&d->watchdog_timer[id], NOW() + SECONDS(timeout));
     }
 
+ unlock:
     spin_unlock(&d->watchdog_lock);
-    return 0;
+
+    return rc;
 }
 
 void watchdog_domain_init(struct domain *d)