On Fri, 30 Mar 2007, Andrew Morton wrote:
> > +struct eventfd_ctx {
> > + spinlock_t lock;
> > + wait_queue_head_t wqh;
> > + __u64 count;
> > +};
>
> Again, can we borrow wqh.lock?
>
> `count' needs documentation - these things are key to understanding the
> code.
Added.
> > + */
>
> So it is the caller's responsibility to ensure that *file refers to an
> eventfd file?
In which function? I lost you ...
> > +int eventfd_signal(struct file *file, int n)
> > +{
> > + struct eventfd_ctx *ctx = file->private_data;
> > + unsigned long flags;
> > +
> > + if (n < 0)
> > + return -EINVAL;
> > + spin_lock_irqsave(&ctx->lock, flags);
> > + if (ULLONG_MAX - ctx->count < n)
> > + n = (int) (ULLONG_MAX - ctx->count);
> > + ctx->count += n;
> > + if (waitqueue_active(&ctx->wqh))
> > + wake_up_locked(&ctx->wqh);
> > + spin_unlock_irqrestore(&ctx->lock, flags);
> > +
> > + return n;
> > +}
>
> Neither the incoming arg (usefully named "n") nor the return value are
> documented.
Documented now.
> Needs interface documentation, please. Even the changelog doesn't tell us
> what an EAGAIN return from read() means.
I'll be adding the errno documentation to all of them.
> > +static ssize_t eventfd_write(struct file *file, const char __user *buf,
> > size_t count,
> > + loff_t *ppos)
> > +{
> > + struct eventfd_ctx *ctx = file->private_data;
> > + ssize_t res;
> > + __u64 ucnt;
> > + DECLARE_WAITQUEUE(wait, current);
> > +
> > + if (count < sizeof(ucnt))
> > + return -EINVAL;
> > + if (get_user(ucnt, (const __u64 __user *) buf))
> > + return -EFAULT;
>
> Some architectures do not implement 64-bit get_user()
copy_from_user it is, then ...
- Davide
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
