lkml.org 
[lkml]   [2005]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] generic_file_sendpage (updated patch)
Andrew Morton wrote:
>
>>+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.
>

Ok

>>+
>>+ /* 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.
>

Hmm, yeah ... -EINVAL should do it.

>
>>+ 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.

The sendfile() mainloop is in generic_file_sendfile().
generic_file_sendpage() is called via the file_send_actor() from
do_generic_file_read(). Therefore generic_file_sendpage() is called for
every page which is written to the file. Since signals are checked
before every page this is solving the uninterruptible problem. IMHO, the
sendpage() should not be interrupted in between a single page.

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

The return value of sendfile() is not the same as the return value of
sendpage(). This is done in file_send_actor().

>
> I don't know if we want to add this feature, really. It's such a
> specialised thing.

This issue comes up every now and than. I know of at least two users:
COW links and my union mounts. I think this solution is quite good. I
tested it under qemu and it is working for me.

Jan

BTW: I'm out for the weekend now.

\
 
 \ /
  Last update: 2005-07-15 14:36    [W:0.067 / U:0.632 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site