Re: [PATCH v2] read-modify-write page updating

[Peter Staubach <staubach@redhat.com>]

Trond Myklebust wrote:
> On Thu, 2009-07-09 at 10:12 -0400, Peter Staubach wrote:
>
>   
>> Signed-off-by: Peter Staubach <staubach@redhat.com>
>>     
>
> Please could you send such patches as inline, rather than as
> attachments. It makes it harder to comment on the patch contents...
>
>   

I will investigate how to do this.

>> +static int nfs_want_read_modify_write(struct file *file, struct page *page,
>> +			loff_t pos, unsigned len)
>> +{
>> +	unsigned int pglen = nfs_page_length(page);
>> +	unsigned int offset = pos & (PAGE_CACHE_SIZE - 1);
>> +	unsigned int end = offset + len;
>> +
>> +	if ((file->f_mode & FMODE_READ) &&	/* open for read? */
>> +	    !PageUptodate(page) &&		/* Uptodate? */
>> +	    !PageDirty(page) &&			/* Dirty already? */
>> +	    !PagePrivate(page) &&		/* i/o request already? */
>>     
>
> I don't think you need the PageDirty() test. These days we should be
> guaranteed to always have PagePrivate() set whenever PageDirty() is
> (although the converse is not true). Anything else would be a bug...
>
>   

Okie doke.  It seemed to me that this should be true, but it was
safer to leave both tests.

I will remove that PageDirty test, retest, and then send another
version of the patch.  I will be out next week, so it will take a
couple of weeks.

    Thanx...

       ps

>> +	    pglen &&				/* valid bytes of file? */
>> +	    (end < pglen || offset))		/* replace all valid bytes? */
>> +		return 1;
>> +	return 0;
>> +}
>> +
>>     
>
>