Re: [PATCH 0/3] Remove scsi_cmnd.tag

[Hannes Reinecke]

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@xxxxxxx                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer