Areca driver recap + status

Areca driver recap + status

[Robert Mueller]

Hi

As happy users of areca raid cards, we're keen to see a stable driver make 
it into the next kernel release. I thought I might quickly recap what's 
happened with the driver over the last 6 months to see where things are at:

The driver went into 2.6.11-rc3-mm1 here:
http://marc.theaimsgroup.com/?l=linux-kernel&m=110754432622498&w=2

A thread about overall style + api problems here:
http://marc.theaimsgroup.com/?l=linux-kernel&m=113525798005993&w=2

Some specific comments + patches from Randy and Chris about problems:
http://marc.theaimsgroup.com/?l=linux-scsi&m=113597128115672&w=2
http://marc.theaimsgroup.com/?l=linux-kernel&m=114045972531035&w=2

An update from erich went into 2.6.16-rc6-mm2 to address a number of the 
problems:
http://marc.theaimsgroup.com/?l=linux-kernel&m=114282535310724&w=2

End of thread about problem ARCMSR_MAX_XFER_SECTORS of 4096 vs 512:
http://marc.theaimsgroup.com/?l=linux-kernel&m=114602195422443&w=2

Comment from Andrew about helping to get the driver in:
http://marc.theaimsgroup.com/?l=linux-kernel&m=114945466405532&w=2

Though there did still seem to be quite a few things missing:
http://marc.theaimsgroup.com/?l=linux-scsi&m=114556263632510&w=2

General comment on areca driver is "improving", we seem to need a sound 
"todo" list:
http://marc.theaimsgroup.com/?l=linux-kernel&m=114949920416720&w=2

Some of those remaining problems seem pretty serious concerns, though I 
don't really know any of the technical details. The latest driver from -mm 
seems to be working well for us at the moment. Is the general concensus that 
the driver should go in despite these issues, and they can be fixed over 
time? Or are these still show stoppers?

Thanks

Rob

Re: Areca driver recap + status

[Andrew Morton]

On Thu, 22 Jun 2006 14:18:23 +1000
"Robert Mueller" <robm@fastmail.fm> wrote:

> The driver went into 2.6.11-rc3-mm1 here:
> http://marc.theaimsgroup.com/?l=linux-kernel&m=110754432622498&w=2

One and a half years.

Would the world end if we just merged the dang thing?

Re: Areca driver recap + status

[Jeff Garzik]

Andrew Morton wrote:
> On Thu, 22 Jun 2006 14:18:23 +1000
> "Robert Mueller" <robm@fastmail.fm> wrote:
> 
>> The driver went into 2.6.11-rc3-mm1 here:
>> http://marc.theaimsgroup.com/?l=linux-kernel&m=110754432622498&w=2
> 
> One and a half years.
> 
> Would the world end if we just merged the dang thing?

No.  Please do...

	Jeff

Re: Areca driver recap + status

[James Bottomley]

On Wed, 2006-06-21 at 22:28 -0700, Andrew Morton wrote: 
> On Thu, 22 Jun 2006 14:18:23 +1000
> "Robert Mueller" <robm@fastmail.fm> wrote:
> 
> > The driver went into 2.6.11-rc3-mm1 here:
> > http://marc.theaimsgroup.com/?l=linux-kernel&m=110754432622498&w=2
> 
> One and a half years.
> 
> Would the world end if we just merged the dang thing?

Not the world perhaps, but I'm unwilling to concede that if a driver
author is given a list of major issues and does nothing, then the driver
should go in after everyone gets impatient.

The rules for inclusion are elastic and include broad leeway for good
behaviour, but this would stretch the elasticity way beyond breaking
point.

The list of issues is here:

http://marc.theaimsgroup.com/?l=linux-scsi&m=114556263632510

Most of the serious stuff is fixed with the exception of:

- sysfs has more than one value per file
- BE platform support
- PAE (cast of dma_addr_t to unsigned long) issues.
- SYNCHRONIZE_CACHE is ignored.  This is wrong.  The sync cache in the
shutdown notifier isn't sufficient.

At least the sysfs files have to be fixed before it goes in ... unless
you want to be lynched by Greg?

