eventfd: change int to __u64 in eventfd_signal()
PATCH 1/1 eventfd: change int to __u64 in eventfd_signal()
From: Sha Zhengju <[email protected]>
From: Sha Zhengju <[email protected]>
eventfd_ctx->count is an __u64 counter which is allowed to reach ULLONG_MAX. Now eventfd_write() add an __u64 value to "count", but kernel side eventfd_signal() only add an int value to it. So make them consistent here.
--- fs/eventfd.c | 4 ++-- include/linux/eventfd.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
fs/eventfd.c +2 -2
@@ -51,7 +51,7 @@ struct eventfd_ctx {* * -EINVAL : The value of @n is negative. */ -int eventfd_signal (struct eventfd_ctx *ctx,int n) +__u64 eventfd_signal (struct eventfd_ctx *ctx,__u64 n) {unsigned long flags;
@@ -59,7 +59,7 @@ int eventfd_signal(struct eventfd_ctx *ctx, int n)return -EINVAL ;spin_lock_irqsave (&ctx->wqh.lock, flags);if (ULLONG_MAX - ctx->count < n) - n = (int ) (ULLONG_MAX - ctx->count); + n =ULLONG_MAX - ctx->count; ctx->count += n;if (waitqueue_active (&ctx->wqh))wake_up_locked_poll (&ctx->wqh,POLLIN );
include/linux/eventfd.h +1 -1
@@ -34,7 +34,7 @@ void eventfd_ctx_put(struct eventfd_ctx *ctx);struct file *eventfd_fget (int fd);struct eventfd_ctx *eventfd_ctx_fdget (int fd);struct eventfd_ctx *eventfd_ctx_fileget (struct file *file); -int eventfd_signal (struct eventfd_ctx *ctx,int n); +__u64 eventfd_signal (struct eventfd_ctx *ctx,__u64 n);ssize_t eventfd_ctx_read (struct eventfd_ctx *ctx,int no_wait,__u64 *cnt);int eventfd_ctx_remove_wait_queue (struct eventfd_ctx *ctx,wait_queue_t *wait,__u64 *cnt);
Signature · 1 line
1.7.4.1
Quoted 22 lines · On Thu, 12 Apr 2012 18:01:20 +0800 [email protected] wrote:
From: Sha Zhengju <[email protected]>
From: Sha Zhengju <[email protected]>
eventfd_ctx->countis an__u64counter which is allowed to reachULLONG_MAX. Noweventfd_write()add an__u64value to "count", but kernel sideeventfd_signal()only add an int value to it. So make them consistent here....
fs/eventfd.c +1
-1@@ -51,7 +51,7 @@ struct eventfd_ctx {* * -EINVAL : The value of @n is negative. */ -int eventfd_signal (struct eventfd_ctx *ctx,int n) +__u64 eventfd_signal (struct eventfd_ctx *ctx,__u64 n) {unsigned long flags;
fs/eventfd.c int eventfd_signal(struct eventfd_ctx *ctx, int n) · line 65
Quoted 8 lines
fs/eventfd.c +1
-1@@ -59,7 +59,7 @@ int eventfd_signal(struct eventfd_ctx *ctx, int n)return -EINVAL ;spin_lock_irqsave (&ctx->wqh.lock, flags);if (ULLONG_MAX - ctx->count < n) - n = (int ) (ULLONG_MAX - ctx->count); + n =ULLONG_MAX - ctx->count; ctx->count += n;if (waitqueue_active (&ctx->wqh))wake_up_locked_poll (&ctx->wqh,POLLIN );
Changing `n' to an unsigned type makes the "if (n < 0)" test a no-op.
Every in-kernel caller of eventfd_signal() passes n=1. All of them. Perhaps we can just remove that argument and hard-wire the +1 assumption into eventfd_signal().
Quoted 24 lines · Andrew Morton
Quoted 21 lines · On Thu, 12 Apr 2012 18:01:20 +0800 [email protected] wrote:
From: Sha Zhengju<[email protected]>
From: Sha Zhengju<[email protected]>
eventfd_ctx->countis an__u64counter which is allowed to reachULLONG_MAX. Noweventfd_write()add an__u64value to "count", but kernel sideeventfd_signal()only add an int value to it. So make them consistent here....
fs/eventfd.c +1
-1@@ -51,7 +51,7 @@ struct eventfd_ctx {* * -EINVAL : The value of @n is negative. */ -int eventfd_signal (struct eventfd_ctx *ctx,int n) +__u64 eventfd_signal (struct eventfd_ctx *ctx,__u64 n) {unsigned long flags;
Quoted 11 lines
fs/eventfd.c int eventfd_signal(struct eventfd_ctx *ctx, int n) · line 65
Quoted 9 lines
fs/eventfd.c +1
-1@@ -59,7 +59,7 @@ int eventfd_signal(struct eventfd_ctx *ctx, int n)return -EINVAL ;spin_lock_irqsave (&ctx->wqh.lock, flags);if (ULLONG_MAX - ctx->count< n) - n = (int ) (ULLONG_MAX - ctx->count); + n =ULLONG_MAX - ctx->count; ctx->count += n;if (waitqueue_active (&ctx->wqh))wake_up_locked_poll (&ctx->wqh,POLLIN );Changing `n' to an unsigned type makes the "if (n< 0)" test a no-op.
yeah, it's really not necessary. I slipped up..
Quoted 3 lines
Every in-kernel caller of
eventfd_signal()passes n=1. All of them. Perhaps we can just remove that argument and hard-wire the +1 assumption intoeventfd_signal().
A userspace write(2) performing on an eventfd can pass an u64 value to "count" and issue a wakeup, I think the kernel side can just be in accord with it. Though all the current in-kernel callers just pass 1 to eventfd_signal(), it doesn't mean there is no possibility of other usages in future. Actually, we're considering to encode some usefully info to "count" and sent it to userspace. :-)