Messages in this thread | | | Subject | Re: [patch 1/3] new timerfd API - new timerfd API | From | Thomas Gleixner <> | Date | Mon, 24 Sep 2007 10:26:04 +0200 |
| |
Davide,
On Sun, 2007-09-23 at 15:49 -0700, Davide Libenzi wrote: > This is the new timerfd API as it is implemented by the following patch: > --- > fs/compat.c | 32 ++++++- > fs/timerfd.c | 199 ++++++++++++++++++++++++++++++----------------- > include/linux/compat.h | 7 + > include/linux/syscalls.h | 7 + > 4 files changed, 168 insertions(+), 77 deletions(-) > > Index: linux-2.6.mod/fs/timerfd.c > =================================================================== > --- linux-2.6.mod.orig/fs/timerfd.c 2007-09-23 15:18:09.000000000 -0700 > +++ linux-2.6.mod/fs/timerfd.c 2007-09-23 15:25:55.000000000 -0700 > @@ -23,15 +23,17 @@ > > struct timerfd_ctx { > struct hrtimer tmr; > + int clockid; > ktime_t tintv; > wait_queue_head_t wqh; > int expired; > + u64 ticks; > };
Can you please restructure the struct in a way which does not result in padding by the compiler ?
struct timerfd_ctx { struct hrtimer tmr; ktime_t tintv; wait_queue_head_t wqh; u64 ticks; int expired; int clockid; };
> + ticks += (u64) > hrtimer_forward(&ctx->tmr, > hrtimer_cb_get_time(&ctx->tmr),
You need to use ctx->tmr.base->get_time() here, otherwise you might read a stale time value (in case that CONFIG_HIGH_RES_TIMERS is off).
> - ctx->tintv); > + ctx->tintv) - 1; > hrtimer_restart(&ctx->tmr); > > +asmlinkage long sys_timerfd_create(int clockid) > { > - int error; > + int error, ufd; > struct timerfd_ctx *ctx; > struct file *file; > struct inode *inode; > - struct itimerspec ktmr; > - > - if (copy_from_user(&ktmr, utmr, sizeof(ktmr))) > - return -EFAULT; > > if (clockid != CLOCK_MONOTONIC && > clockid != CLOCK_REALTIME) > return -EINVAL; > + > + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + > + init_waitqueue_head(&ctx->wqh); > + ctx->clockid = clockid; > + hrtimer_init(&ctx->tmr, clockid, HRTIMER_MODE_ABS); > + > + error = anon_inode_getfd(&ufd, &inode, &file, "[timerfd]", > + &timerfd_fops, ctx); > + if (error) > + goto err_kfree_ctx; > + > + return ufd; > + > +err_kfree_ctx: > + kfree(ctx); > + return error;
You really can avoid the goto here.
tglx
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |