Re: [PATCH 6/9] PCI: consolidate PCI config entry in drivers/pci
On Fri, Oct 19, 2018 at 09:58:46PM +0900, Masahiro Yamada wrote: > On Fri, Oct 19, 2018 at 9:23 PM Russell King - ARM Linux > wrote: > > > > index a68b34183107..b185794549be 100644 > > > --- a/arch/arm/mach-pxa/Kconfig > > > +++ b/arch/arm/mach-pxa/Kconfig > > > @@ -125,7 +125,7 @@ config MACH_ARMCORE > > > bool "CompuLab CM-X255/CM-X270 modules" > > > select ARCH_HAS_DMA_SET_COHERENT_MASK if PCI > > > select IWMMXT > > > - select MIGHT_HAVE_PCI > > > + select HAVE_PCI > > > select NEED_MACH_IO_H if PCI > > > select PXA25x > > > select PXA27x > > > > This is wrong. "MIGHT_HAVE_PCI" is _not_ the same as "HAVE_PCI" - we > > have a bunch of platforms that mandatorily have PCI and these select > > PCI directly. "MIGHT_HAVE_PCI" controls the _visibility_ of the PCI > > menu option, but does not prevent it being selected. Your patch will > > cause Kconfig to complain for those which mandatorily have PCI but > > do not set HAVE_PCI. > > > Good catch! > But, adding a bunch of 'select HAVE_PCI' along with 'select PCI' is ugly. > > Do you have any suggestion? > > How about letting CONFIG_ARM to select HAVE_PCI ? Well, the situation we have on ARM is rather optimal - when the rest of the configuration supports PCI, PCI is made visible. When there's no hardware support for PCI, PCI is hidden. When PCI is mandatory, PCI is selected and mostly always hidden. It seems that with this consolidation, we lose that - we end up with PCI being visible for every ARM config, not only those where PCI is "impossible" but also for those where PCI is forcefully selected. That said, offering PCI for platforms that do not have any possibility of PCI hardware shouldn't cause any compile issues, and would increase build coverage - but I'd say it wouldn't _usefully_ increase build coverage. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: remove exofs and the T10 OSD code V2
On 2018-11-01 1:03 a.m., Boaz Harrosh wrote: On 31/10/18 23:10, Douglas Gilbert wrote: On 2018-10-31 4:57 p.m., Boaz Harrosh wrote: On 30/10/18 09:45, Christoph Hellwig wrote: On Mon, Oct 29, 2018 at 02:42:12PM -0600, Jens Axboe wrote: LGTM, for both: I also have this one on top as requested by Martin. The core block bidi support is unfortunately also used by bsg-lib, although it is not anywhere near as invasive. But that is another argument for looking into moving bsg-lib away from using block queues.. BUT this patch is very very wrong. Totally apart from T10-OSD and its use in the field. Support for scsi BIDI commands is not exclusive to T10-OSD at all. Even the simple scsi-array command-set has BIDI operations defined. for example the write-return-xor and so on. Also some private administrative CDBs of some vendor devices uses SCSI-BIDI. So this patch just broke some drivers. (User-mode apps use bsg pass through) Also you might (try hard and) remove all usage of scsi-bidi as an initiator from the Linux Kernel. But what about target mode. As a target we have supported on the wire bidi protocols like write-return-xor and others for a long time. Are you willing to silently break all these setups in the field on the next update? Are you so sure these are never used? PLEASE, I beg of you guys. Do not remove SCSI-BIDI. It is a cry of generations. And I think by the rules of Linus, as far as target mode. You are not allowed to break users in this way. Hi, I'm in the process of rebuilding the sg driver with the following goals: - undo 20 years of road wear, some of which is caused by literally hundreds of "laparoscopic" patches that gradually ruin a driver, at least from the maintainer's viewpoint. Comments that once made sense become cryptic or just nonsense; naming schemes that obliterate variable names to the extent that a random name generator would be easier to follow; and just plain broken code. For example, why sort out the existing locking at a particular level when you can add a new lock in a completely non-orthogonal way? [Yes, I looking at you, google.] Anyway, my first cut at this is out there (on the linux-scsi list, see: "[PATCH v3 0/8] sg: major cleanup, remove max_queue limit"). Not much new there, unless you look very closely - the next step is to add to the sg driver async SCSI command capability based on the sg_io_v4 structure previously only used by the bsg driver and now removed from bsg. The main advantage of the sg_io_v4 structure over previous pass-through interface attempts is the support of SCSI bidi commands - as part of this effort introduce two new ioctls: SG_IOSUBMIT and SG_IORECEIVE to replace the write()/read() technique currently in use (since Linux 1.0 in 1992). The write()/read() technique seems to be responsible for various security folks losing clumps of their hair. One advantage of ioctls, as Alan Cox pointed out, is the ability to write to and read back from the kernel in a way that is naturally synchronized. [Actually, those security folks shouldn't look too closely at sg_read() in that respect.] In discussions with several folks who are on the T10 committee, I wondered why there was no READ GATHERED command (there has been a WRITE SCATTERED for 2 years). The answer was lack of interest ***, plus the difficultly of implementation. You see, READ GATHERED needs to send a scatter gather list to the device and get the corresponding data back (as a linear array). And that requires either: a) bidi commands (scatter gather list in the data-out, corresponding "read" data in the data-in), or b) lng SCSI commands, up to around 256 bytes long in which the sgat list is the latter part of that command And the T10 folks say neither of those options is well supported or is expensive. It is supported in Linux scsi/osd driver is a proof of that. And expensive it is not. I have demonstrated the ability to saturate a 10G link over a raid of devices from a single writer. In OSD everything is bidi. I'm guessing they are referring to Linux and Windows. I haven't argued much beyond that point, but it looks like a bit of a chicken and egg situation. Don't know too much about the T10 OSD stuff. But I can see that it uses both long SCSI commands and a lot of bidi. IMO it seems to be 10 or 20 years before its time. Maybe ibm/redhat need to (re-)discover it for it to catch on. Plus there are proprietary SCSI bidi commands out there. People contact me and ask me how to issue them with sg3_utils package. Easy, I tell them, just use sg_raw with a bsg device. Typically, in my experience, "no news is good news" after suggestions like that. When I give bad advice, I usually hear back relatively quickly. Anyone who wants SCSI bidi _async_ support is currently out of luck. And I forgot to mention that the
Re: [PATCHSET v3 0/30] blk-mq driver conversions and legacy path removal
On 10/31/18 8:35 PM, Ming Lei wrote: > On Wed, Oct 31, 2018 at 11:58:52AM -0600, Jens Axboe wrote: >> This patch series converts the remaining drivers to blk-mq. SCSI >> supports both paths, this removes the legacy IO path from SCSI. At the >> end, legacy IO code and schedulers are killed off. >> >> I'm not aware of any issues with this series. >> >> This patch series is on top of current -git. It can also be bound in >> my mq-conversions branch. >> >> Changes since v2: >> >> - Kill q->softirq_done_fn() >> >> Changes since v1: >> >> - Fix removed q->mq_ops non-NULL check in wbt_enable_default() >> - Remove spurious return in ide-io.c:ide_timer_expiry() >> - Dropped DM legacy path removal patch, now in mainline >> - Dropped ib_srp patch, now in mainline >> - Fixed a missing port unlock in IDE >> - Add SCSI ufs to the BSG conversions >> - Add patch to remove bsg-lib queue hook dependencies >> - Fixed missing clear of IO contexts >> - Added blk-mq backend for blk_lld_busy() >> >> Documentation/block/biodoc.txt | 88 - >> Documentation/block/cfq-iosched.txt| 291 -- >> Documentation/scsi/scsi-parameters.txt |5 - >> block/Kconfig |6 - >> block/Kconfig.iosched | 61 - >> block/Makefile |5 +- >> block/bfq-iosched.c|1 - >> block/blk-cgroup.c | 55 - >> block/blk-core.c | 1836 +--- >> block/blk-exec.c | 20 +- >> block/blk-flush.c | 154 +- >> block/blk-ioc.c| 33 +- >> block/blk-merge.c | 35 +- >> block/blk-mq-debugfs.c |2 - >> block/blk-mq-tag.c |6 +- >> block/blk-mq.c | 30 +- >> block/blk-settings.c | 55 - >> block/blk-softirq.c| 24 +- >> block/blk-sysfs.c | 39 +- >> block/blk-tag.c| 378 --- >> block/blk-timeout.c| 99 +- >> block/blk-wbt.c|3 +- >> block/blk.h| 60 +- >> block/bsg-lib.c| 146 +- >> block/cfq-iosched.c| 4916 >> >> block/deadline-iosched.c | 560 >> block/elevator.c | 447 +-- >> block/kyber-iosched.c |1 - >> block/mq-deadline.c|1 - >> block/noop-iosched.c | 124 - >> drivers/block/sunvdc.c | 149 +- >> drivers/ide/ide-atapi.c| 25 +- >> drivers/ide/ide-cd.c | 175 +- >> drivers/ide/ide-disk.c |5 +- >> drivers/ide/ide-io.c | 100 +- >> drivers/ide/ide-park.c |4 +- >> drivers/ide/ide-pm.c | 28 +- >> drivers/ide/ide-probe.c| 68 +- >> drivers/memstick/core/ms_block.c | 110 +- >> drivers/memstick/core/ms_block.h |1 + >> drivers/memstick/core/mspro_block.c| 121 +- >> drivers/s390/block/dasd_ioctl.c| 22 +- >> drivers/scsi/Kconfig | 12 - >> drivers/scsi/cxlflash/main.c |6 - >> drivers/scsi/hosts.c | 29 +- >> drivers/scsi/lpfc/lpfc_scsi.c |2 +- >> drivers/scsi/osd/osd_initiator.c |4 +- >> drivers/scsi/osst.c|2 +- >> drivers/scsi/qedi/qedi_main.c |3 +- >> drivers/scsi/qla2xxx/qla_os.c | 30 +- >> drivers/scsi/scsi.c|5 +- >> drivers/scsi/scsi_debug.c |3 +- >> drivers/scsi/scsi_error.c |4 +- >> drivers/scsi/scsi_lib.c| 599 +--- >> drivers/scsi/scsi_priv.h |1 - >> drivers/scsi/scsi_scan.c | 10 +- >> drivers/scsi/scsi_sysfs.c |8 +- >> drivers/scsi/scsi_transport_fc.c | 71 +- >> drivers/scsi/scsi_transport_iscsi.c|7 +- >> drivers/scsi/scsi_transport_sas.c | 10 +- >> drivers/scsi/sg.c |2 +- >> drivers/scsi/st.c |2 +- >> drivers/scsi/ufs/ufs_bsg.c |4 +- >> drivers/scsi/ufs/ufshcd.c |6 - >> drivers/target/target_core_pscsi.c |2 +- >> include/linux/blk-cgroup.h | 108 - >> include/linux/blk-mq.h |9 +- >> include/linux/blkdev.h | 179 +- >> include/linux/bsg-lib.h|6 +- >> include/linux/elevator.h | 90 +- >> include/linux/ide.h| 13 +- >> include/linux/init.h |1 - >> include/scsi/scsi_host.h | 18 +- >> include/scsi/scsi_tcq.h| 14 +- >> init/do_mounts_initrd.c|3 - >>
Re: [PATCH] scsi/aic94xx/aic94xx_hwi.c: Use dma_pool_zalloc
On Wed, Oct 31, 2018 at 9:19 PM, Souptick Joarder wrote:
> Replaced dma_pool_alloc + memset with dma_pool_zalloc
>
> Signed-off-by: Brajeswar Ghosh
> Signed-off-by: Souptick Joarder
Reviewed-by: Kees Cook
-Kees
> ---
> drivers/scsi/aic94xx/aic94xx_hwi.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/aic94xx/aic94xx_hwi.c
> b/drivers/scsi/aic94xx/aic94xx_hwi.c
> index 3b8ad55e59de..2bc7615193bd 100644
> --- a/drivers/scsi/aic94xx/aic94xx_hwi.c
> +++ b/drivers/scsi/aic94xx/aic94xx_hwi.c
> @@ -1057,14 +1057,13 @@ static struct asd_ascb *asd_ascb_alloc(struct
> asd_ha_struct *asd_ha,
>
> if (ascb) {
> ascb->dma_scb.size = sizeof(struct scb);
> - ascb->dma_scb.vaddr = dma_pool_alloc(asd_ha->scb_pool,
> + ascb->dma_scb.vaddr = dma_pool_zalloc(asd_ha->scb_pool,
> gfp_flags,
>
> &ascb->dma_scb.dma_handle);
> if (!ascb->dma_scb.vaddr) {
> kmem_cache_free(asd_ascb_cache, ascb);
> return NULL;
> }
> - memset(ascb->dma_scb.vaddr, 0, sizeof(struct scb));
> asd_init_ascb(asd_ha, ascb);
>
> spin_lock_irqsave(&seq->tc_index_lock, flags);
> --
> 2.17.1
>
--
Kees Cook
Re: [PATCH 09/30] scsi: kill off the legacy IO path
On Wed, Oct 31, 2018 at 11:59:01AM -0600, Jens Axboe wrote:
> Cc: linux-scsi@vger.kernel.org
> Acked-by: Himanshu Madhani
> Reviewed-by: Hannes Reinecke
> Signed-off-by: Jens Axboe
A bunch of really trivial nitpicks below, only to prove that I read the
thing ;)
Reviewed-by: Omar Sandoval
> ---
> Documentation/scsi/scsi-parameters.txt | 5 -
> drivers/scsi/Kconfig | 12 -
> drivers/scsi/cxlflash/main.c | 6 -
> drivers/scsi/hosts.c | 29 +-
> drivers/scsi/lpfc/lpfc_scsi.c | 2 +-
> drivers/scsi/qedi/qedi_main.c | 3 +-
> drivers/scsi/qla2xxx/qla_os.c | 30 +-
> drivers/scsi/scsi.c| 5 +-
> drivers/scsi/scsi_debug.c | 3 +-
> drivers/scsi/scsi_error.c | 2 +-
> drivers/scsi/scsi_lib.c| 603 ++---
> drivers/scsi/scsi_priv.h | 1 -
> drivers/scsi/scsi_scan.c | 10 +-
> drivers/scsi/scsi_sysfs.c | 8 +-
> drivers/scsi/ufs/ufshcd.c | 6 -
> include/scsi/scsi_host.h | 18 +-
> include/scsi/scsi_tcq.h| 14 +-
> 17 files changed, 77 insertions(+), 680 deletions(-)
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 8794e54f43a9..3e2665c66bc4 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -857,13 +857,9 @@ qla2xxx_queuecommand(struct Scsi_Host *host, struct
> scsi_cmnd *cmd)
> }
>
> if (ha->mqenable) {
> - if (shost_use_blk_mq(vha->host)) {
> - tag = blk_mq_unique_tag(cmd->request);
> - hwq = blk_mq_unique_tag_to_hwq(tag);
> - qpair = ha->queue_pair_map[hwq];
> - } else if (vha->vp_idx && vha->qpair) {
> - qpair = vha->qpair;
> - }
> + tag = blk_mq_unique_tag(cmd->request);
> + hwq = blk_mq_unique_tag_to_hwq(tag);
> + qpair = ha->queue_pair_map[hwq];
>
> if (qpair)
> return qla2xxx_mqueuecommand(host, cmd, qpair);
> @@ -3153,7 +3149,7 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct
> pci_device_id *id)
> goto probe_failed;
> }
>
> - if (ha->mqenable && shost_use_blk_mq(host)) {
> + if (ha->mqenable) {
> /* number of hardware queues supported by blk/scsi-mq*/
> host->nr_hw_queues = ha->max_qpairs;
>
> @@ -3265,25 +3261,17 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct
> pci_device_id *id)
> base_vha->mgmt_svr_loop_id, host->sg_tablesize);
>
> if (ha->mqenable) {
> - bool mq = false;
> bool startit = false;
>
> - if (QLA_TGT_MODE_ENABLED()) {
> - mq = true;
> + if (QLA_TGT_MODE_ENABLED())
> startit = false;
> - }
>
> - if ((ql2x_ini_mode == QLA2XXX_INI_MODE_ENABLED) &&
> - shost_use_blk_mq(host)) {
> - mq = true;
> + if (ql2x_ini_mode == QLA2XXX_INI_MODE_ENABLED)
> startit = true;
> - }
This could just be
startit = (QLA_TGT_MODE_ENABLED() ||
(ql2x_ini_mode == QLA2XXX_INI_MODE_ENABLED));
>
> - if (mq) {
> - /* Create start of day qpairs for Block MQ */
> - for (i = 0; i < ha->max_qpairs; i++)
> - qla2xxx_create_qpair(base_vha, 5, 0, startit);
> - }
> + /* Create start of day qpairs for Block MQ */
> + for (i = 0; i < ha->max_qpairs; i++)
> + qla2xxx_create_qpair(base_vha, 5, 0, startit);
> }
>
> if (ha->flags.running_gold_fw)
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index fc1356d101b0..99db3f4316b5 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -780,11 +780,8 @@ MODULE_LICENSE("GPL");
> module_param(scsi_logging_level, int, S_IRUGO|S_IWUSR);
> MODULE_PARM_DESC(scsi_logging_level, "a bit mask of logging levels");
>
> -#ifdef CONFIG_SCSI_MQ_DEFAULT
> +/* Kill module parameter */
Is this a leftover todo comment for yourself, or a note for the future?
If the latter, I think it could be clearer.
> bool scsi_use_blk_mq = true;
> -#else
> -bool scsi_use_blk_mq = false;
> -#endif
> module_param_named(use_blk_mq, scsi_use_blk_mq, bool, S_IWUSR | S_IRUGO);
>
> static int __init init_scsi(void)
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 60bcc6df97a9..4740f1e9dd17 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -5881,8 +5881,7 @@ static int sdebug_driver_probe(struct device *dev)
> }
> /* Decide whether to tell scsi subsystem that we want mq */
> /* Following should giv
Re: [PATCHSET v3 0/30] blk-mq driver conversions and legacy path removal
On Wed, Oct 31, 2018 at 11:58:52AM -0600, Jens Axboe wrote: > This patch series converts the remaining drivers to blk-mq. SCSI > supports both paths, this removes the legacy IO path from SCSI. At the > end, legacy IO code and schedulers are killed off. > > I'm not aware of any issues with this series. > > This patch series is on top of current -git. It can also be bound in > my mq-conversions branch. > > Changes since v2: > > - Kill q->softirq_done_fn() > > Changes since v1: > > - Fix removed q->mq_ops non-NULL check in wbt_enable_default() > - Remove spurious return in ide-io.c:ide_timer_expiry() > - Dropped DM legacy path removal patch, now in mainline > - Dropped ib_srp patch, now in mainline > - Fixed a missing port unlock in IDE > - Add SCSI ufs to the BSG conversions > - Add patch to remove bsg-lib queue hook dependencies > - Fixed missing clear of IO contexts > - Added blk-mq backend for blk_lld_busy() > > Documentation/block/biodoc.txt | 88 - > Documentation/block/cfq-iosched.txt| 291 -- > Documentation/scsi/scsi-parameters.txt |5 - > block/Kconfig |6 - > block/Kconfig.iosched | 61 - > block/Makefile |5 +- > block/bfq-iosched.c|1 - > block/blk-cgroup.c | 55 - > block/blk-core.c | 1836 +--- > block/blk-exec.c | 20 +- > block/blk-flush.c | 154 +- > block/blk-ioc.c| 33 +- > block/blk-merge.c | 35 +- > block/blk-mq-debugfs.c |2 - > block/blk-mq-tag.c |6 +- > block/blk-mq.c | 30 +- > block/blk-settings.c | 55 - > block/blk-softirq.c| 24 +- > block/blk-sysfs.c | 39 +- > block/blk-tag.c| 378 --- > block/blk-timeout.c| 99 +- > block/blk-wbt.c|3 +- > block/blk.h| 60 +- > block/bsg-lib.c| 146 +- > block/cfq-iosched.c| 4916 > > block/deadline-iosched.c | 560 > block/elevator.c | 447 +-- > block/kyber-iosched.c |1 - > block/mq-deadline.c|1 - > block/noop-iosched.c | 124 - > drivers/block/sunvdc.c | 149 +- > drivers/ide/ide-atapi.c| 25 +- > drivers/ide/ide-cd.c | 175 +- > drivers/ide/ide-disk.c |5 +- > drivers/ide/ide-io.c | 100 +- > drivers/ide/ide-park.c |4 +- > drivers/ide/ide-pm.c | 28 +- > drivers/ide/ide-probe.c| 68 +- > drivers/memstick/core/ms_block.c | 110 +- > drivers/memstick/core/ms_block.h |1 + > drivers/memstick/core/mspro_block.c| 121 +- > drivers/s390/block/dasd_ioctl.c| 22 +- > drivers/scsi/Kconfig | 12 - > drivers/scsi/cxlflash/main.c |6 - > drivers/scsi/hosts.c | 29 +- > drivers/scsi/lpfc/lpfc_scsi.c |2 +- > drivers/scsi/osd/osd_initiator.c |4 +- > drivers/scsi/osst.c|2 +- > drivers/scsi/qedi/qedi_main.c |3 +- > drivers/scsi/qla2xxx/qla_os.c | 30 +- > drivers/scsi/scsi.c|5 +- > drivers/scsi/scsi_debug.c |3 +- > drivers/scsi/scsi_error.c |4 +- > drivers/scsi/scsi_lib.c| 599 +--- > drivers/scsi/scsi_priv.h |1 - > drivers/scsi/scsi_scan.c | 10 +- > drivers/scsi/scsi_sysfs.c |8 +- > drivers/scsi/scsi_transport_fc.c | 71 +- > drivers/scsi/scsi_transport_iscsi.c|7 +- > drivers/scsi/scsi_transport_sas.c | 10 +- > drivers/scsi/sg.c |2 +- > drivers/scsi/st.c |2 +- > drivers/scsi/ufs/ufs_bsg.c |4 +- > drivers/scsi/ufs/ufshcd.c |6 - > drivers/target/target_core_pscsi.c |2 +- > include/linux/blk-cgroup.h | 108 - > include/linux/blk-mq.h |9 +- > include/linux/blkdev.h | 179 +- > include/linux/bsg-lib.h|6 +- > include/linux/elevator.h | 90 +- > include/linux/ide.h| 13 +- > include/linux/init.h |1 - > include/scsi/scsi_host.h | 18 +- > include/scsi/scsi_tcq.h| 14 +- > init/do_mounts_initrd.c|3 - > init/initramfs.c |6 - > init/main.c| 12 - > 77 files changed, 837 insertions
Re: [PATCH 21/30] block: remove dead elevator code
On Wed, Oct 31, 2018 at 11:59:13AM -0600, Jens Axboe wrote:
> This removes a bunch of core and elevator related code. On the core
> front, we remove anything related to queue running, draining,
> initialization, plugging, and congestions. We also kill anything
> related to request allocation, merging, retrieval, and completion.
>
> Remove any checking for single queue IO schedulers, as they no
> longer exist. This means we can also delete a bunch of code related
> to request issue, adding, completion, etc - and all the SQ related
> ops and helpers.
>
> Also kill the load_default_modules(), as all that did was provide
> for a way to load the default single queue elevator.
>
> Signed-off-by: Jens Axboe
[snip]
> -struct elevator_ops
> -{
> - elevator_merge_fn *elevator_merge_fn;
> - elevator_merged_fn *elevator_merged_fn;
> - elevator_merge_req_fn *elevator_merge_req_fn;
> - elevator_allow_bio_merge_fn *elevator_allow_bio_merge_fn;
> - elevator_allow_rq_merge_fn *elevator_allow_rq_merge_fn;
> - elevator_bio_merged_fn *elevator_bio_merged_fn;
> -
> - elevator_dispatch_fn *elevator_dispatch_fn;
> - elevator_add_req_fn *elevator_add_req_fn;
> - elevator_activate_req_fn *elevator_activate_req_fn;
> - elevator_deactivate_req_fn *elevator_deactivate_req_fn;
> -
> - elevator_completed_req_fn *elevator_completed_req_fn;
> -
> - elevator_request_list_fn *elevator_former_req_fn;
> - elevator_request_list_fn *elevator_latter_req_fn;
> -
> - elevator_init_icq_fn *elevator_init_icq_fn; /* see iocontext.h */
> - elevator_exit_icq_fn *elevator_exit_icq_fn; /* ditto */
> -
> - elevator_set_req_fn *elevator_set_req_fn;
> - elevator_put_req_fn *elevator_put_req_fn;
> -
> - elevator_may_queue_fn *elevator_may_queue_fn;
> -
> - elevator_init_fn *elevator_init_fn;
> - elevator_exit_fn *elevator_exit_fn;
> - elevator_registered_fn *elevator_registered_fn;
> -};
> -
> struct blk_mq_alloc_data;
> struct blk_mq_hw_ctx;
>
> @@ -138,16 +70,15 @@ struct elevator_type
>
> /* fields provided by elevator implementation */
> union {
> - struct elevator_ops sq;
> struct elevator_mq_ops mq;
> } ops;
> +
Lol. Maybe we should get rid of this union in a followup? At least until
we rewrite the block layer again.
Re: [PATCH 09/30] scsi: kill off the legacy IO path
On 11/1/18 3:11 PM, Omar Sandoval wrote:
>> -if ((ql2x_ini_mode == QLA2XXX_INI_MODE_ENABLED) &&
>> -shost_use_blk_mq(host)) {
>> -mq = true;
>> +if (ql2x_ini_mode == QLA2XXX_INI_MODE_ENABLED)
>> startit = true;
>> -}
>
> This could just be
>
> startit = (QLA_TGT_MODE_ENABLED() ||
> (ql2x_ini_mode == QLA2XXX_INI_MODE_ENABLED));
I agree, just didn't want to make changes that would be harder to
verify. So kept as much as the original style as possible.
>> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
>> index fc1356d101b0..99db3f4316b5 100644
>> --- a/drivers/scsi/scsi.c
>> +++ b/drivers/scsi/scsi.c
>> @@ -780,11 +780,8 @@ MODULE_LICENSE("GPL");
>> module_param(scsi_logging_level, int, S_IRUGO|S_IWUSR);
>> MODULE_PARM_DESC(scsi_logging_level, "a bit mask of logging levels");
>>
>> -#ifdef CONFIG_SCSI_MQ_DEFAULT
>> +/* Kill module parameter */
>
> Is this a leftover todo comment for yourself, or a note for the future?
> If the latter, I think it could be clearer.
It's just a note for the future. I'll improve it.
>> -unsigned long flags;
>> -
>> -if (bidi_bytes)
>> -scsi_release_bidi_buffers(cmd);
>> -scsi_release_buffers(cmd);
>> -scsi_put_command(cmd);
>> +/*
>> + * In the MQ case the command gets freed by __blk_mq_end_request,
>> + * so we have to do all cleanup that depends on it earlier.
>> + *
>> + * We also can't kick the queues from irq context, so we
>> + * will have to defer it to a workqueue.
>> + */
>
> This comment is slightly stale, since everything is the MQ case now.
Agree, but it's just copied over. Not a huge deal since it's generally
applicable, there's just no alternative :-)
>> @@ -367,7 +367,6 @@ store_shost_eh_deadline(struct device *dev, struct
>> device_attribute *attr,
>>
>> static DEVICE_ATTR(eh_deadline, S_IRUGO | S_IWUSR, show_shost_eh_deadline,
>> store_shost_eh_deadline);
>>
>> -shost_rd_attr(use_blk_mq, "%d\n");
>> shost_rd_attr(unique_id, "%u\n");
>> shost_rd_attr(cmd_per_lun, "%hd\n");
>> shost_rd_attr(can_queue, "%hd\n");
>> @@ -386,6 +385,13 @@ show_host_busy(struct device *dev, struct
>> device_attribute *attr, char *buf)
>> }
>> static DEVICE_ATTR(host_busy, S_IRUGO, show_host_busy, NULL);
>>
>> +static ssize_t
>> +show_use_blk_mq(struct device *dev, struct device_attribute *attr, char
>> *buf)
>> +{
>> +return snprintf(buf, 20, "1\n");
>> +}
>
> Looks like you forgot to change this to sprintf()
Indeed, I'll make that change.
Thanks for the review!
--
Jens Axboe
Re: [PATCH 21/30] block: remove dead elevator code
On 11/1/18 3:26 PM, Omar Sandoval wrote:
> On Wed, Oct 31, 2018 at 11:59:13AM -0600, Jens Axboe wrote:
>> This removes a bunch of core and elevator related code. On the core
>> front, we remove anything related to queue running, draining,
>> initialization, plugging, and congestions. We also kill anything
>> related to request allocation, merging, retrieval, and completion.
>>
>> Remove any checking for single queue IO schedulers, as they no
>> longer exist. This means we can also delete a bunch of code related
>> to request issue, adding, completion, etc - and all the SQ related
>> ops and helpers.
>>
>> Also kill the load_default_modules(), as all that did was provide
>> for a way to load the default single queue elevator.
>>
>> Signed-off-by: Jens Axboe
>
> [snip]
>
>> -struct elevator_ops
>> -{
>> -elevator_merge_fn *elevator_merge_fn;
>> -elevator_merged_fn *elevator_merged_fn;
>> -elevator_merge_req_fn *elevator_merge_req_fn;
>> -elevator_allow_bio_merge_fn *elevator_allow_bio_merge_fn;
>> -elevator_allow_rq_merge_fn *elevator_allow_rq_merge_fn;
>> -elevator_bio_merged_fn *elevator_bio_merged_fn;
>> -
>> -elevator_dispatch_fn *elevator_dispatch_fn;
>> -elevator_add_req_fn *elevator_add_req_fn;
>> -elevator_activate_req_fn *elevator_activate_req_fn;
>> -elevator_deactivate_req_fn *elevator_deactivate_req_fn;
>> -
>> -elevator_completed_req_fn *elevator_completed_req_fn;
>> -
>> -elevator_request_list_fn *elevator_former_req_fn;
>> -elevator_request_list_fn *elevator_latter_req_fn;
>> -
>> -elevator_init_icq_fn *elevator_init_icq_fn; /* see iocontext.h */
>> -elevator_exit_icq_fn *elevator_exit_icq_fn; /* ditto */
>> -
>> -elevator_set_req_fn *elevator_set_req_fn;
>> -elevator_put_req_fn *elevator_put_req_fn;
>> -
>> -elevator_may_queue_fn *elevator_may_queue_fn;
>> -
>> -elevator_init_fn *elevator_init_fn;
>> -elevator_exit_fn *elevator_exit_fn;
>> -elevator_registered_fn *elevator_registered_fn;
>> -};
>> -
>> struct blk_mq_alloc_data;
>> struct blk_mq_hw_ctx;
>>
>> @@ -138,16 +70,15 @@ struct elevator_type
>>
>> /* fields provided by elevator implementation */
>> union {
>> -struct elevator_ops sq;
>> struct elevator_mq_ops mq;
>> } ops;
>> +
>
> Lol. Maybe we should get rid of this union in a followup? At least until
> we rewrite the block layer again.
Totally agree, wanted to do that in a followup patch, but just
haven't gotten around to it. I'll tack one on.
--
Jens Axboe
Re: [PATCH] scsi/aic94xx/aic94xx_hwi.c: Use dma_pool_zalloc
On Fri, Nov 2, 2018 at 2:25 AM Kees Cook wrote:
>
> On Wed, Oct 31, 2018 at 9:19 PM, Souptick Joarder
> wrote:
> > Replaced dma_pool_alloc + memset with dma_pool_zalloc
> >
> > Signed-off-by: Brajeswar Ghosh
> > Signed-off-by: Souptick Joarder
>
> Reviewed-by: Kees Cook
Thanks Kees.
>
> -Kees
>
> > ---
> > drivers/scsi/aic94xx/aic94xx_hwi.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/scsi/aic94xx/aic94xx_hwi.c
> > b/drivers/scsi/aic94xx/aic94xx_hwi.c
> > index 3b8ad55e59de..2bc7615193bd 100644
> > --- a/drivers/scsi/aic94xx/aic94xx_hwi.c
> > +++ b/drivers/scsi/aic94xx/aic94xx_hwi.c
> > @@ -1057,14 +1057,13 @@ static struct asd_ascb *asd_ascb_alloc(struct
> > asd_ha_struct *asd_ha,
> >
> > if (ascb) {
> > ascb->dma_scb.size = sizeof(struct scb);
> > - ascb->dma_scb.vaddr = dma_pool_alloc(asd_ha->scb_pool,
> > + ascb->dma_scb.vaddr = dma_pool_zalloc(asd_ha->scb_pool,
> > gfp_flags,
> >
> > &ascb->dma_scb.dma_handle);
> > if (!ascb->dma_scb.vaddr) {
> > kmem_cache_free(asd_ascb_cache, ascb);
> > return NULL;
> > }
> > - memset(ascb->dma_scb.vaddr, 0, sizeof(struct scb));
> > asd_init_ascb(asd_ha, ascb);
> >
> > spin_lock_irqsave(&seq->tc_index_lock, flags);
> > --
> > 2.17.1
> >
>
>
>
> --
> Kees Cook