What I could do is set up a holding tree for all the fixed ... but -mm
is doing a good job of that at the moment.

James

Re: Areca driver recap + status

[Andrew Morton]

On Mon, 26 Jun 2006 09:48:58 -0500
James Bottomley <James.Bottomley@SteelEye.com> wrote:

> On Wed, 2006-06-21 at 22:28 -0700, Andrew Morton wrote: 
> > On Thu, 22 Jun 2006 14:18:23 +1000
> > "Robert Mueller" <robm@fastmail.fm> wrote:
> > 
> > > The driver went into 2.6.11-rc3-mm1 here:
> > > http://marc.theaimsgroup.com/?l=linux-kernel&m=110754432622498&w=2
> > 
> > One and a half years.
> > 
> > Would the world end if we just merged the dang thing?
> 
> Not the world perhaps, but I'm unwilling to concede that if a driver
> author is given a list of major issues and does nothing, then the driver
> should go in after everyone gets impatient.
> 
> The rules for inclusion are elastic and include broad leeway for good
> behaviour, but this would stretch the elasticity way beyond breaking
> point.
> 
> The list of issues is here:
> 
> http://marc.theaimsgroup.com/?l=linux-scsi&m=114556263632510

I'm under the impression that Erich is under the impression that they've all
been addressed.

> Most of the serious stuff is fixed with the exception of:
> 
> - sysfs has more than one value per file
> - BE platform support
> - PAE (cast of dma_addr_t to unsigned long) issues.
> - SYNCHRONIZE_CACHE is ignored.  This is wrong.  The sync cache in the
> shutdown notifier isn't sufficient.

So this is progress.

Erich, can you please fix these things up and then re-review the issues
list which I'm maintaining in
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.17/2.6.17-mm2/broken-out/areca-raid-linux-scsi-driver.patch,
make sure that everything has been addressed?

If there are some things in those lists which you cannot/will not address
then please identify them and give us the reasoning behind your decision.

Re: Areca driver recap + status

[Robert Mueller]

> - PAE (cast of dma_addr_t to unsigned long) issues.

Can you explain a bit what this is about and what the effect is? It's just 
that we've been using the driver (older version from the areca website) in a 
PAE kernel on machines with 8G of memory and haven't had a problem yet 
(running high IO load for several weeks) but this sounds like it 
might/should cause corruption or crashing in this situation?

Rob

Re: Areca driver recap + status

[Dax Kelson]

On Mon, 2006-06-26 at 09:48 -0500, James Bottomley wrote:

> Not the world perhaps, but I'm unwilling to concede that if a driver
> author is given a list of major issues and does nothing, then the driver
> should go in after everyone gets impatient.

That isn't accurate or fair. Erich has submitted large patches to
address issues. That hardly qualifies as "does nothing".

> The list of issues is here:
> 
> http://marc.theaimsgroup.com/?l=linux-scsi&m=114556263632510
> 
> Most of the serious stuff is fixed with the exception of:
> 
> - sysfs has more than one value per file
> - BE platform support
> - PAE (cast of dma_addr_t to unsigned long) issues.
> - SYNCHRONIZE_CACHE is ignored.  This is wrong.  The sync cache in the
> shutdown notifier isn't sufficient.
> 
> At least the sysfs files have to be fixed before it goes in ... unless
> you want to be lynched by Greg?

Thanks for the new list. Erich has been eager to get work on any
remaining blockers.

It would have been nice to have gotten it back on May 19th, they might
have been resolved by now.

http://marc.theaimsgroup.com/?l=linux-kernel&m=114801926400287&w=2

Re: Areca driver recap + status

[erich]

Dear Robert Mueller,

Does arcmsr still has more than one value per file issue on it?
Maybe I am miss-understand the means of one value per file.
Please tell me the issue, thanks.
About the BE platform, Areca's user on linux sparc platform had ran this 
driver successfuly.
But I attempt to install linux system on ppc platform and run this driver 
for more long time testing this day.
I will patch PAE issue on pci_map_single again and handle SYNCHRONIZE_CACHE 
for your request.

