Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1231009 > unrolled thread

Re: [RFT v3] eata: Convert eata driver as normal PCI and platform device drivers

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFT v3] eata: Convert eata driver as normal PCI and platform  device drivers Arthur Marsh <arthur.marsh@internode.on.net> - 2015-09-23 00:30 +0200
    Re: [RFT v3] eata: Convert eata driver as normal PCI and platform  device drivers James Bottomley <James.Bottomley@HansenPartnership.com> - 2015-09-23 00:50 +0200
      Re: [RFT v3] eata: Convert eata driver as normal PCI and platform  device drivers Jiang Liu <jiang.liu@linux.intel.com> - 2015-09-23 07:30 +0200
        Re: [RFT v3] eata: Convert eata driver as normal PCI and platform  device drivers Arthur Marsh <arthur.marsh@internode.on.net> - 2015-09-23 12:50 +0200
          Re: [RFT v3] eata: Convert eata driver as normal PCI and platform  device drivers James Bottomley <James.Bottomley@HansenPartnership.com> - 2015-09-23 16:50 +0200
            Re: [RFT v3] eata: Convert eata driver as normal PCI and platform  device drivers Jiang Liu <jiang.liu@linux.intel.com> - 2015-09-24 06:30 +0200
              Re: [RFT v3] eata: Convert eata driver as normal PCI and platform  device drivers Arthur Marsh <arthur.marsh@internode.on.net> - 2015-09-24 08:00 +0200
                Re: [RFT v3] eata: Convert eata driver as normal PCI and platform  device drivers Arthur Marsh <arthur.marsh@internode.on.net> - 2015-09-26 08:30 +0200
                  Re: [RFT v3] eata: Convert eata driver as normal PCI and platform  device drivers Jiang Liu <jiang.liu@linux.intel.com> - 2015-10-03 10:20 +0200
                    Re: [RFT v3] eata: Convert eata driver as normal PCI and platform  device drivers Arthur Marsh <arthur.marsh@internode.on.net> - 2015-10-03 13:20 +0200
                    Re: [RFT v3] eata: Convert eata driver as normal PCI and platform  device drivers Arthur Marsh <arthur.marsh@internode.on.net> - 2015-10-05 10:30 +0200

#1231009 — Re: [RFT v3] eata: Convert eata driver as normal PCI and platform device drivers

