On 2019/10/8 20:11, Peter Zijlstra wrote:
On Tue, Oct 01, 2019 at 08:33:45PM +0800, Like Xu wrote:
Hi Peter,
On 2019/10/1 16:23, Peter Zijlstra wrote:
On Mon, Sep 30, 2019 at 03:22:57PM +0800, Like Xu wrote:
+ union {
+ u8 event_count :7; /* the total number of created perf_events */
+ bool enable_cleanup :1;
That's atrocious, don't ever create a bitfield with base _Bool.
I saw this kind of usages in the tree such as "struct
arm_smmu_master/tipc_mon_state/regmap_irq_chip".
Because other people do tasteless things doesn't make it right.
I'm not sure is this your personal preference or is there a technical
reason such as this usage is not incompatible with union syntax?
Apparently it 'works', so there is no hard technical reason, but
consider that _Bool is specified as an integer type large enough to
store the values 0 and 1, then consider it as a base type for a
bitfield. That's just disguisting.
It's reasonable. Thanks.
Now, I suppose it 'works', but there is no actual benefit over just
using a single bit of any other base type.
My design point is to save a little bit space without introducing
two variables such as "int event_count & bool enable_cleanup".
Your design is questionable, the structure is _huge_, and your union has
event_count:0 and enable_cleanup:0 as the same bit, which I don't think
was intentional.
Did you perhaps want to write:
struct {
u8 event_count : 7;
u8 event_cleanup : 1;
};
which has a total size of 1 byte and uses the low 7 bits as count and the
msb as cleanup.
Yes, my union here is wrong and let me fix it in the next version.
Also, the structure has plenty holes to stick proper variables in
without further growing it.
Yes, we could benefit from it.
By the way, is the lazy release mechanism looks reasonable to you?
I've no idea how it works.. I don't know much about virt.
