Re: [PATCH] relayfs redux, part 3

[Andi Kleen <ak@muc.de>]

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