Re: [PATCH] generic_file_sendpage

[Andrew Morton <akpm@osdl.org>]

Jan Blunck <j.blunck@tu-harburg.de> wrote:
>
> This is a generic sendpage() for regular files.
> 

> +static inline size_t
> +filemap_copy_from_kernel(struct page *page, unsigned long offset,
> +			 const char *buf, unsigned bytes)
> +{
> +	char *kaddr;
> +
> +	kaddr = kmap(page);
> +	memcpy(kaddr + offset, buf, bytes);
> +	kunmap(page);

Use kmap_atomic().

Move the flush_dcache_page() into here.

> +static ssize_t
> +__generic_kernel_file_write(struct file *file, const char *buf,
> +			    size_t count, loff_t *ppos)
> +{
> +	struct address_space * mapping = file->f_mapping;
> +	struct address_space_operations *a_ops = mapping->a_ops;
> +	struct inode 	*inode = mapping->host;
> +	long		status = 0;
> +	loff_t		pos;
> +	struct page	*page;
> +	struct page	*cached_page = NULL;
> +	const int	isblk = S_ISBLK(inode->i_mode);
> +	ssize_t		written;
> +	ssize_t		err;
> +	size_t		bytes;
> +	struct pagevec	lru_pvec;
> +
> +	/* There is no sane reason to use O_DIRECT */
> +	BUG_ON(file->f_flags & O_DIRECT);

err, this seems like an easy way for people to make the kernel go BUG.

> +	if (unlikely(signal_pending(current)))
> +		return -EINTR;

This doesn't help.  The reason we've avoided file-to-file sendfile() is
that it can cause applications to get uninterruptibly stuck in the kernel
for ages.  This code doesn't solve that problem.  It needs to handle
signal_pending() inside the main loop.

And it probably needs to return a sane value (number of bytes copied)
rather than -EINTR.

I don't know if we want to add this feature, really.  It's such a
specialised thing.
-
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/