Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1495883 > unrolled thread
| Started by | chengang@emindsoft.com.cn |
|---|---|
| First post | 2016-10-05 15:50 +0200 |
| Last post | 2016-10-06 13:10 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] uapi: linux: acct: Remove redundant type comp2_t from kernel chengang@emindsoft.com.cn - 2016-10-05 15:50 +0200
Re: [PATCH] uapi: linux: acct: Remove redundant type comp2_t from kernel Chen Gang <chengang@emindsoft.com.cn> - 2016-10-06 13:10 +0200
| From | chengang@emindsoft.com.cn |
|---|---|
| Date | 2016-10-05 15:50 +0200 |
| Subject | [PATCH] uapi: linux: acct: Remove redundant type comp2_t from kernel |
| Message-ID | <soUK5-1DV-17@gated-at.bofh.it> |
From: Chen Gang <chengang@emindsoft.com.cn>
In api itself, kernel does not use it -- it is divided into ac_etime_hi
and ac_etime_lo. So kernel side only need generate the correct
ac_etime_hi and ac_etime_lo, but need not know about comp2_t.
At present, kernel use normal u64 type for it, when kernel provdes it to
outside, kernel can translate it into ac_etime_hi and ac_etime_lo,
directly, but need not notice about comp2_t, in fact.
The patch notices about coding styles:
- Use 1 instead of 1ul, since type int is enough.
- Use white space between operator and constant or macro.
- Initialize variables directly, when declare it, since they need be
initialized and can be initialized by constant (include constant
macros).
- Remove redundant empty line.
Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
include/uapi/linux/acct.h | 6 ++++--
kernel/acct.c | 31 ++++++++++++++-----------------
2 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/include/uapi/linux/acct.h b/include/uapi/linux/acct.h
index df2f9a0..97acdd4 100644
--- a/include/uapi/linux/acct.h
+++ b/include/uapi/linux/acct.h
@@ -24,12 +24,15 @@
* comp_t is a 16-bit "floating" point number with a 3-bit base 8
* exponent and a 13-bit fraction.
* comp2_t is 24-bit with 5-bit base 2 exponent and 20 bit fraction
- * (leading 1 not stored).
+ * (leading 1 not stored). And it is described as ac_etime_hi and
+ * ac_etime_lo in kernel.
* See linux/kernel/acct.c for the specific encoding systems used.
*/
typedef __u16 comp_t;
+#ifndef __KERNEL__
typedef __u32 comp2_t;
+#endif /* __KERNEL */
/*
* accounting file record
@@ -120,5 +123,4 @@ struct acct_v3
#define AHZ (HZ)
#endif /* __KERNEL */
-
#endif /* _UAPI_LINUX_ACCT_H */
diff --git a/kernel/acct.c b/kernel/acct.c
index 74963d1..f707a10 100644
--- a/kernel/acct.c
+++ b/kernel/acct.c
@@ -338,7 +338,7 @@ static comp_t encode_comp_t(unsigned long value)
#if ACCT_VERSION == 1 || ACCT_VERSION == 2
/*
- * encode an u64 into a comp2_t (24 bits)
+ * encode an u64 into ac_etime_hi and ac_etime_lo (24 bits)
*
* Format: 5 bit base 2 exponent, 20 bits mantissa.
* The leading bit of the mantissa is not stored, but implied for
@@ -348,15 +348,15 @@ static comp_t encode_comp_t(unsigned long value)
#define MANTSIZE2 20 /* 20 bit mantissa. */
#define EXPSIZE2 5 /* 5 bit base 2 exponent. */
-#define MAXFRACT2 ((1ul << MANTSIZE2) - 1) /* Maximum fractional value. */
-#define MAXEXP2 ((1 << EXPSIZE2) - 1) /* Maximum exponent. */
+#define MAXFRACT2 ((1 << MANTSIZE2) - 1) /* Maximum fractional value. */
+#define MAXEXP2 ((1 << EXPSIZE2) - 1) /* Maximum exponent. */
-static comp2_t encode_comp2_t(u64 value)
+static void encode_ac_etime_hilo(u64 value, acct_t *ac)
{
- int exp, rnd;
+ int exp = (value > (MAXFRACT2 >> 1));
+ int rnd = 0;
+ u32 etime;
- exp = (value > (MAXFRACT2>>1));
- rnd = 0;
while (value > MAXFRACT2) {
rnd = value & 1;
value >>= 1;
@@ -373,10 +373,12 @@ static comp2_t encode_comp2_t(u64 value)
if (exp > MAXEXP2) {
/* Overflow. Return largest representable number instead. */
- return (1ul << (MANTSIZE2+EXPSIZE2-1)) - 1;
+ etime = (1 << (MANTSIZE2 + EXPSIZE2 - 1)) - 1;
} else {
- return (value & (MAXFRACT2>>1)) | (exp << (MANTSIZE2-1));
+ etime = (value & (MAXFRACT2 >> 1)) | (exp << (MANTSIZE2 - 1));
}
+ ac->ac_etime_hi = etime >> 16;
+ ac->ac_etime_lo = (u16) etime;
}
#endif
@@ -436,17 +438,12 @@ static void fill_ac(acct_t *ac)
(unsigned long) elapsed : (unsigned long) -1l);
#endif
#if ACCT_VERSION == 1 || ACCT_VERSION == 2
- {
- /* new enlarged etime field */
- comp2_t etime = encode_comp2_t(elapsed);
-
- ac->ac_etime_hi = etime >> 16;
- ac->ac_etime_lo = (u16) etime;
- }
+ /* new enlarged etime field */
+ encode_ac_etime_hilo(elapsed, ac);
#endif
do_div(elapsed, AHZ);
ac->ac_btime = get_seconds() - elapsed;
-#if ACCT_VERSION==2
+#if ACCT_VERSION == 2
ac->ac_ahz = AHZ;
#endif
--
1.9.3
[toc] | [next] | [standalone]
| From | Chen Gang <chengang@emindsoft.com.cn> |
|---|---|
| Date | 2016-10-06 13:10 +0200 |
| Subject | Re: [PATCH] uapi: linux: acct: Remove redundant type comp2_t from kernel |
| Message-ID | <speIO-7Nv-35@gated-at.bofh.it> |
| In reply to | #1495883 |
Hello:
For me if necessary, can add additional comments for it:
If kernel will really need comp2_t in the future (I guess not), kernel
can define it as a union with structures (need not must be the same as
the user mode), which will be easier for kernel using. e.g.
#pragma push(1)
typedef union {
#if ACCT_BYTEORDER == 0x80 /* big endian */
struct {
__u8 pad0;
__u8 ac_etime_hi;
__u16 ac_etime_lo;
};
struct {
__u32 pad1 : 8;
__u32 exp : 5;
__u32 mant : 19;
};
#else
struct {
__u16 ac_etime_lo;
__u8 ac_etime_hi;
};
struct {
__u32 mant : 19;
__u32 exp : 5;
};
#endif
__u32 v;
} comp2_t;
#pragma pop()
Some members may be not interested in bits definition (mant, exp), I
guess, it is because of performance reason (but I am not quite sure).
But ac_etime_lo and ac_etime_hi are useful.
Although this definition may be better than u32, we can not change the
uapi -- which may cause user mode application generate building break,
e.g. some informal usage -- use u32 directly for shift operations.
Thanks.
On 10/5/16 21:40, chengang@emindsoft.com.cn wrote:
> From: Chen Gang <chengang@emindsoft.com.cn>
>
> In api itself, kernel does not use it -- it is divided into ac_etime_hi
> and ac_etime_lo. So kernel side only need generate the correct
> ac_etime_hi and ac_etime_lo, but need not know about comp2_t.
>
> At present, kernel use normal u64 type for it, when kernel provdes it to
> outside, kernel can translate it into ac_etime_hi and ac_etime_lo,
> directly, but need not notice about comp2_t, in fact.
>
> The patch notices about coding styles:
>
> - Use 1 instead of 1ul, since type int is enough.
>
> - Use white space between operator and constant or macro.
>
> - Initialize variables directly, when declare it, since they need be
> initialized and can be initialized by constant (include constant
> macros).
>
> - Remove redundant empty line.
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
> include/uapi/linux/acct.h | 6 ++++--
> kernel/acct.c | 31 ++++++++++++++-----------------
> 2 files changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/include/uapi/linux/acct.h b/include/uapi/linux/acct.h
> index df2f9a0..97acdd4 100644
> --- a/include/uapi/linux/acct.h
> +++ b/include/uapi/linux/acct.h
> @@ -24,12 +24,15 @@
> * comp_t is a 16-bit "floating" point number with a 3-bit base 8
> * exponent and a 13-bit fraction.
> * comp2_t is 24-bit with 5-bit base 2 exponent and 20 bit fraction
> - * (leading 1 not stored).
> + * (leading 1 not stored). And it is described as ac_etime_hi and
> + * ac_etime_lo in kernel.
> * See linux/kernel/acct.c for the specific encoding systems used.
> */
>
> typedef __u16 comp_t;
> +#ifndef __KERNEL__
> typedef __u32 comp2_t;
> +#endif /* __KERNEL */
>
> /*
> * accounting file record
> @@ -120,5 +123,4 @@ struct acct_v3
> #define AHZ (HZ)
> #endif /* __KERNEL */
>
> -
> #endif /* _UAPI_LINUX_ACCT_H */
> diff --git a/kernel/acct.c b/kernel/acct.c
> index 74963d1..f707a10 100644
> --- a/kernel/acct.c
> +++ b/kernel/acct.c
> @@ -338,7 +338,7 @@ static comp_t encode_comp_t(unsigned long value)
>
> #if ACCT_VERSION == 1 || ACCT_VERSION == 2
> /*
> - * encode an u64 into a comp2_t (24 bits)
> + * encode an u64 into ac_etime_hi and ac_etime_lo (24 bits)
> *
> * Format: 5 bit base 2 exponent, 20 bits mantissa.
> * The leading bit of the mantissa is not stored, but implied for
> @@ -348,15 +348,15 @@ static comp_t encode_comp_t(unsigned long value)
>
> #define MANTSIZE2 20 /* 20 bit mantissa. */
> #define EXPSIZE2 5 /* 5 bit base 2 exponent. */
> -#define MAXFRACT2 ((1ul << MANTSIZE2) - 1) /* Maximum fractional value. */
> -#define MAXEXP2 ((1 << EXPSIZE2) - 1) /* Maximum exponent. */
> +#define MAXFRACT2 ((1 << MANTSIZE2) - 1) /* Maximum fractional value. */
> +#define MAXEXP2 ((1 << EXPSIZE2) - 1) /* Maximum exponent. */
>
> -static comp2_t encode_comp2_t(u64 value)
> +static void encode_ac_etime_hilo(u64 value, acct_t *ac)
> {
> - int exp, rnd;
> + int exp = (value > (MAXFRACT2 >> 1));
> + int rnd = 0;
> + u32 etime;
>
> - exp = (value > (MAXFRACT2>>1));
> - rnd = 0;
> while (value > MAXFRACT2) {
> rnd = value & 1;
> value >>= 1;
> @@ -373,10 +373,12 @@ static comp2_t encode_comp2_t(u64 value)
>
> if (exp > MAXEXP2) {
> /* Overflow. Return largest representable number instead. */
> - return (1ul << (MANTSIZE2+EXPSIZE2-1)) - 1;
> + etime = (1 << (MANTSIZE2 + EXPSIZE2 - 1)) - 1;
> } else {
> - return (value & (MAXFRACT2>>1)) | (exp << (MANTSIZE2-1));
> + etime = (value & (MAXFRACT2 >> 1)) | (exp << (MANTSIZE2 - 1));
> }
> + ac->ac_etime_hi = etime >> 16;
> + ac->ac_etime_lo = (u16) etime;
> }
> #endif
>
> @@ -436,17 +438,12 @@ static void fill_ac(acct_t *ac)
> (unsigned long) elapsed : (unsigned long) -1l);
> #endif
> #if ACCT_VERSION == 1 || ACCT_VERSION == 2
> - {
> - /* new enlarged etime field */
> - comp2_t etime = encode_comp2_t(elapsed);
> -
> - ac->ac_etime_hi = etime >> 16;
> - ac->ac_etime_lo = (u16) etime;
> - }
> + /* new enlarged etime field */
> + encode_ac_etime_hilo(elapsed, ac);
> #endif
> do_div(elapsed, AHZ);
> ac->ac_btime = get_seconds() - elapsed;
> -#if ACCT_VERSION==2
> +#if ACCT_VERSION == 2
> ac->ac_ahz = AHZ;
> #endif
>
>
--
Chen Gang (陈刚)
Managing Natural Environments is the Duty of Human Beings.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web