Re: generic_file_write code segment in 2.2.18

["Stephen C. Tweedie" <sct@redhat.com>]

Hi,

On Thu, Jan 04, 2001 at 08:51:37AM +0100, Andi Kleen wrote:
> On Wed, Jan 03, 2001 at 09:29:48PM -0800, Asang K Dani wrote:
> 
> The code is buggy as far as I can see. copy_from_user doesn't return the 
> number of bytes copied, but the number of bytes not copied when an error
> occurs (or 0 on success).  

But in this case, _any_ non-zero number of bytes copied is a success,
because it indicates that we have dirtied a portion of the page cache.

> Correct would be:
> 
> 	
> --- linux-work/mm/filemap.c-o	Wed Jan  3 17:37:27 2001
>  		dest = (char *) page_address(page) + offset;
>  		if (dest != buf) { /* See comment in update_vm_cache_cond. */
> -			bytes -= copy_from_user(dest, buf, bytes);
> +			if (copy_from_user(dest, buf, bytes))
> +				status = -EFAULT; 
>  			flush_dcache_page(page_address(page));
>  		}
> -		status = -EFAULT;
> -		if (bytes)
> +		if (!status)
>  			status = inode->i_op->updatepage(file, page, offset, bytes, sync);

No, because then you'd be skipping the updatepage() call if we took a
fault mid-copy after copying some data.  That would imply you had
dirtied the page cache without an updatepage().

The current behaviour should just result in a short IO, which should
be fine.

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