* [PATCH 0/2] lockprof: eliminate a minor bug and a quirk
@ 2020-07-23 10:50 Jan Beulich
2020-07-23 10:51 ` [PATCH 1/2] lockprof: don't leave locks uninitialized upon allocation failure Jan Beulich
2020-07-23 10:52 ` [PATCH 2/2] lockprof: don't pass name into registration function Jan Beulich
0 siblings, 2 replies; 6+ messages in thread
From: Jan Beulich @ 2020-07-23 10:50 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, George Dunlap,
Andrew Cooper, Ian Jackson
1: don't leave locks uninitialized upon allocation failure
2: don't pass name into registration function
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] lockprof: don't leave locks uninitialized upon allocation failure
2020-07-23 10:50 [PATCH 0/2] lockprof: eliminate a minor bug and a quirk Jan Beulich
@ 2020-07-23 10:51 ` Jan Beulich
2020-07-23 11:23 ` Andrew Cooper
2020-07-23 10:52 ` [PATCH 2/2] lockprof: don't pass name into registration function Jan Beulich
1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2020-07-23 10:51 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, George Dunlap,
Andrew Cooper, Ian Jackson
Even if a specific struct lock_profile instance can't be allocated, the
lock itself should still be functional. As this isn't a production use
feature, also log a message in the event that the profiling struct can't
be allocated.
Fixes: d98feda5c756 ("Make lock profiling usable again")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -103,10 +103,16 @@ struct lock_profile_qhead {
do { \
struct lock_profile *prof; \
prof = xzalloc(struct lock_profile); \
- if (!prof) break; \
+ (s)->l = (spinlock_t)_SPIN_LOCK_UNLOCKED(prof); \
+ if ( !prof ) \
+ { \
+ printk(XENLOG_WARNING \
+ "lock profiling unavailable for %p(%d)'s " #l "\n", \
+ s, (s)->profile_head.idx); \
+ break; \
+ } \
prof->name = #l; \
prof->lock = &(s)->l; \
- (s)->l = (spinlock_t)_SPIN_LOCK_UNLOCKED(prof); \
prof->next = (s)->profile_head.elem_q; \
(s)->profile_head.elem_q = prof; \
} while(0)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] lockprof: don't pass name into registration function
2020-07-23 10:50 [PATCH 0/2] lockprof: eliminate a minor bug and a quirk Jan Beulich
2020-07-23 10:51 ` [PATCH 1/2] lockprof: don't leave locks uninitialized upon allocation failure Jan Beulich
@ 2020-07-23 10:52 ` Jan Beulich
2020-07-23 11:27 ` Andrew Cooper
1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2020-07-23 10:52 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, George Dunlap,
Andrew Cooper, Ian Jackson
The type uniquely identifies the associated name, hence the name fields
can be statically initialized.
Also constify not just the involved struct field, but also struct
lock_profile's. Rather than specifying lock_profile_ancs[]' dimension at
definition time, add a suitable build time check, such that at least
missing tail additions to the initializer can be spotted easily.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -392,7 +392,7 @@ struct domain *domain_create(domid_t dom
d->max_vcpus = config->max_vcpus;
}
- lock_profile_register_struct(LOCKPROF_TYPE_PERDOM, d, domid, "Domain");
+ lock_profile_register_struct(LOCKPROF_TYPE_PERDOM, d, domid);
if ( (err = xsm_alloc_security_domain(d)) != 0 )
goto fail;
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -338,7 +338,7 @@ void _spin_unlock_recursive(spinlock_t *
struct lock_profile_anc {
struct lock_profile_qhead *head_q; /* first head of this type */
- char *name; /* descriptive string for print */
+ const char *name; /* descriptive string for print */
};
typedef void lock_profile_subfunc(
@@ -348,7 +348,10 @@ extern struct lock_profile *__lock_profi
extern struct lock_profile *__lock_profile_end;
static s_time_t lock_profile_start;
-static struct lock_profile_anc lock_profile_ancs[LOCKPROF_TYPE_N];
+static struct lock_profile_anc lock_profile_ancs[] = {
+ [LOCKPROF_TYPE_GLOBAL] = { .name = "Global" },
+ [LOCKPROF_TYPE_PERDOM] = { .name = "Domain" },
+};
static struct lock_profile_qhead lock_profile_glb_q;
static spinlock_t lock_profile_lock = SPIN_LOCK_UNLOCKED;
@@ -473,13 +476,12 @@ int spinlock_profile_control(struct xen_
}
void _lock_profile_register_struct(
- int32_t type, struct lock_profile_qhead *qhead, int32_t idx, char *name)
+ int32_t type, struct lock_profile_qhead *qhead, int32_t idx)
{
qhead->idx = idx;
spin_lock(&lock_profile_lock);
qhead->head_q = lock_profile_ancs[type].head_q;
lock_profile_ancs[type].head_q = qhead;
- lock_profile_ancs[type].name = name;
spin_unlock(&lock_profile_lock);
}
@@ -504,6 +506,8 @@ static int __init lock_prof_init(void)
{
struct lock_profile **q;
+ BUILD_BUG_ON(ARRAY_SIZE(lock_profile_ancs) != LOCKPROF_TYPE_N);
+
for ( q = &__lock_profile_start; q < &__lock_profile_end; q++ )
{
(*q)->next = lock_profile_glb_q.elem_q;
@@ -511,9 +515,8 @@ static int __init lock_prof_init(void)
(*q)->lock->profile = *q;
}
- _lock_profile_register_struct(
- LOCKPROF_TYPE_GLOBAL, &lock_profile_glb_q,
- 0, "Global lock");
+ _lock_profile_register_struct(LOCKPROF_TYPE_GLOBAL,
+ &lock_profile_glb_q, 0);
return 0;
}
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -72,7 +72,7 @@ struct spinlock;
struct lock_profile {
struct lock_profile *next; /* forward link */
- char *name; /* lock name */
+ const char *name; /* lock name */
struct spinlock *lock; /* the lock itself */
u64 lock_cnt; /* # of complete locking ops */
u64 block_cnt; /* # of complete wait for lock */
@@ -118,11 +118,11 @@ struct lock_profile_qhead {
} while(0)
void _lock_profile_register_struct(
- int32_t, struct lock_profile_qhead *, int32_t, char *);
+ int32_t, struct lock_profile_qhead *, int32_t);
void _lock_profile_deregister_struct(int32_t, struct lock_profile_qhead *);
-#define lock_profile_register_struct(type, ptr, idx, print) \
- _lock_profile_register_struct(type, &((ptr)->profile_head), idx, print)
+#define lock_profile_register_struct(type, ptr, idx) \
+ _lock_profile_register_struct(type, &((ptr)->profile_head), idx)
#define lock_profile_deregister_struct(type, ptr) \
_lock_profile_deregister_struct(type, &((ptr)->profile_head))
@@ -138,7 +138,7 @@ struct lock_profile_qhead { };
#define DEFINE_SPINLOCK(l) spinlock_t l = SPIN_LOCK_UNLOCKED
#define spin_lock_init_prof(s, l) spin_lock_init(&((s)->l))
-#define lock_profile_register_struct(type, ptr, idx, print)
+#define lock_profile_register_struct(type, ptr, idx)
#define lock_profile_deregister_struct(type, ptr)
#define spinlock_profile_printall(key)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] lockprof: don't leave locks uninitialized upon allocation failure
2020-07-23 10:51 ` [PATCH 1/2] lockprof: don't leave locks uninitialized upon allocation failure Jan Beulich
@ 2020-07-23 11:23 ` Andrew Cooper
2020-07-23 11:26 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2020-07-23 11:23 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: George Dunlap, Ian Jackson, Stefano Stabellini, Julien Grall, Wei Liu
On 23/07/2020 11:51, Jan Beulich wrote:
> Even if a specific struct lock_profile instance can't be allocated, the
> lock itself should still be functional. As this isn't a production use
> feature, also log a message in the event that the profiling struct can't
> be allocated.
>
> Fixes: d98feda5c756 ("Make lock profiling usable again")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/include/xen/spinlock.h
> +++ b/xen/include/xen/spinlock.h
> @@ -103,10 +103,16 @@ struct lock_profile_qhead {
> do { \
> struct lock_profile *prof; \
> prof = xzalloc(struct lock_profile); \
> - if (!prof) break; \
> + (s)->l = (spinlock_t)_SPIN_LOCK_UNLOCKED(prof); \
> + if ( !prof ) \
> + { \
> + printk(XENLOG_WARNING \
> + "lock profiling unavailable for %p(%d)'s " #l "\n", \
> + s, (s)->profile_head.idx); \
You'll end up with far less .rodata if you use %s and pass #l in as a
parameter.
Either way, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
> + break; \
> + } \
> prof->name = #l; \
> prof->lock = &(s)->l; \
> - (s)->l = (spinlock_t)_SPIN_LOCK_UNLOCKED(prof); \
> prof->next = (s)->profile_head.elem_q; \
> (s)->profile_head.elem_q = prof; \
> } while(0)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] lockprof: don't leave locks uninitialized upon allocation failure
2020-07-23 11:23 ` Andrew Cooper
@ 2020-07-23 11:26 ` Jan Beulich
0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2020-07-23 11:26 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Julien Grall, Wei Liu, George Dunlap,
Ian Jackson, xen-devel
On 23.07.2020 13:23, Andrew Cooper wrote:
> On 23/07/2020 11:51, Jan Beulich wrote:
>> Even if a specific struct lock_profile instance can't be allocated, the
>> lock itself should still be functional. As this isn't a production use
>> feature, also log a message in the event that the profiling struct can't
>> be allocated.
>>
>> Fixes: d98feda5c756 ("Make lock profiling usable again")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/include/xen/spinlock.h
>> +++ b/xen/include/xen/spinlock.h
>> @@ -103,10 +103,16 @@ struct lock_profile_qhead {
>> do { \
>> struct lock_profile *prof; \
>> prof = xzalloc(struct lock_profile); \
>> - if (!prof) break; \
>> + (s)->l = (spinlock_t)_SPIN_LOCK_UNLOCKED(prof); \
>> + if ( !prof ) \
>> + { \
>> + printk(XENLOG_WARNING \
>> + "lock profiling unavailable for %p(%d)'s " #l "\n", \
>> + s, (s)->profile_head.idx); \
>
> You'll end up with far less .rodata if you use %s and pass #l in as a
> parameter.
Well, "far less" perhaps goes a little far, as we currently have
just three use sites, but I see your point and hence will switch.
> Either way, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Thanks, Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] lockprof: don't pass name into registration function
2020-07-23 10:52 ` [PATCH 2/2] lockprof: don't pass name into registration function Jan Beulich
@ 2020-07-23 11:27 ` Andrew Cooper
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Cooper @ 2020-07-23 11:27 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: George Dunlap, Ian Jackson, Stefano Stabellini, Julien Grall, Wei Liu
On 23/07/2020 11:52, Jan Beulich wrote:
> The type uniquely identifies the associated name, hence the name fields
> can be statically initialized.
>
> Also constify not just the involved struct field, but also struct
> lock_profile's. Rather than specifying lock_profile_ancs[]' dimension at
> definition time, add a suitable build time check, such that at least
> missing tail additions to the initializer can be spotted easily.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-23 11:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-23 10:50 [PATCH 0/2] lockprof: eliminate a minor bug and a quirk Jan Beulich
2020-07-23 10:51 ` [PATCH 1/2] lockprof: don't leave locks uninitialized upon allocation failure Jan Beulich
2020-07-23 11:23 ` Andrew Cooper
2020-07-23 11:26 ` Jan Beulich
2020-07-23 10:52 ` [PATCH 2/2] lockprof: don't pass name into registration function Jan Beulich
2020-07-23 11:27 ` Andrew Cooper
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).