Re: [PATCH] relayfs redux, part 3

[Andi Kleen]

Just a minor comment, not reading it all. 

On Fri, Feb 04, 2005 at 02:17:37PM -0600, Tom Zanussi wrote:
_write - write data into the channel
> + *	@chan: relay channel
> + *	@data: data to be written
> + *	@length: number of bytes to write
> + *
> + *	Returns the number of bytes written, 0 if full.
> + *
> + *	Writes data into the current cpu's channel buffer.  irq safe.

This needs a better comment on the allowed contexts. 

> + */
> +static inline unsigned relay_write(struct rchan *chan,
> +				   const void *data,
> +				   unsigned length)
> +{
> +	unsigned long flags;
> +	struct rchan_buf *buf = relay_get_buf(chan, smp_processor_id());

Needs to be inside the local_irq_save of course.

> +
> +	local_irq_save(flags);
> +	if (unlikely(buf->offset + length > chan->subbuf_size))
> +		length = relay_switch_subbuf(buf, length);
> +	memcpy(buf->data + buf->offset, data, length);
> +	buf->offset += length;
> +	local_irq_restore(flags);
> + 
> +	return length;

Is there any useful user case for returning length here? 
(e.g. are users likely to handle errors? I doubt it somehow) 

If not I would eliminate it.

> +}
> +
> +/**
> + *	__relay_write - write data into the channel
> + *	@chan: relay channel
> + *	@data: data to be written
> + *	@length: number of bytes to write
> + *
> + *	Returns the number of bytes written, 0 if full.
> + *
> + *	Writes data into the current cpu's channel buffer.  Preempt safe.

And this needs more comments on the context too.

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/