| Message ID | 1628862553-179450-1-git-send-email-john.garry@huawei.com |
|---|---|
| Headers | show |
| Series | Remove scsi_cmnd.tag | expand |
The whole series looks good to me:
Reviewed-by: Christoph Hellwig <hch@lst.de>
John,
> There is no need for scsi_cmnd.tag, so remove it.
Applied to 5.15/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
On 16/08/2021 18:34, Martin K. Petersen wrote: > > John, > >> There is no need for scsi_cmnd.tag, so remove it. > > Applied to 5.15/scsi-staging, thanks! > Hi Martin, As you may have seen, some arm32 build has also broken. My build coverage was not good enough, and I don't see a point in me playing whack-a-mole with the kernel test robot. So how about revert the final patch or even all of them, and I can try get better build coverage and then re-post? Or maybe you or Bart have a better idea? Thanks!
John, > As you may have seen, some arm32 build has also broken. My build > coverage was not good enough, and I don't see a point in me playing > whack-a-mole with the kernel test robot. So how about revert the final > patch or even all of them, and I can try get better build coverage and > then re-post? Or maybe you or Bart have a better idea? My due diligence involved: $ git grep -Ei -- "cmn?d->tag" drivers/scsi But in retrospect that should have been: $ git grep -Ei -- "(cmn?d|scpnt)->tag" drivers/scsi I cringe every time I see "SCpnt" so maybe that's why I didn't think of it. Anyway. Please fix the drivers/scsi/arm bits up. There is still time. -- Martin K. Petersen Oracle Linux Engineering
On 8/18/21 11:08 AM, John Garry wrote:
> Or maybe you or Bart have a better idea?
This is how I test compilation of SCSI drivers on a SUSE system (only
the cross-compilation prefix is distro specific):
# Acorn RiscPC
make ARCH=arm xconfig
# Select the RiscPC architecture (ARCH_RPC)
make -j9 ARCH=arm CROSS_COMPILE=arm-suse-linux-gnueabi- </dev/null
# Atari, Amiga
make ARCH=m68k xconfig<br>
# Select Amiga + Atari + 68060 + Q40 + SCSI + Zorro +
# SCSI_FDOMAIN_ISA
make -j9 ARCH=m68k CROSS_COMPILE=m68k-suse-linux- </dev/null
# MIPS
make ARCH=powerpc xconfig<br>
# Select the SGI IP28 machine type and also the WD93C93 SCSI
# driver
make -j9 ARCH=mips CROSS_COMPILE=mips-suse-linux- </dev/null
# PowerPC
make ARCH=powerpc xconfig<br>
# Select the ibmvfc and ibmvscsi drivers<br>
make -j9 ARCH=powerpc CROSS_COMPILE=powerpc64-suse-linux- \
</dev/null
# S/390
make ARCH=s390 xconfig
# Select the zfcp driver
make -j9 ARCH=s390 CROSS_COMPILE=s390x-suse-linux- </dev/null
Bart.
Hey Bart, Thanks for this! Really helpful. Just a tiny wee snag: On 8/19/21 4:41 AM, Bart Van Assche wrote: > On 8/18/21 11:08 AM, John Garry wrote: >> Or maybe you or Bart have a better idea? > > This is how I test compilation of SCSI drivers on a SUSE system (only > the cross-compilation prefix is distro specific): > > # Acorn RiscPC > make ARCH=arm xconfig > # Select the RiscPC architecture (ARCH_RPC) > make -j9 ARCH=arm CROSS_COMPILE=arm-suse-linux-gnueabi- </dev/null > Acorn RiscPC is ARMv3, which sadly isn't supported anymore with gcc9. So for compilation I had to modify Kconfig to select ARMv4: diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig index 8355c3895894..22ec9e275335 100644 --- a/arch/arm/mm/Kconfig +++ b/arch/arm/mm/Kconfig @@ -278,7 +278,7 @@ config CPU_ARM1026 # SA110 config CPU_SA110 bool - select CPU_32v3 if ARCH_RPC + select CPU_32v4 if ARCH_RPC select CPU_32v4 if !ARCH_RPC select CPU_ABRT_EV4 select CPU_CACHE_V4WB Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
On 19/08/2021 08:15, Hannes Reinecke wrote: > Hey Bart, > > Thanks for this! > Really helpful. > > Just a tiny wee snag: > > On 8/19/21 4:41 AM, Bart Van Assche wrote: >> On 8/18/21 11:08 AM, John Garry wrote: >>> Or maybe you or Bart have a better idea? >> >> This is how I test compilation of SCSI drivers on a SUSE system (only >> the cross-compilation prefix is distro specific): >> >> # Acorn RiscPC >> make ARCH=arm xconfig >> # Select the RiscPC architecture (ARCH_RPC) >> make -j9 ARCH=arm CROSS_COMPILE=arm-suse-linux-gnueabi- </dev/null >> > > Acorn RiscPC is ARMv3, which sadly isn't supported anymore with gcc9. > So for compilation I had to modify Kconfig to select ARMv4: > Yeah, that is what I was tackling this very moment. > diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig > index 8355c3895894..22ec9e275335 100644 > --- a/arch/arm/mm/Kconfig > +++ b/arch/arm/mm/Kconfig > @@ -278,7 +278,7 @@ config CPU_ARM1026 > # SA110 > config CPU_SA110 > bool > - select CPU_32v3 if ARCH_RPC > + select CPU_32v4 if ARCH_RPC Does that build fully for xconfig or any others which you tried? > select CPU_32v4 if !ARCH_RPC > select CPU_ABRT_EV4 > select CPU_CACHE_V4WB > Thanks to all!
On 8/19/21 9:27 AM, John Garry wrote: > On 19/08/2021 08:15, Hannes Reinecke wrote: >> Hey Bart, >> >> Thanks for this! >> Really helpful. >> >> Just a tiny wee snag: >> >> On 8/19/21 4:41 AM, Bart Van Assche wrote: >>> On 8/18/21 11:08 AM, John Garry wrote: >>>> Or maybe you or Bart have a better idea? >>> >>> This is how I test compilation of SCSI drivers on a SUSE system (only >>> the cross-compilation prefix is distro specific): >>> >>> # Acorn RiscPC >>> make ARCH=arm xconfig >>> # Select the RiscPC architecture (ARCH_RPC) >>> make -j9 ARCH=arm CROSS_COMPILE=arm-suse-linux-gnueabi- </dev/null >>> >> >> Acorn RiscPC is ARMv3, which sadly isn't supported anymore with gcc9. >> So for compilation I had to modify Kconfig to select ARMv4: >> > > Yeah, that is what I was tackling this very moment. > >> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig >> index 8355c3895894..22ec9e275335 100644 >> --- a/arch/arm/mm/Kconfig >> +++ b/arch/arm/mm/Kconfig >> @@ -278,7 +278,7 @@ config CPU_ARM1026 >> # SA110 >> config CPU_SA110 >> bool >> - select CPU_32v3 if ARCH_RPC >> + select CPU_32v4 if ARCH_RPC > > Does that build fully for xconfig or any others which you tried? > Yep, xconfig and full build works. Well. Would've worked if you hadn't messed up tag handling for acornscsi :-) Besides: tag handling in acornscsi (and fas216, for that matter) seems to be completely broken. Consider this beauty: #ifdef CONFIG_SCSI_ACORNSCSI_TAGGED_QUEUE /* * tagged queueing - allocate a new tag to this command */ if (SCpnt->device->simple_tags) { SCpnt->device->current_tag += 1; if (SCpnt->device->current_tag == 0) SCpnt->device->current_tag = 1; SCpnt->tag = SCpnt->device->current_tag; } else #endif which is broken on _soo many_ counts. Not only does it try to allocate its own tags, the code also assumes that a tag value of '0' indicates that tagged queueing is not active: static void acornscsi_abortcmd(AS_Host *host, unsigned char tag) { host->scsi.phase = PHASE_ABORTED; sbic_arm_write(host, SBIC_CMND, CMND_ASSERTATN); msgqueue_flush(&host->scsi.msgs); #ifdef CONFIG_SCSI_ACORNSCSI_TAGGED_QUEUE if (tag) msgqueue_addmsg(&host->scsi.msgs, 2, ABORT_TAG, tag); else #endif msgqueue_addmsg(&host->scsi.msgs, 1, ABORT); } And, of course, there's the usual confusion about when to check for sdev->tagged_supported and sdev->simple_tags. Drop me a note if I can give a hand. Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
On 19/08/2021 08:50, Hannes Reinecke wrote: >>> select CPU_32v4 if ARCH_RPC >> >> Does that build fully for xconfig or any others which you tried? >> > > Yep, xconfig and full build works. > > Well. > > Would've worked if you hadn't messed up tag handling for acornscsi :-) > > Besides: tag handling in acornscsi (and fas216, for that matter) seems > to be completely broken. > > Consider this beauty: > > #ifdef CONFIG_SCSI_ACORNSCSI_TAGGED_QUEUE > /* > * tagged queueing - allocate a new tag to this command > */ > if (SCpnt->device->simple_tags) { > SCpnt->device->current_tag += 1; > if (SCpnt->device->current_tag == 0) > SCpnt->device->current_tag = 1; > SCpnt->tag = SCpnt->device->current_tag; > } else > #endif So isn't this just using the scsi_cmnd.tag as it own scribble? > > which is broken on _soo many_ counts. > Not only does it try to allocate its own tags, the code also assumes > that a tag value of '0' indicates that tagged queueing is not active: > In case you missed it, Arnd B tried to clear out some old platforms earlier this year. With respect to rpc, Russell King apparently still uses it and has some SCSI patches: https://lore.kernel.org/lkml/20210109174357.GB1551@shell.armlinux.org.uk/ I wonder what they are and maybe we can check. Anyway... I'd run any changes by him... > static > void acornscsi_abortcmd(AS_Host *host, unsigned char tag) > { > host->scsi.phase = PHASE_ABORTED; > sbic_arm_write(host, SBIC_CMND, CMND_ASSERTATN); > > msgqueue_flush(&host->scsi.msgs); > #ifdef CONFIG_SCSI_ACORNSCSI_TAGGED_QUEUE > if (tag) > msgqueue_addmsg(&host->scsi.msgs, 2, ABORT_TAG, tag); > else > #endif > msgqueue_addmsg(&host->scsi.msgs, 1, ABORT); > } > > And, of course, there's the usual confusion about when to check for > sdev->tagged_supported and sdev->simple_tags. > > Drop me a note if I can give a hand. Thanks! Let's see what happens to the series which you just sent.
On Fri, 13 Aug 2021 21:49:10 +0800, John Garry wrote: > There is no need for scsi_cmnd.tag, so remove it. > > Based on next-20210811 > > John Garry (3): > scsi: wd719: Stop using scsi_cmnd.tag > scsi: fnic: Stop setting scsi_cmnd.tag > scsi: Remove scsi_cmnd.tag > > [...] Applied to 5.15/scsi-queue, thanks! [1/3] scsi: wd719: Stop using scsi_cmnd.tag https://git.kernel.org/mkp/scsi/c/e2a1dc571e19 [2/3] scsi: fnic: Stop setting scsi_cmnd.tag https://git.kernel.org/mkp/scsi/c/e0aebd25fdd9 [3/3] scsi: Remove scsi_cmnd.tag https://git.kernel.org/mkp/scsi/c/4c7b6ea336c1 -- Martin K. Petersen Oracle Linux Engineering
On 24/08/2021 05:03, Martin K. Petersen wrote: > On Fri, 13 Aug 2021 21:49:10 +0800, John Garry wrote: > >> There is no need for scsi_cmnd.tag, so remove it. >> >> Based on next-20210811 >> >> John Garry (3): >> scsi: wd719: Stop using scsi_cmnd.tag >> scsi: fnic: Stop setting scsi_cmnd.tag >> scsi: Remove scsi_cmnd.tag >> >> [...] > > Applied to 5.15/scsi-queue, thanks! > > [1/3] scsi: wd719: Stop using scsi_cmnd.tag > https://git.kernel.org/mkp/scsi/c/e2a1dc571e19 > [2/3] scsi: fnic: Stop setting scsi_cmnd.tag > https://git.kernel.org/mkp/scsi/c/e0aebd25fdd9 > [3/3] scsi: Remove scsi_cmnd.tag > https://git.kernel.org/mkp/scsi/c/4c7b6ea336c1 > Thanks, but we still have the issue with the arm drivers. I'll ping Russell again if he doesn't reply soon. Hannes also sent a series - that may be the way forward, but need Russell involved. John