Jiang Liu wrote on 22/09/15 17:00:
> Previously the eata driver just grabs and accesses eata PCI devices
> without implementing a PCI device driver, that causes troubles with
> latest IRQ related
>
> Commit 991de2e59090 ("PCI, x86: Implement pcibios_alloc_irq() and
> pcibios_free_irq()") changes the way to allocate PCI legacy IRQ
> for PCI devices on x86 platforms. Instead of allocating PCI legacy
> IRQs when pcibios_enable_device() gets called, now pcibios_alloc_irq()
> will be called by pci_device_probe() to allocate PCI legacy IRQs
> when binding PCI drivers to PCI devices.
>
> But the eata driver directly accesses PCI devices without implementing
> corresponding PCI drivers, so pcibios_alloc_irq() won't be called for
> those PCI devices and wrong IRQ number may be used to manage the PCI
> device.
>
> This patch implements a PCI device driver to manage eata PCI devices,
> so eata driver could properly cooperate with the PCI core. It also
> provides headroom for PCI hotplug with eata driver.
>
> It also represents non-PCI eata devices as platform devices, so it could
> be managed as normal devices.
>
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Ballabio, Dario <dario.ballabio@emc.com>
> Cc: Christoph Hellwig <hch@infradead.org>
> ---

Not really any change with this driver:

previously

http://www.users.on.net/~arthur.marsh/20150915547.jpg

now

http://www.users.on.net/~arthur.marsh/20150922553.jpg

If there was any way of capturing any more debug output I'd be happy to 
do it.

Arthur.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1231019

On Wed, 2015-09-23 at 07:55 +0930, Arthur Marsh wrote:
> 
> Jiang Liu wrote on 22/09/15 17:00:
> > Previously the eata driver just grabs and accesses eata PCI devices
> > without implementing a PCI device driver, that causes troubles with
> > latest IRQ related
> >
> > Commit 991de2e59090 ("PCI, x86: Implement pcibios_alloc_irq() and
> > pcibios_free_irq()") changes the way to allocate PCI legacy IRQ
> > for PCI devices on x86 platforms. Instead of allocating PCI legacy
> > IRQs when pcibios_enable_device() gets called, now pcibios_alloc_irq()
> > will be called by pci_device_probe() to allocate PCI legacy IRQs
> > when binding PCI drivers to PCI devices.
> >
> > But the eata driver directly accesses PCI devices without implementing
> > corresponding PCI drivers, so pcibios_alloc_irq() won't be called for
> > those PCI devices and wrong IRQ number may be used to manage the PCI
> > device.
> >
> > This patch implements a PCI device driver to manage eata PCI devices,
> > so eata driver could properly cooperate with the PCI core. It also
> > provides headroom for PCI hotplug with eata driver.
> >
> > It also represents non-PCI eata devices as platform devices, so it could
> > be managed as normal devices.
> >
> > Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> > Cc: Hannes Reinecke <hare@suse.de>
> > Cc: Ballabio, Dario <dario.ballabio@emc.com>
> > Cc: Christoph Hellwig <hch@infradead.org>
> > ---
> 
> Not really any change with this driver:
> 
> previously
> 
> http://www.users.on.net/~arthur.marsh/20150915547.jpg
> 
> now
> 
> http://www.users.on.net/~arthur.marsh/20150922553.jpg
> 
> If there was any way of capturing any more debug output I'd be happy to 
> do it.

It looks to be some problem in shut down.  Can you simply remove and
re-insert the driver successfully?  If it's your root disk driver,
you'll have to do this from an initrd so as not to have root mounted
from the eata controller.

If the remove and reinsert fails, it means we have a problem in the
driver shut down.  If not, it's likely something kexec related.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1231157

[Multipart message — attachments visible in raw view] — view raw

On 2015/9/23 7:36, Arthur Marsh wrote:
> James Bottomley wrote on 23/09/15 08:15:
>> On Wed, 2015-09-23 at 07:55 +0930, Arthur Marsh wrote:
>> It looks to be some problem in shut down.  Can you simply remove and
>> re-insert the driver successfully?  If it's your root disk driver,
>> you'll have to do this from an initrd so as not to have root mounted
>> from the eata controller.
>>
>> If the remove and reinsert fails, it means we have a problem in the
>> driver shut down.  If not, it's likely something kexec related.
>>
>> James
> 
> OK, it looks like there was a problem with unloading the driver.
> 
> After un-mounting file systems on the disk attached to the SCSI
> controller using the eata driver I could do a:
> 
> modprobe -r eata
> 
> but received the output of the attached dmesg log.
> 
> Attempting to do
> 
> modprobe eata
> 
> after the previous modprobe -r eata resulted in a complete lock-up.
Hi Arthur,
	I have found the cause of the warning messages, it's caused
by a flaw in the conversion. But according to my understanding,
it isn't related to the kexec/kdump failure. Could you please help
to test the attached new version?
Thanks!
Gerry

> 
> Arthur.

[toc] | [prev] | [next] | [standalone]

#1231338

Jiang Liu wrote on 23/09/15 14:54:

> Hi Arthur,
> 	I have found the cause of the warning messages, it's caused
> by a flaw in the conversion. But according to my understanding,
> it isn't related to the kexec/kdump failure. Could you please help
> to test the attached new version?
> Thanks!
> Gerry
>

Thanks, the patch worked, I could successfully unload and reload the 
eata module, and perform a kexec reboot with the eata module loading 
successfully afterwards.

Arthur.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1231485

On Wed, 2015-09-23 at 20:14 +0930, Arthur Marsh wrote:
> 
> Jiang Liu wrote on 23/09/15 14:54:
> 
> > Hi Arthur,
> > 	I have found the cause of the warning messages, it's caused
> > by a flaw in the conversion. But according to my understanding,
> > it isn't related to the kexec/kdump failure. Could you please help
> > to test the attached new version?
> > Thanks!
> > Gerry
> >
> 
> Thanks, the patch worked, I could successfully unload and reload the 
> eata module, and perform a kexec reboot with the eata module loading 
> successfully afterwards.

Great, so the bug was unconditionally unregistering the platform driver
when it would fail to attach if none of the legacy IO ports were
detected.

I think the driver needs a bit of a tidy up.  There's no need at all to
use ida_get_simple(): the only reason for a dense array of numbers was
for storing the hba private data in the array you got rid of; we can now
simply use shost->host_no ... it's more useful anyway because the
numbers match those SCSI is using.

Also, if you insist on converting the printk's to dev warn, you no
longer need to print out the driver name ... dev_printk already prints
out the device and driver name as the prefix.

The if (error == 0) is usually written as if (!error) but that's minor.

Thanks for doing the conversion,

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1231850

[Multipart message — attachments visible in raw view] — view raw

On 2015/9/23 22:40, James Bottomley wrote:
> On Wed, 2015-09-23 at 20:14 +0930, Arthur Marsh wrote:
>>
>> Jiang Liu wrote on 23/09/15 14:54:
>>
>>> Hi Arthur,
>>> 	I have found the cause of the warning messages, it's caused
>>> by a flaw in the conversion. But according to my understanding,
>>> it isn't related to the kexec/kdump failure. Could you please help
>>> to test the attached new version?
>>> Thanks!
>>> Gerry
>>>
>>
>> Thanks, the patch worked, I could successfully unload and reload the 
>> eata module, and perform a kexec reboot with the eata module loading 
>> successfully afterwards.
> 
> Great, so the bug was unconditionally unregistering the platform driver
> when it would fail to attach if none of the legacy IO ports were
> detected.
> 
> I think the driver needs a bit of a tidy up.  There's no need at all to
> use ida_get_simple(): the only reason for a dense array of numbers was
> for storing the hba private data in the array you got rid of; we can now
> simply use shost->host_no ... it's more useful anyway because the
> numbers match those SCSI is using.
> 
> Also, if you insist on converting the printk's to dev warn, you no
> longer need to print out the driver name ... dev_printk already prints
> out the device and driver name as the prefix.
> 
> The if (error == 0) is usually written as if (!error) but that's minor.
Hi James,
	Thanks for review. How about the attached patch which addresses
the three suggestions from you?
Thanks!
Gerry


> 
> Thanks for doing the conversion,
> 
> James
> 
> 

[toc] | [prev] | [next] | [standalone]

#1231864

Jiang Liu wrote on 24/09/15 13:58:

> Hi James,
> 	Thanks for review. How about the attached patch which addresses
> the three suggestions from you?
> Thanks!
> Gerry

I've applied the patch, rebuilt the kernel and verified that it allows 
unloading of the eata module and reloading it, as well as a successful 
kexec.

Regards,

Arthur.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1233153

Arthur Marsh wrote on 24/09/15 15:26:
>
>
> Jiang Liu wrote on 24/09/15 13:58:
>
>> Hi James,
>>     Thanks for review. How about the attached patch which addresses
>> the three suggestions from you?
>> Thanks!
>> Gerry
>
> I've applied the patch, rebuilt the kernel and verified that it allows
> unloading of the eata module and reloading it, as well as a successful
> kexec.
>
> Regards,
>
> Arthur.

After some more thorough testing I've encountered an ongoing problem 
trying to use kexec with filesystems mounted with the eata driver.

If I boot up and have the eata driver loaded but no filesystem check or 
mounting of filesystems on the disk attached to the DPT2044W controller, 
then attempt a kexec reboot I get the reboot pausing after the 
"synchronizing scsi cache" messages and getting the errors that I have 
included as pictures in my previous reports.

If I do a normal boot which includes eata being loaded, the disk 
attached to the DPT2044W controller having its filesystems checked and 
mounted, then attempt a kexec reboot, I get the reboot pausing after the 
"synchronizing SCSI cache" messages as before.

If I un-mount the filesystems on the disk attached to the DPT2044W 
controller after start-up and try a reboot I get the same problem.

If I do modprobe -r eata after un-mounting the filesystems on the disk 
attached to the DPT2044W controller after a start-up kexec *works fine*.

If I do:

start-up
un-mount filesystems on disk attached to DPT2044W controller
modprobe -r eata
modprobe eata
fsck -a of filesystems on disk attached to DPT2044W controller
mount filesystems

then a kexec reboot works fine.

I did some more experimenting and found a workaround:

I was unable to blacklist the eata module but if I did:

modprobe -r eata
modprobe eata

in a cron job before the fsck and mount commands then
I could then perform a kexec reboot successfully.

I also verified that if I did:

modprobe -r eata

after eata was loaded on boot-up without any fsck or mounting of 
filesystems on the disk attached to the DPT2044W controller using the 
eata the kexec reboot worked fine.

In summary:

if eata is loaded kexec reboot will fail unless a modprobe -r eata is 
done either manually or by a cron job.

if a modprobe -r eata has been done, then even if I modprobe eata and 
fsck and mount filesystems, kexec reboot works.

Any suggestions for further tests or checks welcome.

Arthur.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1238767

[Multipart message — attachments visible in raw view] — view raw

On 2015/9/26 14:27, Arthur Marsh wrote:
> 
> 
> Arthur Marsh wrote on 24/09/15 15:26:
>>
>>
>> Jiang Liu wrote on 24/09/15 13:58:
>>
>>> Hi James,
>>>     Thanks for review. How about the attached patch which addresses
>>> the three suggestions from you?
>>> Thanks!
>>> Gerry
>>
>> I've applied the patch, rebuilt the kernel and verified that it allows
>> unloading of the eata module and reloading it, as well as a successful
>> kexec.
>>
>> Regards,
>>
>> Arthur.
> 
> After some more thorough testing I've encountered an ongoing problem
> trying to use kexec with filesystems mounted with the eata driver.
> 
> If I boot up and have the eata driver loaded but no filesystem check or
> mounting of filesystems on the disk attached to the DPT2044W controller,
> then attempt a kexec reboot I get the reboot pausing after the
> "synchronizing scsi cache" messages and getting the errors that I have
> included as pictures in my previous reports.
> 
> If I do a normal boot which includes eata being loaded, the disk
> attached to the DPT2044W controller having its filesystems checked and
> mounted, then attempt a kexec reboot, I get the reboot pausing after the
> "synchronizing SCSI cache" messages as before.
> 
> If I un-mount the filesystems on the disk attached to the DPT2044W
> controller after start-up and try a reboot I get the same problem.
> 
> If I do modprobe -r eata after un-mounting the filesystems on the disk
> attached to the DPT2044W controller after a start-up kexec *works fine*.
Hi Arthur,
	The above results suggest that we need to shutdown eata
controller for kexec. So could you please try to apply the attached
patch upon the previous two patches?
Thanks!
Gerry

[toc] | [prev] | [next] | [standalone]

#1238812

Jiang Liu wrote on 03/10/15 17:41:

> Hi Arthur,
> 	The above results suggest that we need to shutdown eata
> controller for kexec. So could you please try to apply the attached
> patch upon the previous two patches?
> Thanks!
> Gerry
>

Hi, I still get kexec shutdown errors like this with the 3rd patch applied:

http://www.users.on.net/~arthur.marsh/20151003566.jpg

I can still unmount filesystems, modprobe -r eata and modprobe eata to 
get things into a state where a kexec reboot works.

Regards,

Arthur.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1239341

Jiang Liu wrote on 03/10/15 17:41:

>> If I do a normal boot which includes eata being loaded, the disk
>> attached to the DPT2044W controller having its filesystems checked and
>> mounted, then attempt a kexec reboot, I get the reboot pausing after the
>> "synchronizing SCSI cache" messages as before.
>>
>> If I un-mount the filesystems on the disk attached to the DPT2044W
>> controller after start-up and try a reboot I get the same problem.
>>
>> If I do modprobe -r eata after un-mounting the filesystems on the disk
>> attached to the DPT2044W controller after a start-up kexec *works fine*.
> Hi Arthur,
> 	The above results suggest that we need to shutdown eata
> controller for kexec. So could you please try to apply the attached
> patch upon the previous two patches?
> Thanks!
> Gerry
>

To clarify, if the eata driver gets loaded once and stays loaded, at a 
kexec reboot attempt the "Synchronising SCSI cache" message is missing 
for the SCSI disk attached to the controller using the eata driver and 
eventually other error messages appear as seen in screen images that I 
have previously posted.

If the eata driver is loaded, unloaded via modprobe -r, then reloaded, a 
kexec reboot shows 2 "Synchronising SCSI cache" messages for the SCSI 
disk attached to the controller using the eata driver and the kexec 
reboot is successful.


Arthur.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web