From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Layton Subject: Re: fsx-linux failing with latest cifs-2.6 git tree Date: Fri, 21 Nov 2008 15:50:35 -0500 Message-ID: <20081121155035.2fffb0a9@tleilax.poochiereds.net> References: <20081121105613.09a8cb8e@tleilax.poochiereds.net> <524f69650811210820s549de2bah3181cbc0c5633091@mail.gmail.com> <20081121112249.0b408b55@tleilax.poochiereds.net> <524f69650811210846q7502fd99m6f4d335bb6ac1b65@mail.gmail.com> <524f69650811211109w659e5decoa34a8e0f907772a3@mail.gmail.com> <524f69650811211113q4fffcc70of88cb85db531c358@mail.gmail.com> <1227296476.20845.8.camel@norville.austin.ibm.com> <524f69650811211218v78295682lcf6dce842327b097@mail.gmail.com> <524f69650811211238x3c81d899r225f46776aeb52f1@mail.gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: "Dave Kleikamp" , "Nick Piggin" , pbadari@us.ibm.com, linux-fsdevel , "linux-cifs-client@lists.samba.org" To: "Steve French" Return-path: Received: from mx2.redhat.com ([66.187.237.31]:44722 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751026AbYKUUuu (ORCPT ); Fri, 21 Nov 2008 15:50:50 -0500 In-Reply-To: <524f69650811211238x3c81d899r225f46776aeb52f1@mail.gmail.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, 21 Nov 2008 14:38:18 -0600 "Steve French" wrote: > Fix attached. > > Shaggy/Jeff/Nick etc. do you want to review/ack it since it is late in the rc? > > On Fri, Nov 21, 2008 at 2:18 PM, Steve French wrote: > > Looks like the following change to cifs_write_begin does fix it ... > > thanks Shaggy ... > > > > @@ -2062,8 +2074,10 @@ static int cifs_write_begin(struct file *file, > > struct address_space *mapping, > > { > > pgoff_t index = pos >> PAGE_CACHE_SHIFT; > > loff_t offset = pos & (PAGE_CACHE_SIZE - 1); > > + loff_t page_start = pos & PAGE_MASK; > > > > cFYI(1, ("write_begin from %lld len %d", (long long)pos, len)); > > @@ -2081,13 +2095,14 @@ static int cifs_write_begin(struct file *file, > > struct address_space *mapping, > > int rc; > > > > /* might as well read a page, it is fast enough */ > > - rc = cifs_readpage_worker(file, *pagep, &offset); > > + rc = cifs_readpage_worker(file, *pagep, &page_start); > > > > /* we do not need to pass errors back > > e.g. if we do not have read access to the file > > because cifs_write_end will attempt synchronous writes > > -- shaggy */ > > > > > > On Fri, Nov 21, 2008 at 1:41 PM, Dave Kleikamp > > wrote: > >> On Fri, 2008-11-21 at 13:13 -0600, Steve French wrote: > >>> Looks like this section of code is wrong in cifs_write_begin: > >>> > >>> if ((file->f_flags & O_ACCMODE) != O_WRONLY) { > >>> int rc; > >>> > >>> /* might as well read a page, it is fast enough */ > >>> rc = cifs_readpage_worker(file, *pagep, &offset); > >>> > >>> /* we do not need to pass errors back > >>> e.g. if we do not have read access to the file > >>> because cifs_write_end will attempt synchronous writes > >>> -- shaggy */ > >>> > >>> > >>> We see a case in which a write begins at offset 0x2e42f but the range > >>> immediately before it is unitialized in write_begin > >>> > >>> shouldn't we be doing a read of the whole page? > >> > >> What cifs_write_begin() passes in as offset is completely wrong. It > >> should be the file offset of the beginning of the page rather than some > >> offset within the page. > >> -- > >> David Kleikamp > >> IBM Linux Technology Center > >> Good catch... Acked-by: Jeff Layton