Best Regards
Erich Chen
----- Original Message ----- 
From: "James Bottomley" <James.Bottomley@SteelEye.com>
To: "Andrew Morton" <akpm@osdl.org>
Cc: <rdunlap@xenotime.net>; <hch@infradead.org>; <erich@areca.com.tw>; 
<brong@fastmail.fm>; <dax@gurulabs.com>; <linux-kernel@vger.kernel.org>; 
<linux-scsi@vger.kernel.org>; "Robert Mueller" <robm@fastmail.fm>
Sent: Monday, June 26, 2006 10:48 PM
Subject: Re: Areca driver recap + status


> On Wed, 2006-06-21 at 22:28 -0700, Andrew Morton wrote:
>> On Thu, 22 Jun 2006 14:18:23 +1000
>> "Robert Mueller" <robm@fastmail.fm> wrote:
>>
>> > The driver went into 2.6.11-rc3-mm1 here:
>> > http://marc.theaimsgroup.com/?l=linux-kernel&m=110754432622498&w=2
>>
>> One and a half years.
>>
>> Would the world end if we just merged the dang thing?
>
> Not the world perhaps, but I'm unwilling to concede that if a driver
> author is given a list of major issues and does nothing, then the driver
> should go in after everyone gets impatient.
>
> The rules for inclusion are elastic and include broad leeway for good
> behaviour, but this would stretch the elasticity way beyond breaking
> point.
>
> The list of issues is here:
>
> http://marc.theaimsgroup.com/?l=linux-scsi&m=114556263632510
>
> Most of the serious stuff is fixed with the exception of:
>
> - sysfs has more than one value per file
> - BE platform support
> - PAE (cast of dma_addr_t to unsigned long) issues.
> - SYNCHRONIZE_CACHE is ignored.  This is wrong.  The sync cache in the
> shutdown notifier isn't sufficient.
>
> At least the sysfs files have to be fixed before it goes in ... unless
> you want to be lynched by Greg?
>
> What I could do is set up a holding tree for all the fixed ... but -mm
> is doing a good job of that at the moment.
>
> James
>
> 

Re: Areca driver recap + status

[James Bottomley]

On Mon, 2006-06-26 at 18:23 -0600, Dax Kelson wrote:
> On Mon, 2006-06-26 at 09:48 -0500, James Bottomley wrote:
> > Not the world perhaps, but I'm unwilling to concede that if a driver
> > author is given a list of major issues and does nothing, then the driver
> > should go in after everyone gets impatient.
> 
> That isn't accurate or fair. Erich has submitted large patches to
> address issues. That hardly qualifies as "does nothing".

Yes, that was unfair, and I apologise for it.  However ...

> > The list of issues is here:
> > 
> > http://marc.theaimsgroup.com/?l=linux-scsi&m=114556263632510
> > 
> > Most of the serious stuff is fixed with the exception of:
> > 
> > - sysfs has more than one value per file
> > - BE platform support
> > - PAE (cast of dma_addr_t to unsigned long) issues.
> > - SYNCHRONIZE_CACHE is ignored.  This is wrong.  The sync cache in the
> > shutdown notifier isn't sufficient.
> > 
> > At least the sysfs files have to be fixed before it goes in ... unless
> > you want to be lynched by Greg?
> 
> Thanks for the new list. Erich has been eager to get work on any
> remaining blockers.

This isn't a new list.  It's extracted from the old list summary of
serious issues which an inspection of the driver reveals to be still
unresolved.  The original list was posted on 20 April

http://marc.theaimsgroup.com/?l=linux-scsi&m=114556263632510

And that one was an redux of a previously posed list that I can't be
bothered to find.

> It would have been nice to have gotten it back on May 19th, they might
> have been resolved by now.
> 
> http://marc.theaimsgroup.com/?l=linux-kernel&m=114801926400287&w=2

Well, this is what's adding to the rising frustration ... there's
apparently been another hiatus where I thought someone was working on
the rest of the issues labelled serious and no-one was.

James

Re: Areca driver recap + status

[James Bottomley]

