Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On Wed, Jun 25, 2014 at 2:51 PM, Luis R. Rodriguez wrote: > I'll go ahead and test this on the other > distribution you mentioned you had issues, curious what could trigger a > timeout > failure there that would be distribution specific. I've tested this on SLE12 and see no issues as well. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On Wed, Jun 25, 2014 at 04:00:19AM +0200, Luis R. Rodriguez wrote:
> On Wed, Jun 25, 2014 at 01:39:51AM +0200, Luis R. Rodriguez wrote:
> > On Tue, Jun 24, 2014 at 09:34:19AM -0700, Casey Leedom wrote:
> > > On 06/24/14 08:55, Casey Leedom wrote:
> > >> On 06/23/14 17:29, Luis R. Rodriguez wrote:
> > > So I just did this for a normal modprobe (after the system is up):
> > >
> > > JiffiesProcess
> > > ---
> > > 0begin firmware load process
> > > 3request_firmware() returns
> > > 7start looking at the adapter
> > > 10finish reading the first sector of existing adapter firmware
> > > 26we've decided that we're going to upgrade the firmware
> > > 28actual firmware upgrade process starts
> > > 448we've finished halting the adapter processor
> > > 451we enter the firmware write routine
> > > 8,470we've finished erasing the firmware FLASH sectors
> > > 14,336write of new firmware is complete
> > > 14,340the new firmware load is complete
> > > 14,949the adapter processor has been restarted; new firmware running
> > > 14,952firmware upgrade process complete
> > >
> > > Maybe request_firmware() takes more time during the boot phase but as we
> > > can see from the above timings, it's the actual firmware upgrade process
> > > which takes the most time ...
> >
> > OK so yeah the kernel work on request_firmware() isn't what takes over a
> > minute, its the actual hardware poking with the firmware it gets, and then
> > doing all the things you mentioned (a port for each netdevice, etc). This
> > is a
> > particularly interesting driver, apart from this I see some code about bus
> > master and loading firmware only once. Can you elaborate a bit on how that
> > is
> > designed to work? Is it that only one PCI bus master device is allowed, and
> > that will do the request_firmware() for all PCI devices? I'm a bit confused
> > about this part, are we sure the bus master device will probe first? We can
> > surely keep all this code on the driver but it seems that if all these
> > complexitities might become the norm we should consider an API for sharing a
> > clean framework for it.
Casey here was the first series of questions.
> > As you noted the complexities on firmware loading, the number of different
> > netdevices one device might actually have would make it impractical to try
> > to do any completion on firmware on the ndo_init() with
> > request_firmware_nowait().
> > Apart from a netdev specific API to handle all this cleanly, I wonder if
> > drivers like these merit getting placed by default onto the
> > deferred_probe_active_list.
> > Typically this is triggered when drivers don't have a resource a subsystem
> > hasn't yet brought up, the driver returns -EPROBE_DEFER and the core driver
> > infrastructure later probes these devices on a secondary list. Greg?
>
> Actually another option to clean this up is to use
> platform_device_register_simple()
> after the initial firmware load and start poking at stuff there. Check out
> drivers/net/ethernet/8390/ne.c for an example with probe and all. I think
> that can help split up the code paths quite nicely and let you do your
> pre port thing there. Thoughts?
And here was the other one, what your thoughts are on splitting things up
a bit more for probe as ports part of a platform driver?
In the meantime I'll go and hunt down to see if there are any timeouts other
than the one embedded within request_firmware() (or udev if used). As we have
clarified now the 60s timeout is a timeout embedded as part of the filesystem
lookup of the firmware, not the actual loading of the firmware onto the device.
I for instance can introduce a huge delay on purpose right after
request_firmware() say 3 minutes on a test driver and it loads just fine, but
on my OpenSUSE 13.1 system. I'll go ahead and test this on the other
distribution you mentioned you had issues, curious what could trigger a timeout
failure there that would be distribution specific.
ergon:~ # ls > /lib/firmware/fake.bin
mcgrof@ergon ~/devel/fake-firmware-delay $ cat Makefile
all: modules
.PHONY: modules
modules:
make -C /lib/modules/$(shell uname -r)/build M=$(PWD) modules
.PHONY: clean
clean:
make -C /lib/modules/$(shell uname -r)/build clean M=$(PWD)
obj-m += test.o
mcgrof@ergon ~/devel/fake-firmware-delay $ cat test.c
#include
#include
#include
#include
#include
static struct platform_device *pdev;
static int __init test_init(void)
{
int ret;
const struct firmware *config;
pdev = platform_device_register_simple("fake-dev", 0, NULL, 0);
if (IS_ERR(pdev))
return PTR_ERR(pdev);
ret = request_firmware(, "fake.bin", >dev);
if (ret < 0) {
dev_set_uevent_suppress(>dev, true);
platform_device_unregister(pdev);
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On Wed, Jun 25, 2014 at 04:00:19AM +0200, Luis R. Rodriguez wrote:
On Wed, Jun 25, 2014 at 01:39:51AM +0200, Luis R. Rodriguez wrote:
On Tue, Jun 24, 2014 at 09:34:19AM -0700, Casey Leedom wrote:
On 06/24/14 08:55, Casey Leedom wrote:
On 06/23/14 17:29, Luis R. Rodriguez wrote:
So I just did this for a normal modprobe (after the system is up):
JiffiesProcess
---
0begin firmware load process
3request_firmware() returns
7start looking at the adapter
10finish reading the first sector of existing adapter firmware
26we've decided that we're going to upgrade the firmware
28actual firmware upgrade process starts
448we've finished halting the adapter processor
451we enter the firmware write routine
8,470we've finished erasing the firmware FLASH sectors
14,336write of new firmware is complete
14,340the new firmware load is complete
14,949the adapter processor has been restarted; new firmware running
14,952firmware upgrade process complete
Maybe request_firmware() takes more time during the boot phase but as we
can see from the above timings, it's the actual firmware upgrade process
which takes the most time ...
OK so yeah the kernel work on request_firmware() isn't what takes over a
minute, its the actual hardware poking with the firmware it gets, and then
doing all the things you mentioned (a port for each netdevice, etc). This
is a
particularly interesting driver, apart from this I see some code about bus
master and loading firmware only once. Can you elaborate a bit on how that
is
designed to work? Is it that only one PCI bus master device is allowed, and
that will do the request_firmware() for all PCI devices? I'm a bit confused
about this part, are we sure the bus master device will probe first? We can
surely keep all this code on the driver but it seems that if all these
complexitities might become the norm we should consider an API for sharing a
clean framework for it.
Casey here was the first series of questions.
As you noted the complexities on firmware loading, the number of different
netdevices one device might actually have would make it impractical to try
to do any completion on firmware on the ndo_init() with
request_firmware_nowait().
Apart from a netdev specific API to handle all this cleanly, I wonder if
drivers like these merit getting placed by default onto the
deferred_probe_active_list.
Typically this is triggered when drivers don't have a resource a subsystem
hasn't yet brought up, the driver returns -EPROBE_DEFER and the core driver
infrastructure later probes these devices on a secondary list. Greg?
Actually another option to clean this up is to use
platform_device_register_simple()
after the initial firmware load and start poking at stuff there. Check out
drivers/net/ethernet/8390/ne.c for an example with probe and all. I think
that can help split up the code paths quite nicely and let you do your
pre port thing there. Thoughts?
And here was the other one, what your thoughts are on splitting things up
a bit more for probe as ports part of a platform driver?
In the meantime I'll go and hunt down to see if there are any timeouts other
than the one embedded within request_firmware() (or udev if used). As we have
clarified now the 60s timeout is a timeout embedded as part of the filesystem
lookup of the firmware, not the actual loading of the firmware onto the device.
I for instance can introduce a huge delay on purpose right after
request_firmware() say 3 minutes on a test driver and it loads just fine, but
on my OpenSUSE 13.1 system. I'll go ahead and test this on the other
distribution you mentioned you had issues, curious what could trigger a timeout
failure there that would be distribution specific.
ergon:~ # ls /lib/firmware/fake.bin
mcgrof@ergon ~/devel/fake-firmware-delay $ cat Makefile
all: modules
.PHONY: modules
modules:
make -C /lib/modules/$(shell uname -r)/build M=$(PWD) modules
.PHONY: clean
clean:
make -C /lib/modules/$(shell uname -r)/build clean M=$(PWD)
obj-m += test.o
mcgrof@ergon ~/devel/fake-firmware-delay $ cat test.c
#include linux/kernel.h
#include linux/module.h
#include linux/firmware.h
#include linux/platform_device.h
#include linux/delay.h
static struct platform_device *pdev;
static int __init test_init(void)
{
int ret;
const struct firmware *config;
pdev = platform_device_register_simple(fake-dev, 0, NULL, 0);
if (IS_ERR(pdev))
return PTR_ERR(pdev);
ret = request_firmware(config, fake.bin, pdev-dev);
if (ret 0) {
dev_set_uevent_suppress(pdev-dev, true);
platform_device_unregister(pdev);
return ret;
}
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On Wed, Jun 25, 2014 at 2:51 PM, Luis R. Rodriguez mcg...@suse.com wrote: I'll go ahead and test this on the other distribution you mentioned you had issues, curious what could trigger a timeout failure there that would be distribution specific. I've tested this on SLE12 and see no issues as well. Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On Wed, Jun 25, 2014 at 01:39:51AM +0200, Luis R. Rodriguez wrote: > On Tue, Jun 24, 2014 at 09:34:19AM -0700, Casey Leedom wrote: > > On 06/24/14 08:55, Casey Leedom wrote: > >> On 06/23/14 17:29, Luis R. Rodriguez wrote: > > So I just did this for a normal modprobe (after the system is up): > > > > JiffiesProcess > > --- > > 0begin firmware load process > > 3request_firmware() returns > > 7start looking at the adapter > > 10finish reading the first sector of existing adapter firmware > > 26we've decided that we're going to upgrade the firmware > > 28actual firmware upgrade process starts > > 448we've finished halting the adapter processor > > 451we enter the firmware write routine > > 8,470we've finished erasing the firmware FLASH sectors > > 14,336write of new firmware is complete > > 14,340the new firmware load is complete > > 14,949the adapter processor has been restarted; new firmware running > > 14,952firmware upgrade process complete > > > > Maybe request_firmware() takes more time during the boot phase but as we > > can see from the above timings, it's the actual firmware upgrade process > > which takes the most time ... > > OK so yeah the kernel work on request_firmware() isn't what takes over a > minute, its the actual hardware poking with the firmware it gets, and then > doing all the things you mentioned (a port for each netdevice, etc). This is > a > particularly interesting driver, apart from this I see some code about bus > master and loading firmware only once. Can you elaborate a bit on how that is > designed to work? Is it that only one PCI bus master device is allowed, and > that will do the request_firmware() for all PCI devices? I'm a bit confused > about this part, are we sure the bus master device will probe first? We can > surely keep all this code on the driver but it seems that if all these > complexitities might become the norm we should consider an API for sharing a > clean framework for it. > > As you noted the complexities on firmware loading, the number of different > netdevices one device might actually have would make it impractical to try > to do any completion on firmware on the ndo_init() with > request_firmware_nowait(). > Apart from a netdev specific API to handle all this cleanly, I wonder if > drivers like these merit getting placed by default onto the > deferred_probe_active_list. > Typically this is triggered when drivers don't have a resource a subsystem > hasn't yet brought up, the driver returns -EPROBE_DEFER and the core driver > infrastructure later probes these devices on a secondary list. Greg? Actually another option to clean this up is to use platform_device_register_simple() after the initial firmware load and start poking at stuff there. Check out drivers/net/ethernet/8390/ne.c for an example with probe and all. I think that can help split up the code paths quite nicely and let you do your pre port thing there. Thoughts? I still do have that question about bus master requirement though and ensuring that there are no races. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On Tue, Jun 24, 2014 at 09:34:19AM -0700, Casey Leedom wrote: > On 06/24/14 08:55, Casey Leedom wrote: >> On 06/23/14 17:29, Luis R. Rodriguez wrote: > So I just did this for a normal modprobe (after the system is up): > > JiffiesProcess > --- > 0begin firmware load process > 3request_firmware() returns > 7start looking at the adapter > 10finish reading the first sector of existing adapter firmware > 26we've decided that we're going to upgrade the firmware > 28actual firmware upgrade process starts > 448we've finished halting the adapter processor > 451we enter the firmware write routine > 8,470we've finished erasing the firmware FLASH sectors > 14,336write of new firmware is complete > 14,340the new firmware load is complete > 14,949the adapter processor has been restarted; new firmware running > 14,952firmware upgrade process complete > > Maybe request_firmware() takes more time during the boot phase but as we > can see from the above timings, it's the actual firmware upgrade process > which takes the most time ... OK so yeah the kernel work on request_firmware() isn't what takes over a minute, its the actual hardware poking with the firmware it gets, and then doing all the things you mentioned (a port for each netdevice, etc). This is a particularly interesting driver, apart from this I see some code about bus master and loading firmware only once. Can you elaborate a bit on how that is designed to work? Is it that only one PCI bus master device is allowed, and that will do the request_firmware() for all PCI devices? I'm a bit confused about this part, are we sure the bus master device will probe first? We can surely keep all this code on the driver but it seems that if all these complexitities might become the norm we should consider an API for sharing a clean framework for it. As you noted the complexities on firmware loading, the number of different netdevices one device might actually have would make it impractical to try to do any completion on firmware on the ndo_init() with request_firmware_nowait(). Apart from a netdev specific API to handle all this cleanly, I wonder if drivers like these merit getting placed by default onto the deferred_probe_active_list. Typically this is triggered when drivers don't have a resource a subsystem hasn't yet brought up, the driver returns -EPROBE_DEFER and the core driver infrastructure later probes these devices on a secondary list. Greg? Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On 06/24/14 08:55, Casey Leedom wrote: On 06/23/14 17:29, Luis R. Rodriguez wrote: On Mon, Jun 23, 2014 at 12:06:48PM -0700, Casey Leedom wrote: Also, it might be useful if there was a way for the driver module to "tell" the timeout mechanism that forward progress _is_ being made so it doesn't blow away the driver module load. Indeed if this is actually needed, but believe the issue here for the huge delays might be instead the lack of not using request_firmware_direct() and actual device initialization time, which I do not believe we penalize, we should be penalizing only the amount of time it takes either the kernel or udev to read the firmware from the filesystem. If you want I can time the actual phases of loading new firmware: request_firmware(), writing it to FLASH, release_firmware() ... So I just did this for a normal modprobe (after the system is up): JiffiesProcess --- 0begin firmware load process 3request_firmware() returns 7start looking at the adapter 10finish reading the first sector of existing adapter firmware 26we've decided that we're going to upgrade the firmware 28actual firmware upgrade process starts 448we've finished halting the adapter processor 451we enter the firmware write routine 8,470we've finished erasing the firmware FLASH sectors 14,336write of new firmware is complete 14,340the new firmware load is complete 14,949the adapter processor has been restarted; new firmware running 14,952firmware upgrade process complete Maybe request_firmware() takes more time during the boot phase but as we can see from the above timings, it's the actual firmware upgrade process which takes the most time ... Casey -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On 06/23/14 17:29, Luis R. Rodriguez wrote: On Mon, Jun 23, 2014 at 12:06:48PM -0700, Casey Leedom wrote: I've looked through the patch and I might be wrong, but it appears that all the uses of the asynchronous request_firmware_nowait() are followed immediately by wait_for_completion() calls which essentially would be the same as the previous code with an added layer of mechanism. Am I missing something? No you're right and I frailed to mention my original goal really is to see if we can instead move that to ndo_init(). Okay, thanks for confirming that. I thought I was being very stupid. The problem I think with ndo_init() is that comes off a Network Device (struct net_device) and you can't have Network Devices till you talk to the adpater, discover how many ports you have, what their MAC Addresses are, etc. And of course you'd then have to be prepared for an ndo_init() call on every instantiated Network Device on an adapter and only do the device initialization for the first (while holding off any and all activity on the others), etc. All doable but a bit more complicated than doing this at Device Probe time. We do have a problem with initialization of multiple adapters with external PHYs since, for each adapter we can check to see if the main adapter firmware needs updating, and then load the PHY firmware. If the main firmware needs updating on more than one adapter, the combined time to update each adapter's main firmware plus load the PHY firmware can exceed some Distribution's default limits for a driver module's load time (since the kernel seems to be processing the PCI Probe of each device sequentially). I noticed that for configuration updates it is optional for these configuration updates to exist, in which case then if udev is enabled the driver will wait quite a long time unncessarily. To fix that it seems request_firmware_direct() would be better. Hhmmm, I'm unfamiliar with all of this. I hadn't looked that far under the covers of request_firmware() to know that any of these variants existed, what their semantics were and when one over the other was the preferred API. It seems to me that it's unfortunate that the limit isn't on a per device basis since a system could have an arbitrary number of devices managed by a driver module. The timeout is for the amount of time it takes the kernel to get the firemware, not for device initialization, so its unclear to me that the 60 timeout thing is actually causing an issue here. Are you sure about that? I just loaded firmware on two of my T4-based adapters and each took about 15 seconds to complete. The firmware is ~0.5MB and needs to be written to the on-adapter FLASH. The PHY firmware takes a bit less but not a lot. Add that to the time to do general adapter initialization and you're cruising close to 1 minute for two adapters ... Thus, my comment that whatever timeout is present should really be per-adapter based ... and it would be nice if the driver could inform the kernel as to the expected maximum device probe initialization time so we didn't have to have a huge inappropriate timeout for all devices ... Also, it might be useful if there was a way for the driver module to "tell" the timeout mechanism that forward progress _is_ being made so it doesn't blow away the driver module load. Indeed if this is actually needed, but believe the issue here for the huge delays might be instead the lack of not using request_firmware_direct() and actual device initialization time, which I do not believe we penalize, we should be penalizing only the amount of time it takes either the kernel or udev to read the firmware from the filesystem. If you want I can time the actual phases of loading new firmware: request_firmware(), writing it to FLASH, release_firmware() ... And maybe, if I'm right regarding the sequential nature of the introduction of devices to driver modules, it might make sense for a driver module to be able to "tell" the kernel that it has no per-device dependencies and multiple devices may be probed simultaneously ... What if just configuration updates use request_firmware_direct() and for the actual firmware which is required a request_firmware_nowait() with a proper wait_for_completion() on the ndo_init() ? I'm not quite sure I understand what you're asking here. The cxgb4 driver attaches to the firmware in the probe() stage and, if the firmware is older then the supported version, upgrades the firmware and RESETs the device. Then, for adapters with external PHYs (10Gb/s BT predominantly), the same process can happen for the PHY firmware (or on some adapters which have no PHY-attached FLASH, this happens for every driver load). It's conceivable that we could defer the PHY firmware load till the first ndo_open() but we'd still be stuck with the seemingly broken idea that a simple timeout for a driver module load is good enough when what we seem to
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On 06/23/14 17:29, Luis R. Rodriguez wrote: On Mon, Jun 23, 2014 at 12:06:48PM -0700, Casey Leedom wrote: I've looked through the patch and I might be wrong, but it appears that all the uses of the asynchronous request_firmware_nowait() are followed immediately by wait_for_completion() calls which essentially would be the same as the previous code with an added layer of mechanism. Am I missing something? No you're right and I frailed to mention my original goal really is to see if we can instead move that to ndo_init(). Okay, thanks for confirming that. I thought I was being very stupid. The problem I think with ndo_init() is that comes off a Network Device (struct net_device) and you can't have Network Devices till you talk to the adpater, discover how many ports you have, what their MAC Addresses are, etc. And of course you'd then have to be prepared for an ndo_init() call on every instantiated Network Device on an adapter and only do the device initialization for the first (while holding off any and all activity on the others), etc. All doable but a bit more complicated than doing this at Device Probe time. We do have a problem with initialization of multiple adapters with external PHYs since, for each adapter we can check to see if the main adapter firmware needs updating, and then load the PHY firmware. If the main firmware needs updating on more than one adapter, the combined time to update each adapter's main firmware plus load the PHY firmware can exceed some Distribution's default limits for a driver module's load time (since the kernel seems to be processing the PCI Probe of each device sequentially). I noticed that for configuration updates it is optional for these configuration updates to exist, in which case then if udev is enabled the driver will wait quite a long time unncessarily. To fix that it seems request_firmware_direct() would be better. Hhmmm, I'm unfamiliar with all of this. I hadn't looked that far under the covers of request_firmware() to know that any of these variants existed, what their semantics were and when one over the other was the preferred API. It seems to me that it's unfortunate that the limit isn't on a per device basis since a system could have an arbitrary number of devices managed by a driver module. The timeout is for the amount of time it takes the kernel to get the firemware, not for device initialization, so its unclear to me that the 60 timeout thing is actually causing an issue here. Are you sure about that? I just loaded firmware on two of my T4-based adapters and each took about 15 seconds to complete. The firmware is ~0.5MB and needs to be written to the on-adapter FLASH. The PHY firmware takes a bit less but not a lot. Add that to the time to do general adapter initialization and you're cruising close to 1 minute for two adapters ... Thus, my comment that whatever timeout is present should really be per-adapter based ... and it would be nice if the driver could inform the kernel as to the expected maximum device probe initialization time so we didn't have to have a huge inappropriate timeout for all devices ... Also, it might be useful if there was a way for the driver module to tell the timeout mechanism that forward progress _is_ being made so it doesn't blow away the driver module load. Indeed if this is actually needed, but believe the issue here for the huge delays might be instead the lack of not using request_firmware_direct() and actual device initialization time, which I do not believe we penalize, we should be penalizing only the amount of time it takes either the kernel or udev to read the firmware from the filesystem. If you want I can time the actual phases of loading new firmware: request_firmware(), writing it to FLASH, release_firmware() ... And maybe, if I'm right regarding the sequential nature of the introduction of devices to driver modules, it might make sense for a driver module to be able to tell the kernel that it has no per-device dependencies and multiple devices may be probed simultaneously ... What if just configuration updates use request_firmware_direct() and for the actual firmware which is required a request_firmware_nowait() with a proper wait_for_completion() on the ndo_init() ? I'm not quite sure I understand what you're asking here. The cxgb4 driver attaches to the firmware in the probe() stage and, if the firmware is older then the supported version, upgrades the firmware and RESETs the device. Then, for adapters with external PHYs (10Gb/s BT predominantly), the same process can happen for the PHY firmware (or on some adapters which have no PHY-attached FLASH, this happens for every driver load). It's conceivable that we could defer the PHY firmware load till the first ndo_open() but we'd still be stuck with the seemingly broken idea that a simple timeout for a driver module load is good enough when what we seem to need is
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On 06/24/14 08:55, Casey Leedom wrote: On 06/23/14 17:29, Luis R. Rodriguez wrote: On Mon, Jun 23, 2014 at 12:06:48PM -0700, Casey Leedom wrote: Also, it might be useful if there was a way for the driver module to tell the timeout mechanism that forward progress _is_ being made so it doesn't blow away the driver module load. Indeed if this is actually needed, but believe the issue here for the huge delays might be instead the lack of not using request_firmware_direct() and actual device initialization time, which I do not believe we penalize, we should be penalizing only the amount of time it takes either the kernel or udev to read the firmware from the filesystem. If you want I can time the actual phases of loading new firmware: request_firmware(), writing it to FLASH, release_firmware() ... So I just did this for a normal modprobe (after the system is up): JiffiesProcess --- 0begin firmware load process 3request_firmware() returns 7start looking at the adapter 10finish reading the first sector of existing adapter firmware 26we've decided that we're going to upgrade the firmware 28actual firmware upgrade process starts 448we've finished halting the adapter processor 451we enter the firmware write routine 8,470we've finished erasing the firmware FLASH sectors 14,336write of new firmware is complete 14,340the new firmware load is complete 14,949the adapter processor has been restarted; new firmware running 14,952firmware upgrade process complete Maybe request_firmware() takes more time during the boot phase but as we can see from the above timings, it's the actual firmware upgrade process which takes the most time ... Casey -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On Tue, Jun 24, 2014 at 09:34:19AM -0700, Casey Leedom wrote: On 06/24/14 08:55, Casey Leedom wrote: On 06/23/14 17:29, Luis R. Rodriguez wrote: So I just did this for a normal modprobe (after the system is up): JiffiesProcess --- 0begin firmware load process 3request_firmware() returns 7start looking at the adapter 10finish reading the first sector of existing adapter firmware 26we've decided that we're going to upgrade the firmware 28actual firmware upgrade process starts 448we've finished halting the adapter processor 451we enter the firmware write routine 8,470we've finished erasing the firmware FLASH sectors 14,336write of new firmware is complete 14,340the new firmware load is complete 14,949the adapter processor has been restarted; new firmware running 14,952firmware upgrade process complete Maybe request_firmware() takes more time during the boot phase but as we can see from the above timings, it's the actual firmware upgrade process which takes the most time ... OK so yeah the kernel work on request_firmware() isn't what takes over a minute, its the actual hardware poking with the firmware it gets, and then doing all the things you mentioned (a port for each netdevice, etc). This is a particularly interesting driver, apart from this I see some code about bus master and loading firmware only once. Can you elaborate a bit on how that is designed to work? Is it that only one PCI bus master device is allowed, and that will do the request_firmware() for all PCI devices? I'm a bit confused about this part, are we sure the bus master device will probe first? We can surely keep all this code on the driver but it seems that if all these complexitities might become the norm we should consider an API for sharing a clean framework for it. As you noted the complexities on firmware loading, the number of different netdevices one device might actually have would make it impractical to try to do any completion on firmware on the ndo_init() with request_firmware_nowait(). Apart from a netdev specific API to handle all this cleanly, I wonder if drivers like these merit getting placed by default onto the deferred_probe_active_list. Typically this is triggered when drivers don't have a resource a subsystem hasn't yet brought up, the driver returns -EPROBE_DEFER and the core driver infrastructure later probes these devices on a secondary list. Greg? Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On Wed, Jun 25, 2014 at 01:39:51AM +0200, Luis R. Rodriguez wrote: On Tue, Jun 24, 2014 at 09:34:19AM -0700, Casey Leedom wrote: On 06/24/14 08:55, Casey Leedom wrote: On 06/23/14 17:29, Luis R. Rodriguez wrote: So I just did this for a normal modprobe (after the system is up): JiffiesProcess --- 0begin firmware load process 3request_firmware() returns 7start looking at the adapter 10finish reading the first sector of existing adapter firmware 26we've decided that we're going to upgrade the firmware 28actual firmware upgrade process starts 448we've finished halting the adapter processor 451we enter the firmware write routine 8,470we've finished erasing the firmware FLASH sectors 14,336write of new firmware is complete 14,340the new firmware load is complete 14,949the adapter processor has been restarted; new firmware running 14,952firmware upgrade process complete Maybe request_firmware() takes more time during the boot phase but as we can see from the above timings, it's the actual firmware upgrade process which takes the most time ... OK so yeah the kernel work on request_firmware() isn't what takes over a minute, its the actual hardware poking with the firmware it gets, and then doing all the things you mentioned (a port for each netdevice, etc). This is a particularly interesting driver, apart from this I see some code about bus master and loading firmware only once. Can you elaborate a bit on how that is designed to work? Is it that only one PCI bus master device is allowed, and that will do the request_firmware() for all PCI devices? I'm a bit confused about this part, are we sure the bus master device will probe first? We can surely keep all this code on the driver but it seems that if all these complexitities might become the norm we should consider an API for sharing a clean framework for it. As you noted the complexities on firmware loading, the number of different netdevices one device might actually have would make it impractical to try to do any completion on firmware on the ndo_init() with request_firmware_nowait(). Apart from a netdev specific API to handle all this cleanly, I wonder if drivers like these merit getting placed by default onto the deferred_probe_active_list. Typically this is triggered when drivers don't have a resource a subsystem hasn't yet brought up, the driver returns -EPROBE_DEFER and the core driver infrastructure later probes these devices on a secondary list. Greg? Actually another option to clean this up is to use platform_device_register_simple() after the initial firmware load and start poking at stuff there. Check out drivers/net/ethernet/8390/ne.c for an example with probe and all. I think that can help split up the code paths quite nicely and let you do your pre port thing there. Thoughts? I still do have that question about bus master requirement though and ensuring that there are no races. Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On Mon, Jun 23, 2014 at 12:06:48PM -0700, Casey Leedom wrote: > I've looked through the patch and I might be wrong, but it appears that > all the uses of the asynchronous request_firmware_nowait() are followed > immediately by wait_for_completion() calls which essentially would be the > same as the previous code with an added layer of mechanism. Am I missing > something? No you're right and I frailed to mention my original goal really is to see if we can instead move that to ndo_init(). > We do have a problem with initialization of multiple adapters with > external PHYs since, for each adapter we can check to see if the main > adapter firmware needs updating, and then load the PHY firmware. If the > main firmware needs updating on more than one adapter, the combined time to > update each adapter's main firmware plus load the PHY firmware can exceed > some Distribution's default limits for a driver module's load time (since > the kernel seems to be processing the PCI Probe of each device > sequentially). I noticed that for configuration updates it is optional for these configuration updates to exist, in which case then if udev is enabled the driver will wait quite a long time unncessarily. To fix that it seems request_firmware_direct() would be better. > It seems to me that it's unfortunate that the limit isn't on a per device > basis since a system could have an arbitrary number of devices managed by a > driver module. The timeout is for the amount of time it takes the kernel to get the firemware, not for device initialization, so its unclear to me that the 60 timeout thing is actually causing an issue here. > Also, it might be useful if there was a way for the driver > module to "tell" the timeout mechanism that forward progress _is_ being > made so it doesn't blow away the driver module load. Indeed if this is actually needed, but believe the issue here for the huge delays might be instead the lack of not using request_firmware_direct() and actual device initialization time, which I do not believe we penalize, we should be penalizing only the amount of time it takes either the kernel or udev to read the firmware from the filesystem. > And maybe, if I'm > right regarding the sequential nature of the introduction of devices to > driver modules, it might make sense for a driver module to be able to > "tell" the kernel that it has no per-device dependencies and multiple > devices may be probed simultaneously ... What if just configuration updates use request_firmware_direct() and for the actual firmware which is required a request_firmware_nowait() with a proper wait_for_completion() on the ndo_init() ? Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
I've looked through the patch and I might be wrong, but it appears that all the uses of the asynchronous request_firmware_nowait() are followed immediately by wait_for_completion() calls which essentially would be the same as the previous code with an added layer of mechanism. Am I missing something? We do have a problem with initialization of multiple adapters with external PHYs since, for each adapter we can check to see if the main adapter firmware needs updating, and then load the PHY firmware. If the main firmware needs updating on more than one adapter, the combined time to update each adapter's main firmware plus load the PHY firmware can exceed some Distribution's default limits for a driver module's load time (since the kernel seems to be processing the PCI Probe of each device sequentially). It seems to me that it's unfortunate that the limit isn't on a per device basis since a system could have an arbitrary number of devices managed by a driver module. Also, it might be useful if there was a way for the driver module to "tell" the timeout mechanism that forward progress _is_ being made so it doesn't blow away the driver module load. And maybe, if I'm right regarding the sequential nature of the introduction of devices to driver modules, it might make sense for a driver module to be able to "tell" the kernel that it has no per-device dependencies and multiple devices may be probed simultaneously ... Casey On 06/20/14 17:39, Luis R. Rodriguez wrote: From: "Luis R. Rodriguez" Its reported that loading the cxgb4 can take over 1 minute, use the more sane request_firmware_nowait() API call just in case this amount of time is causing issues. The driver uses the firmware API 3 times, one for the firmware, one for configuration and another one for flash, this provides the port for all cases. I don't have the hardware so please test. I did verify we can use this during pci probe and also during the ethtool flash callback. Luis R. Rodriguez (3): cxgb4: make ethtool set_flash use request_firmware_nowait() cxgb4: make configuration load use request_firmware_nowait() cxgb4: make device firmware load use request_firmware_nowait() drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 13 ++ drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 258 +++- 2 files changed, 176 insertions(+), 95 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
I've looked through the patch and I might be wrong, but it appears that all the uses of the asynchronous request_firmware_nowait() are followed immediately by wait_for_completion() calls which essentially would be the same as the previous code with an added layer of mechanism. Am I missing something? We do have a problem with initialization of multiple adapters with external PHYs since, for each adapter we can check to see if the main adapter firmware needs updating, and then load the PHY firmware. If the main firmware needs updating on more than one adapter, the combined time to update each adapter's main firmware plus load the PHY firmware can exceed some Distribution's default limits for a driver module's load time (since the kernel seems to be processing the PCI Probe of each device sequentially). It seems to me that it's unfortunate that the limit isn't on a per device basis since a system could have an arbitrary number of devices managed by a driver module. Also, it might be useful if there was a way for the driver module to tell the timeout mechanism that forward progress _is_ being made so it doesn't blow away the driver module load. And maybe, if I'm right regarding the sequential nature of the introduction of devices to driver modules, it might make sense for a driver module to be able to tell the kernel that it has no per-device dependencies and multiple devices may be probed simultaneously ... Casey On 06/20/14 17:39, Luis R. Rodriguez wrote: From: Luis R. Rodriguez mcg...@suse.com Its reported that loading the cxgb4 can take over 1 minute, use the more sane request_firmware_nowait() API call just in case this amount of time is causing issues. The driver uses the firmware API 3 times, one for the firmware, one for configuration and another one for flash, this provides the port for all cases. I don't have the hardware so please test. I did verify we can use this during pci probe and also during the ethtool flash callback. Luis R. Rodriguez (3): cxgb4: make ethtool set_flash use request_firmware_nowait() cxgb4: make configuration load use request_firmware_nowait() cxgb4: make device firmware load use request_firmware_nowait() drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 13 ++ drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 258 +++- 2 files changed, 176 insertions(+), 95 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
On Mon, Jun 23, 2014 at 12:06:48PM -0700, Casey Leedom wrote: I've looked through the patch and I might be wrong, but it appears that all the uses of the asynchronous request_firmware_nowait() are followed immediately by wait_for_completion() calls which essentially would be the same as the previous code with an added layer of mechanism. Am I missing something? No you're right and I frailed to mention my original goal really is to see if we can instead move that to ndo_init(). We do have a problem with initialization of multiple adapters with external PHYs since, for each adapter we can check to see if the main adapter firmware needs updating, and then load the PHY firmware. If the main firmware needs updating on more than one adapter, the combined time to update each adapter's main firmware plus load the PHY firmware can exceed some Distribution's default limits for a driver module's load time (since the kernel seems to be processing the PCI Probe of each device sequentially). I noticed that for configuration updates it is optional for these configuration updates to exist, in which case then if udev is enabled the driver will wait quite a long time unncessarily. To fix that it seems request_firmware_direct() would be better. It seems to me that it's unfortunate that the limit isn't on a per device basis since a system could have an arbitrary number of devices managed by a driver module. The timeout is for the amount of time it takes the kernel to get the firemware, not for device initialization, so its unclear to me that the 60 timeout thing is actually causing an issue here. Also, it might be useful if there was a way for the driver module to tell the timeout mechanism that forward progress _is_ being made so it doesn't blow away the driver module load. Indeed if this is actually needed, but believe the issue here for the huge delays might be instead the lack of not using request_firmware_direct() and actual device initialization time, which I do not believe we penalize, we should be penalizing only the amount of time it takes either the kernel or udev to read the firmware from the filesystem. And maybe, if I'm right regarding the sequential nature of the introduction of devices to driver modules, it might make sense for a driver module to be able to tell the kernel that it has no per-device dependencies and multiple devices may be probed simultaneously ... What if just configuration updates use request_firmware_direct() and for the actual firmware which is required a request_firmware_nowait() with a proper wait_for_completion() on the ndo_init() ? Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFT 0/3] cxgb4: use request_firmware_nowait()
From: "Luis R. Rodriguez" Its reported that loading the cxgb4 can take over 1 minute, use the more sane request_firmware_nowait() API call just in case this amount of time is causing issues. The driver uses the firmware API 3 times, one for the firmware, one for configuration and another one for flash, this provides the port for all cases. I don't have the hardware so please test. I did verify we can use this during pci probe and also during the ethtool flash callback. Luis R. Rodriguez (3): cxgb4: make ethtool set_flash use request_firmware_nowait() cxgb4: make configuration load use request_firmware_nowait() cxgb4: make device firmware load use request_firmware_nowait() drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 13 ++ drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 258 +++- 2 files changed, 176 insertions(+), 95 deletions(-) -- 2.0.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFT 0/3] cxgb4: use request_firmware_nowait()
From: Luis R. Rodriguez mcg...@suse.com Its reported that loading the cxgb4 can take over 1 minute, use the more sane request_firmware_nowait() API call just in case this amount of time is causing issues. The driver uses the firmware API 3 times, one for the firmware, one for configuration and another one for flash, this provides the port for all cases. I don't have the hardware so please test. I did verify we can use this during pci probe and also during the ethtool flash callback. Luis R. Rodriguez (3): cxgb4: make ethtool set_flash use request_firmware_nowait() cxgb4: make configuration load use request_firmware_nowait() cxgb4: make device firmware load use request_firmware_nowait() drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 13 ++ drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 258 +++- 2 files changed, 176 insertions(+), 95 deletions(-) -- 2.0.0 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
