linux-kernel - Re: [v5 2/4] mpt3sas: Fix Sync cache command failure during driver unload

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <CAHp75VfENTJjJVMyQ1wnD4GXYKjhgr6h+wocPOfk16p9nWrpeg@mail.gmail.com>
Date:   Thu, 18 Oct 2018 10:23:18 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Suganath Prabu Subramani <suganath-prabu.subramani@...adcom.com>
Cc:     Bjorn Helgaas <helgaas@...nel.org>, Lukas Wunner <lukas@...ner.de>,
        linux-scsi <linux-scsi@...r.kernel.org>,
        linux-pci@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        ruscur@...sell.cc, sbobroff@...ux.ibm.com,
        Oliver <oohall@...il.com>,
        Sathya Prakash <Sathya.Prakash@...adcom.com>,
        Sreekanth Reddy <sreekanth.reddy@...adcom.com>
Subject: Re: [v5 2/4] mpt3sas: Fix Sync cache command failure during driver unload

On Thu, Oct 18, 2018 at 10:11 AM Suganath Prabu Subramani
<suganath-prabu.subramani@...adcom.com> wrote:
>
>
>
> On Wed, Oct 17, 2018 at 2:02 PM Andy Shevchenko <andy.shevchenko@...il.com> wrote:
>>
>> On Wed, Oct 17, 2018 at 8:59 AM Suganath Prabu
>> <suganath-prabu.subramani@...adcom.com> wrote:
>> >
>> > This is to fix Sync cache and start stop command
>> >  failures with DID_NO_CONNECT during driver unload.
>> >
>> > 1) Release drives first from SML, then remove internally
>> > in driver.
>> > 2) And allow sync cache and Start stop commands to firmware,
>> > even when remove_host flag is set.
>>
>> > +       if (ioc->hba_mpi_version_belonged == MPI2_VERSION) {
>> > +               if (ioc->remove_host)
>> > +                       return false;
>> > +
>> > +               return true;
>> > +       }
>> > +
>> > +       if (ioc->remove_host) {
>> > +
>> > +               switch (scmd->cmnd[0]) {
>> > +               case SYNCHRONIZE_CACHE:
>> > +               case START_STOP:
>> > +                       return true;
>> > +               default:
>> > +                       return false;
>> > +               }
>> > +       }
>> > +
>> > +       return true;
>>
>> Wouldn't be the same as

>> Yes it is same, but i feel original code is more readable.

OK (I didn't compare assembly, but I assume this is slow path).

>> if (!ioc->remove_host || ioc->hba_mpi_version_belonged == MPI2_VERSION)
>>    return !ioc->remove_host;
>>
>>  switch (scmd->cmnd[0]) {
>>  case SYNCHRONIZE_CACHE:
>>  case START_STOP:
>>    return true;
>>  default:
>>    return false;
>>  }
>>
>> ?
>>
>> --
>> With Best Regards,
>> Andy Shevchenko



-- 
With Best Regards,
Andy Shevchenko