On Tue, 2006-06-27 at 08:32 +1000, Robert Mueller wrote:
> > - PAE (cast of dma_addr_t to unsigned long) issues.
> 
> Can you explain a bit what this is about and what the effect is? It's just 
> that we've been using the driver (older version from the areca website) in a 
> PAE kernel on machines with 8G of memory and haven't had a problem yet 
> (running high IO load for several weeks) but this sounds like it 
> might/should cause corruption or crashing in this situation?

It's these pieces:

+		pcmd->SCp.ptr = (char *)(unsigned long) dma_addr;


+	else if (pcmd->request_bufflen != 0)
+		pci_unmap_single(acb->pdev,
+			(dma_addr_t)(unsigned long)pcmd->SCp.ptr,
+			pcmd->request_bufflen, pcmd->sc_data_direction);

On a PAE platform, dma_addr_t is u64 and unsigned long is u32, so any
address > 4GB will be truncated by these operations.

I think all this does is cause a slow leak of dma mappings, and on any
kernel > 2.6.16 the leak should be even smaller, since we've severely
restricted the use_sg == 0 case.  It probably is only significant on
x86_64 with the gart iommu enabled.

James

Re: Areca driver recap + status

[James Bottomley]

On Tue, 2006-06-27 at 16:51 +0100, Alan Cox wrote:
> On x86_64 the dma_addr_t and the ulong are both 64bit so the problem
> doesn't arise. 

Yes it does ... apparently 32 bit kernel on x86_64 is a lot more common
than people suppose.  In > 4GB this is exactly PAE.

James

Re: Areca driver recap + status

[Alan Cox]

Ar Maw, 2006-06-27 am 10:27 -0500, ysgrifennodd James Bottomley:
> On a PAE platform, dma_addr_t is u64 and unsigned long is u32, so any
> address > 4GB will be truncated by these operations.
> 
> I think all this does is cause a slow leak of dma mappings, and on any
> kernel > 2.6.16 the leak should be even smaller, since we've severely
> restricted the use_sg == 0 case.  It probably is only significant on
> x86_64 with the gart iommu enabled.

On x86_64 the dma_addr_t and the ulong are both 64bit so the problem
doesn't arise. 

Alan

Re: Areca driver recap + status

[James Bottomley]

On Tue, 2006-06-27 at 17:47 +0800, erich wrote:
> Does arcmsr still has more than one value per file issue on it?
> Maybe I am miss-understand the means of one value per file.

