IF YOU WOULD LIKE TO GET AN ACCOUNT, please write an
email to Administrator. User accounts are meant only to access repo
and report issues and/or generate pull requests.
This is a purpose-specific Git hosting for
BaseALT
projects. Thank you for your understanding!
Только зарегистрированные пользователи имеют доступ к сервису!
Для получения аккаунта, обратитесь к администратору.
Karan Tilak Kumar <kartilak@cisco.com> says:
Hi Martin, reviewers,
This cover letter describes the feature: add support for
multiqueue (MQ) to fnic driver.
Background: The Virtual Interface Card (VIC) firmware exposes several
queues that can be configured for sending IOs and receiving IO
responses. Unified Computing System Manager (UCSM) and Intersight
Manager (IMM) allows users to configure the number of queues to be
used for IOs.
The number of IO queues to be used is stored in a configuration file
by the VIC firmware. The fNIC driver reads the configuration file and
sets the number of queues to be used. Previously, the driver was
hard-coded to use only one queue. With this set of changes, the fNIC
driver will configure itself to use multiple queues. This feature
takes advantage of the block multiqueue layer to parallelize IOs being
sent out of the VIC card.
Here's a brief description of some of the salient patches:
- vnic_scsi.h needs to be in sync with VIC firmware to be able to read
the number of queues from the firmware config file. A patch has been
created for this.
- In an environment with many fnics (like we see in our customer
environments), it is hard to distinguish which fnic is printing logs.
Therefore, an fnic number has been included in the logs.
- read the number of queues from the firmware config file.
- include definitions in fnic.h to support multiqueue.
- modify the interrupt service routines (ISRs) to read from the
correct registers. The numbers that are used here come from discussions
with the VIC firmware team.
- track IO statistics for different queues.
- remove usage of host_lock, and only use fnic_lock in the fnic driver.
- use a hardware queue based spinlock to protect io_req.
- replace the hard-coded zeroth queue with a hardware queue number.
This presents a bulk of the changes.
- modify the definition of fnic_queuecommand to accept multiqueue tags.
- improve log messages, and indicate fnic number and multiqueue tags for
effective debugging.
Even though the patches have been made into a series, some patches are
heavier than others.
But, every effort has been made to keep the purpose of each patch as
a single-purpose, and to compile cleanly.
This patchset has been tested as a whole. Therefore, the tested-by fields
have been added only to two patches
in the set. All the individual patches compile cleanly. However,
I've refrained from adding tested-by to
most of the patches, so as to not mislead the reviewer/reader.
A brief note on the unit tests:
1. Increase number of queues to 64. Load driver. Run IOs via Medusa.
12+ hour run successful.
2. Configure multipathing, and run link flaps on single link.
IOs drop briefly, but pick up as expected.
3. Configure multipathing, and run link flaps on two links, with a
30 second delay in between. IOs drop briefly, but pick up as expected.
Repeat the above tests with 1 queue and 32 queues. All tests were
successful.
Please consider this patch series for the next merge window.
Link: https://lore.kernel.org/r/20231211173617.932990-1-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Improve existing logs by adding fnic number, hardware queue, tag, and mqtag
in the prints. Add logs with the above elements for effective debugging.
Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com>
Tested-by: Karan Tilak Kumar <kartilak@cisco.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
Link: https://lore.kernel.org/r/20231211173617.932990-13-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Implement support for MQ in fnic driver:
The block multiqueue layer issues IO to the fnic driver with an MQ tag. Use
the mqtag and derive a tag from it. Derive the hardware queue from the
mqtag and use it in all paths. Modify queuecommand to handle mqtag.
Replace wq and cq indices to support MQ. Replace the zeroth queue with a
hardware queue. Implement spin locks on a per hardware queue basis.
Replace io_lock with per hardware queue spinlock. Implement out of range
tag checks.
Allocate an io_req_table to track status of the io_req.
Test the driver by building it, loading it, and configuring 64 queues in
UCSM. Issue IOs using Medusa on multiple fnics. Enable/disable links to
exercise the abort and clean up path.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202310300032.2awCqkfn-lkp@intel.com/
Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com>
Tested-by: Karan Tilak Kumar <kartilak@cisco.com>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
Link: https://lore.kernel.org/r/20231211173617.932990-12-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Set map_queues in the fnic_host_template to fnic_mq_map_queues_cpus.
Define fnic_mq_map_queues_cpus to set cpu assignment to fnic queues.
Refactor code in fnic_probe to enable vnic queues before scsi_add_host.
Modify notify set to the correct index.
Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: John Garry <john.g.garry@oracle.com>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
Link: https://lore.kernel.org/r/20231211173617.932990-11-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Remove usage of host_lock. Replace with fnic_lock, where necessary. fnic
does not use host_lock. fnic uses fnic_lock. Use fnic lock to protect fnic
members in fnic_queuecommand. Add log messages in error cases.
Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
Link: https://lore.kernel.org/r/20231211173617.932990-10-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Define an array to track IOs for the different queues, print the IO stats
in fnic get stats data.
Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
Link: https://lore.kernel.org/r/20231211173617.932990-9-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Modify interrupt service routines for INTx, MSI, and MSI-x to support
multiqueue. Modify parameter list of fnic_wq_copy_cmpl_handler to take
cq_index. Modify fnic_cleanup function to use the new function call of
fnic_wq_copy_cmpl_handler. Refactor code to set interrupt mode to MSI-x to
a new function. Add a new stat for intx_dummy.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202310251847.4T8BVZAZ-lkp@intel.com/
Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
Link: https://lore.kernel.org/r/20231211173617.932990-8-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Refactor and re-define values in fnic.h to implement multiqueue (MQ)
functionality.
VIC firmware allows fnic to create up to 64 copy workqueues. Update the
copy workqueue max to 64. Modify the interrupt index to be in sync with
the firmware to support MQ. Add irq number to the MSIX entry. Define a
software workqueue table to track the status of io_reqs. Define a base for
the copy workqueue.
Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
Link: https://lore.kernel.org/r/20231211173617.932990-7-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Get the copy workqueue count and interrupt mode from the configuration. The
config can be changed via UCSM. Add logs to print the interrupt mode and
copy workqueue count. Add logs to print the vNIC resources.
Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
Link: https://lore.kernel.org/r/20231211173617.932990-6-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Rename wq_copy to hw_copy_wq to accurately describe the copy
workqueue. This will also help distinguish this data structure from
software data structures that can be introduced.
Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
Link: https://lore.kernel.org/r/20231211173617.932990-5-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Add fnic_num in fnic.h to identify fnic in a multi-fnic environment.
Increment and set the fnic number during driver load in fnic_probe.
Replace the host number with fnic number in debugfs.
Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
Link: https://lore.kernel.org/r/20231211173617.932990-3-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
VIC firmware has updated definitions. Modify structure and definitions to
sync with the latest VIC firmware.
Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
Link: https://lore.kernel.org/r/20231211173617.932990-2-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The newly introduced error messages get multiple format strings wrong:
size_t must be printed using the %z modifier rather than %l and dma_addr_t
must be printed by reference using the special %pad pointer type:
drivers/scsi/mpi3mr/mpi3mr_app.c: In function 'mpi3mr_build_nvme_prp':
include/linux/kern_levels.h:5:25: error: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'dma_addr_t' {aka 'unsigned int'} [-Werror=format=]
drivers/scsi/mpi3mr/mpi3mr_app.c:949:25: note: in expansion of macro 'dprint_bsg_err'
949 | dprint_bsg_err(mrioc,
| ^~~~~~~~~~~~~~
include/linux/kern_levels.h:5:25: error: format '%ld' expects argument of type 'long int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
drivers/scsi/mpi3mr/mpi3mr_app.c:1112:41: note: in expansion of macro 'dprint_bsg_err'
1112 | dprint_bsg_err(mrioc,
| ^~~~~~~~~~~~~~
Fixes: 9536af615dc9 ("scsi: mpi3mr: Support for preallocation of SGL BSG data buffers part-3")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Link: https://lore.kernel.org/r/20231207142813.935717-1-arnd@kernel.org
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org> # build-tested
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> says:
Hello,
this series converts all drivers below drivers/scsi to struct
platform_driver::remove_new(). See commit 5c5a7680e67b ("platform:
Provide a remove callback that returns no value") for an extended
explanation and the eventual goal.
All conversations are trivial, because all .remove() callbacks returned
zero unconditionally.
Best regards
Uwe
Link: https://lore.kernel.org/r/cover.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/1d385231c23c2a1e6e7dc1968eb111327386d1f6.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/5010d1a4f3d77eaa1114fa254c343c4f23313901.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/84239a68fe06143d1d6fed6c9aaae6a4680ead71.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/2f4b7366ca00a107a9595514795e909e632980da.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/92114604fd1274073915e515cae15003ff07aa4a.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/8242c07f617fc946aab857c9357f540598fe964e.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/1d16e93a498831abd64df8b8cf54fd8872cdd1cd.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/89ce161dad52d99df07135531512ccecb7f25d14.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/9013c84059b8ccd6a5c8305aa35cfdfa314ba74c.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/f71efbe17973c97fd2a1e78f8d7fcf456644510b.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/63294479a4e745210c078859afa88904fa0b3be8.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/27a2b133b1b88a9baf51353c511e93a5027f9602.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/c15ffc57efebc5da3f7e6dd558d69181e129cafe.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/811d180950b76c2d95cd080e3c251757ca011380.1701619134.git.u.kleine-koenig@pengutronix.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Two driver updates from Chandrakanth patil at Broadcom:
scsi: mpi3mr: Update driver version to 8.5.1.0.0
scsi: mpi3mr: Support for preallocation of SGL BSG data buffers part-3
scsi: mpi3mr: Support for preallocation of SGL BSG data buffers part-2
scsi: mpi3mr: Support for preallocation of SGL BSG data buffers part-1
scsi: mpi3mr: Fetch correct device dev handle for status reply descriptor
scsi: mpi3mr: Block PEL Enable Command on Controller Reset and Unrecoverable State
scsi: mpi3mr: Clean up block devices post controller reset
scsi: mpi3mr: Refresh sdev queue depth after controller reset
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The driver acquires the required NVMe SGLs from the pre-allocated
pool.
Co-developed-by: Sathya Prakash <sathya.prakash@broadcom.com>
Signed-off-by: Sathya Prakash <sathya.prakash@broadcom.com>
Signed-off-by: Chandrakanth patil <chandrakanth.patil@broadcom.com>
Link: https://lore.kernel.org/r/20231205191630.12201-4-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The driver acquires the required SGLs from the pre-allocated pool.
Co-developed-by: Sathya Prakash <sathya.prakash@broadcom.com>
Signed-off-by: Sathya Prakash <sathya.prakash@broadcom.com>
Signed-off-by: Chandrakanth patil <chandrakanth.patil@broadcom.com>
Link: https://lore.kernel.org/r/20231205191630.12201-3-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The driver now supports SGLs for BSG data transfer. Upon loading, the
driver pre-allocates SGLs in chunks of 8k, results in a total of 256 * 8k,
equal to 2MB. These pre-allocated SGLs are reserved for handling BSG
commands and are deallocated during driver unload.
Co-developed-by: Sathya Prakash <sathya.prakash@broadcom.com>
Signed-off-by: Sathya Prakash <sathya.prakash@broadcom.com>
Signed-off-by: Chandrakanth patil <chandrakanth.patil@broadcom.com>
Link: https://lore.kernel.org/r/20231205191630.12201-2-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The current dev handle for the status reply is 0xFFFF, which is invalid.
So fetch the correct value.
Co-developed-by: Sumit Saxena <sumit.saxena@broadcom.com>
Signed-off-by: Sumit Saxena <sumit.saxena@broadcom.com>
Signed-off-by: Chandrakanth patil <chandrakanth.patil@broadcom.com>
Link: https://lore.kernel.org/r/20231126053134.10133-5-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
If a controller reset is underway or the controller is in an unrecoverable
state, the PEL enable management command will be returned as EAGAIN or
EFAULT.
Cc: <stable@vger.kernel.org> # v6.1+
Co-developed-by: Sathya Prakash <sathya.prakash@broadcom.com>
Signed-off-by: Sathya Prakash <sathya.prakash@broadcom.com>
Signed-off-by: Chandrakanth patil <chandrakanth.patil@broadcom.com>
Link: https://lore.kernel.org/r/20231126053134.10133-4-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
After a controller reset, if the firmware changes the state of devices to
"hide", then remove those devices from the OS.
Cc: <stable@vger.kernel.org> # v6.6+
Co-developed-by: Sathya Prakash <sathya.prakash@broadcom.com>
Signed-off-by: Sathya Prakash <sathya.prakash@broadcom.com>
Signed-off-by: Chandrakanth patil <chandrakanth.patil@broadcom.com>
Link: https://lore.kernel.org/r/20231126053134.10133-3-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Hannes Reinecke <hare@kernel.org> says:
Hi all,
when testing command timeout with the help of XDP I found that
scsi_try_to_abort_cmd() would always return 'SUCCESS' for FCoE, even
if no commands could be sent over the wire. Which is not only
surprising, but also can lead to data corruption as commands were
never aborted. Root cause was that aborts had been sent twice, once
from FC error recovery and once from SCSI EH, with the former inducing
the latter to assume that the command was already aborted.
As usual, comments and reviews are welcome.
Link: https://lore.kernel.org/r/20231129165832.224100-1-hare@kernel.org
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
When an exchange is completed with FC_TIMED_OUT we should map it to
DID_TIME_OUT to inform the SCSI midlayer that this was a command timeout;
DID_BUS_BUSY implies that the command was never sent which is not the case
here.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Link: https://lore.kernel.org/r/20231129165832.224100-4-hare@kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
We should set the status to FC_TIMED_OUT when a timeout error is passed to
fc_fcp_rec_error().
Signed-off-by: Hannes Reinecke <hare@suse.de>
Link: https://lore.kernel.org/r/20231129165832.224100-3-hare@kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The current FC error recovery is sending up to three REC (recovery) frames
in 10 second intervals, and as a final step sending an ABTS after 30
seconds for the command itself. Unfortunately sending an ABTS is also the
action for the SCSI abort handler, and the default timeout for SCSI
commands is also 30 seconds. This causes two ABTS to be scheduled, with the
libfc one slightly earlier. The ABTS scheduled by SCSI EH then sees the
command to be already aborted, and will always return with a 'GOOD' status
irrespective on the actual result from the first ABTS. This causes the
SCSI EH abort handler to always succeed, and SCSI EH never to be engaged.
Fix this by not issuing an ABTS when a SCSI command is present for the
exchange, but rather wait for the abort scheduled from SCSI EH. And warn
if an abort is already scheduled to avoid similar errors in the future.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Link: https://lore.kernel.org/r/20231129165832.224100-2-hare@kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
aic7770_config() returns both negative and positive error codes. It's
better to make aic7770_probe() only return negative error codes.
A previous commit made ahc_linux_register_host() return negative error
codes, which makes sure aic7770_probe() returns negative error codes.
Signed-off-by: Su Hui <suhui@nfschina.com>
Link: https://lore.kernel.org/r/20231201025955.1584260-4-suhui@nfschina.com
Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
ahc_linux_register_host() can return an error code in case of failure. So
ahc_linux_pci_dev_probe() should return the value of
ahc_linux_register_host() rather than zero.
An earlier commit made ahc_linux_register_host() return negative error
codes, which makes sure ahc_linux_pci_dev_probe() returns negative error
codes.
Signed-off-by: Su Hui <suhui@nfschina.com>
Link: https://lore.kernel.org/r/20231201025955.1584260-3-suhui@nfschina.com
Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
ahc_linux_register_host() returns both positive and negative error codes.
It's better to make this function only return negative error codes.
Signed-off-by: Su Hui <suhui@nfschina.com>
Link: https://lore.kernel.org/r/20231201025955.1584260-2-suhui@nfschina.com
Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
sci_task_request_construct_ssp() has invariant return. Change this function
to void and get rid of unnecessary checks.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Artem Chernyshev <artem.chernyshev@red-soft.ru>
Link: https://lore.kernel.org/r/20231128121159.2373975-1-artem.chernyshev@red-soft.ru
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The IPR driver has a routine to check whether it's running on certain CPU
versions and if so whether the adapter is supported on that CPU.
But none of the CPUs it checks for are supported by Linux anymore.
The most recent CPU it checks for is Power4+ which was removed in commit
471d7ff8b51b ("powerpc/64s: Remove POWER4 support").
So drop the check. That makes the "testmode" module parameter unused, so
remove it as well.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20231127111740.1288463-1-mpe@ellerman.id.au
Acked-by: Brian King <brking@linux.vnet.ibm.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
strncpy() is deprecated for use on NUL-terminated destination strings [1]
and as such we should prefer more robust and less ambiguous string
interfaces.
We expect partition_name to be NUL-terminated based on its usage with
format strings:
| dev_info(hostdata->dev, "host srp version: %s, "
| "host partition %s (%d), OS %d, max io %u\n",
| hostdata->madapter_info.srp_version,
| hostdata->madapter_info.partition_name,
| be32_to_cpu(hostdata->madapter_info.partition_number),
| be32_to_cpu(hostdata->madapter_info.os_type),
| be32_to_cpu(hostdata->madapter_info.port_max_txu[0]));
...
| len = snprintf(buf, PAGE_SIZE, "%s\n",
| hostdata->madapter_info.partition_name);
Moreover, NUL-padding is not required as madapter_info is explicitly
memset to 0:
| memset(&hostdata->madapter_info, 0x00,
| sizeof(hostdata->madapter_info));
Considering the above, a suitable replacement is strscpy() [2] due to the
fact that it guarantees NUL-termination on the destination buffer without
unnecessarily NUL-padding.
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: <linux-hardening@vger.kernel.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
Link: https://lore.kernel.org/r/20231030-strncpy-drivers-scsi-ibmvscsi-ibmvscsi-c-v1-1-f8b06ae9e3d5@google.com
Reviewed-by: Kees Cook <keescook@chromium.org>
Tested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
strncpy() is deprecated for use on NUL-terminated destination strings [1]
and as such we should prefer more robust and less ambiguous string
interfaces.
We expect these fields to be NUL-terminated as the property names from
which they are derived are also NUL-terminated.
Moreover, NUL-padding is not required as our destination buffers are
already NUL-allocated and any future NUL-byte assignments are redundant
(like the ones that strncpy() does).
ibmvfc_probe() ->
| struct ibmvfc_host *vhost;
| struct Scsi_Host *shost;
...
| shost = scsi_host_alloc(&driver_template, sizeof(*vhost));
... **side note: is this a bug? Looks like a type to me ^^^^^**
...
| vhost = shost_priv(shost);
... where shost_priv() is:
| static inline void *shost_priv(struct Scsi_Host *shost)
| {
| return (void *)shost->hostdata;
| }
.. and:
scsi_host_alloc() ->
| shost = kzalloc(sizeof(struct Scsi_Host) + privsize, GFP_KERNEL);
And for login_info->..., NUL-padding is also not required as it is
explicitly memset to 0:
| memset(login_info, 0, sizeof(*login_info));
Considering the above, a suitable replacement is strscpy() [2] due to
the fact that it guarantees NUL-termination on the destination buffer
without unnecessarily NUL-padding.
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: <linux-hardening@vger.kernel.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
Link: https://lore.kernel.org/r/20231030-strncpy-drivers-scsi-ibmvscsi-ibmvfc-c-v1-1-5a4909688435@google.com
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
