Re: Fwd: fsx-linux failing with latest cifs-2.6 git tree

["Steve French" <smfrench@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2352 bytes --]

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 <smfrench@gmail.com> 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
> <shaggy@linux.vnet.ibm.com> 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
>>
>>
>
>
>
> --
> Thanks,
>
> Steve
>



-- 
Thanks,

Steve

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fsx-fail-fix.patch --]
[-- Type: text/x-diff; name=fsx-fail-fix.patch, Size: 852 bytes --]

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index b691b89..726d437 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2061,7 +2061,7 @@ static int cifs_write_begin(struct file *file, struct address_space *mapping,
 			struct page **pagep, void **fsdata)
 {
 	pgoff_t index = pos >> PAGE_CACHE_SHIFT;
-	loff_t offset = pos & (PAGE_CACHE_SIZE - 1);
+	loff_t offset_of_page_start = pos & PAGE_MASK;
 
 	cFYI(1, ("write_begin from %lld len %d", (long long)pos, len));
 
@@ -2081,7 +2081,7 @@ 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, &offset_of_page_start);
 
 		/* we do not need to pass errors back
 		   e.g. if we do not have read access to the file

[-- Attachment #3: Type: text/plain, Size: 172 bytes --]

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@lists.samba.org
https://lists.samba.org/mailman/listinfo/linux-cifs-client