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

J. Bruce Fields
2009-07-10T17:45:16Z

On Fri, Jul 10, 2009 at 11:57:02AM -0400, Peter Staubach wrote:
> 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.

See Documentation/email-clients.txt. (It has an entry for Thunderbird,
for example.)

--b.

>
>>> +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;
>>> +}
>>> +
>>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html