The idea is that sysfs files are named according to their contents, so
you should be able to cat the file to get the value.  The issue, which
is very simple to resolve with your driver, is that all the current
values are preceded by strings, so they have to be parsed to get the
value instead of just being directly read (I think this is a residue of
the fact that it's a straight conversion of a single proc file).

James

Re: Areca driver recap + status

[Alan Cox]

Ar Maw, 2006-06-27 am 10:39 -0500, ysgrifennodd James Bottomley:
> On Tue, 2006-06-27 at 16:51 +0100, Alan Cox wrote:
> > On x86_64 the dma_addr_t and the ulong are both 64bit so the problem
> > doesn't arise. 
> 
> Yes it does ... apparently 32 bit kernel on x86_64 is a lot more common
> than people suppose.  In > 4GB this is exactly PAE.

In your original mail you stated

"It probably is only significant on x86_64 with the gart iommu enabled."

IOMMU is only in the 64bit kernel that I can see.

Re: Areca driver recap + status

[erich]

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

From: Erich Chen <erich@areca.com.tw>

  1- fix sysfs has more than one value per file
  2- PAE issues (cast of dma_addr_t to unsigned long)
  3- unblock SYNCHRONIZE_CACHE

Signed-off-by: Erich Chen <erich@areca.com.tw>

Areca had tested its arcmsr linux raid driver on ppc machines G5 and it 
worked fine.

Best Regards
Erich Chen
----- Original Message ----- 
From: "Andrew Morton" <akpm@osdl.org>
To: "James Bottomley" <James.Bottomley@SteelEye.com>
Cc: <rdunlap@xenotime.net>; <hch@infradead.org>; <erich@areca.com.tw>; 
<brong@fastmail.fm>; <dax@gurulabs.com>; <linux-kernel@vger.kernel.org>; 
<linux-scsi@vger.kernel.org>; <robm@fastmail.fm>
Sent: Monday, June 26, 2006 11:01 PM
Subject: Re: Areca driver recap + status


> On Mon, 26 Jun 2006 09:48:58 -0500
> James Bottomley <James.Bottomley@SteelEye.com> wrote:
>
>> On Wed, 2006-06-21 at 22:28 -0700, Andrew Morton wrote:
>> > On Thu, 22 Jun 2006 14:18:23 +1000
>> > "Robert Mueller" <robm@fastmail.fm> wrote:
>> >
>> > > The driver went into 2.6.11-rc3-mm1 here:
>> > > http://marc.theaimsgroup.com/?l=linux-kernel&m=110754432622498&w=2
>> >
>> > One and a half years.
>> >
>> > Would the world end if we just merged the dang thing?
>>
>> Not the world perhaps, but I'm unwilling to concede that if a driver
>> author is given a list of major issues and does nothing, then the driver
>> should go in after everyone gets impatient.
>>
>> The rules for inclusion are elastic and include broad leeway for good
>> behaviour, but this would stretch the elasticity way beyond breaking
>> point.
>>
>> The list of issues is here:
>>
>> http://marc.theaimsgroup.com/?l=linux-scsi&m=114556263632510
>
> I'm under the impression that Erich is under the impression that they've 
> all
> been addressed.
>
>> Most of the serious stuff is fixed with the exception of:
>>
>> - sysfs has more than one value per file
>> - BE platform support
>> - PAE (cast of dma_addr_t to unsigned long) issues.
>> - SYNCHRONIZE_CACHE is ignored.  This is wrong.  The sync cache in the
>> shutdown notifier isn't sufficient.
>
> So this is progress.
>
> Erich, can you please fix these things up and then re-review the issues
> list which I'm maintaining in
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.17/2.6.17-mm2/broken-out/areca-raid-linux-scsi-driver.patch,
> make sure that everything has been addressed?
>
> If there are some things in those lists which you cannot/will not address
> then please identify them and give us the reasoning behind your decision.
> 

[-- Attachment #2: areca-raid-linux-scsi-driver-update7.patch.gz --]
[-- Type: application/x-compressed, Size: 2082 bytes --]

Re: Areca driver recap + status

[Andrew Morton]

On Fri, 7 Jul 2006 16:41:53 +0800
"erich" <erich@areca.com.tw> wrote:

> From: Erich Chen <erich@areca.com.tw>
> 
>   1- fix sysfs has more than one value per file
>   2- PAE issues (cast of dma_addr_t to unsigned long)
>   3- unblock SYNCHRONIZE_CACHE
> 
> Signed-off-by: Erich Chen <erich@areca.com.tw>
> 
> Areca had tested its arcmsr linux raid driver on ppc machines G5 and it 
> worked fine.

Thanks.

> +static ssize_t
> +arcmsr_sysfs_iop_message_clear(struct kobject *kobj, char *buf, loff_t off,
> +    size_t count)
> +{
> +	struct class_device *cdev = container_of(kobj,struct class_device,kobj);
> +	struct Scsi_Host *host = class_to_shost(cdev);
> +	struct AdapterControlBlock *acb = (struct AdapterControlBlock *) host->hostdata;
> +	struct MessageUnit __iomem *reg = acb->pmu;
> +	uint8_t *pQbuffer;
> +
> +	if (!capable(CAP_SYS_ADMIN) || (count && off) == 0)
> +		return 0;

That (count && off) == 0 looks odd.  Are you sure that's what you meant to
do?

Also, a write() handler shouldn't return zero if it didn't write anything. 
Some applications will see that the write() returned less than expected and
didn't return an error so they'll just loop around and try to write more
data.  They'll hang up when writing to your sysfs file.

http://www.opengroup.org/onlinepubs/009695399/functions/write.html says

RETURN VALUE

    Upon successful completion, write() and pwrite() shall return the
    number of bytes actually written to the file associated with fildes. 
    This number shall never be greater than nbyte.  Otherwise, -1 shall be
    returned and errno set to indicate the error.

So you should return -EINVAL here, and perhaps -EACCES or -EPERM.  Because
nothing was written.