From: Trond Myklebust Subject: Re: [PATCH v2] read-modify-write page updating Date: Thu, 09 Jul 2009 11:39:32 -0400 Message-ID: <1247153972.5766.15.camel@heimdal.trondhjem.org> References: <49FA0CE8.9090706@redhat.com> <1241126587.15476.62.camel@heimdal.trondhjem.org> <1243615595.7155.48.camel@heimdal.trondhjem.org> <1243618500.7155.56.camel@heimdal.trondhjem.org> <1243686363.5209.16.camel@heimdal.trondhjem.org> <1243963631.4868.124.camel@heimdal.trondhjem.org> <18982.41770.293636.786518@fisica.ufpr.br> <1244049027.5603.5.camel@heimdal.trondhjem.org> <1244138698.5203.59.camel@heimdal.trondhjem.org> <4A428482.60403@redhat.com> <4A55FAC8.5040309@redhat.com> Mime-Version: 1.0 Content-Type: text/plain Cc: Brian R Cowan , linux-nfs@vger.kernel.org To: Peter Staubach Return-path: Received: from mail-out2.uio.no ([129.240.10.58]:33168 "EHLO mail-out2.uio.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757182AbZGIPjj (ORCPT ); Thu, 9 Jul 2009 11:39:39 -0400 In-Reply-To: <4A55FAC8.5040309@redhat.com> Sender: linux-nfs-owner@vger.kernel.org List-ID: On Thu, 2009-07-09 at 10:12 -0400, Peter Staubach wrote: > Signed-off-by: Peter Staubach Please could you send such patches as inline, rather than as attachments. It makes it harder to comment on the patch contents... > +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... > + pglen && /* valid bytes of file? */ > + (end < pglen || offset)) /* replace all valid bytes? */ > + return 1; > + return 0; > +} > +