Re: Bugs on Linux 2.6.18-rc2 sg code?

[Douglas Gilbert <dougg@torque.net>]

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

Douglas Gilbert wrote:
> Fajun Chen wrote:
>> Hi Folks,
>>
>> I use ATA pass through via sg ioctl interface for data read/write.
>> Linux 2.6.18-rc2 patched with Jeff Garzik's libata git patch was
>> running on ARM IOP80321 board. The HBA Sil3124 was used.
>>
>> Two problems were observed:
>> 1. sg mmap bug?
>>    My test program could not write data correctly to the mmapped
>> buffer in the user space.      The program did a read immediately
>> after a write and the data mismatches.  Swapped the sg_vma_nopage()
>> function with the one in 2.6.15.4 release fixed the problem. So this
>> appears to be a wrong change to the sg mmap code in 2.6.18-rc2
>> release.
> 
> Thanks for the report. I can confirm that mmap-ed IO
> in the sg driver is broken. Simply reading 16 blocks
> from some arbitrary offset with sg_dd and sgm_dd
> and comparing the fetched data shows mismatches starting
> above the first page (i.e. above byte offset 4096 on
> i386).
> 
> Your point about the change to sg_vma_nopage() between
> lk 2.6.15 and lk 2.6.16 also seems to be correct.
> The most indented part of that function has been
> changed from incrementing the change count on the
> reported page (as indicated by 'offset') in a
> compound page allocation to ignoring the 'offset'
> and incrementing the page count on the first page
> in a compound page allocation.

Fajun,
Could you please try the patch below and report if it fixes
your sg mmap problem. The patch is against lk 2.6.18-rc4
and I assume it will apply against "rc2" as well.
The patch re-instates the former logic and fixes the
problem in my tests.

Doug Gilbert

[-- Attachment #2: sg2618rc4nopage.diff --]
[-- Type: text/x-patch, Size: 950 bytes --]

--- linux/drivers/scsi/sg.c	2006-07-16 08:19:19.000000000 -0400
+++ linux/drivers/scsi/sg.c2618rc4nopage	2006-08-18 23:40:15.000000000 -0400
@@ -18,8 +18,8 @@
  *
  */
 
-static int sg_version_num = 30533;	/* 2 digits for each component */
-#define SG_VERSION_STR "3.5.33"
+static int sg_version_num = 30534;	/* 2 digits for each component */
+#define SG_VERSION_STR "3.5.34"
 
 /*
  *  D. P. Gilbert (dgilbert@interlog.com, dougg@triode.net.au), notes:
@@ -60,7 +60,7 @@
 
 #ifdef CONFIG_SCSI_PROC_FS
 #include <linux/proc_fs.h>
-static char *sg_version_date = "20050908";
+static char *sg_version_date = "20060818";
 
 static int sg_proc_init(void);
 static void sg_proc_cleanup(void);
@@ -1164,7 +1164,7 @@
 		len = vma->vm_end - sa;
 		len = (len < sg->length) ? len : sg->length;
 		if (offset < len) {
-			page = sg->page;
+			page = virt_to_page(page_address(sg->page) + offset);
 			get_page(page);	/* increment page count */
 			break;
 		}