linux-kernel - Re: [v6 1/4] mpt3sas: Separate out mpt3sas_wait_for_ioc_to

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 30 Oct 2018 08:22:32 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Suganath Prabu <suganath-prabu.subramani@...adcom.com>
Cc:     lukas@...ner.de, linux-scsi@...r.kernel.org,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        benh@...nel.crashing.org, ruscur@...sell.cc,
        sbobroff@...ux.ibm.com, oohall@...il.com,
        andy.shevchenko@...il.com, Sathya.Prakash@...adcom.com,
        sreekanth.reddy@...adcom.com
Subject: Re: [v6 1/4] mpt3sas: Separate out
 mpt3sas_wait_for_ioc_to_operational

On Tue, Oct 23, 2018 at 03:51:18PM +0530, Suganath Prabu wrote:
> No functional changes. This section of code
>  "wait for IOC to be operational" is used in many places
>  across the driver, and hence moved this code in to
>  a function "mpt3sas_wait_for_ioc_to_operational()"
> 
> Signed-off-by: Suganath Prabu <suganath-prabu.subramani@...adcom.com>

I'm not really a reviewer of mpt3sas, but since I commented earlier,
I took a quick look at this.  Minor comments:

  - Remove leading spaces from changelog and reformat to use the whole
    width, e.g.,

      No functional changes. This section of code "wait for IOC to be
      operational" is used in many places across the driver.  Factor
      this code out into a new mpt3sas_wait_for_ioc_to_operational().

  - "mpt3sas_wait_for_ioc_to_operational" isn't really grammatical in
    English; maybe "mpt3sas_wait_for_ioc" would be better?

  - It would be slightly easier to review this if it were split into three
    patches: (1) add and use the IOC_OPERATIONAL_WAIT_COUNT #define, (2)
    factor out mpt3sas_wait_for_ioc() using the exact same code structure
    (the while loop), and (3) convert mpt3sas_wait_for_ioc() to use the do
    loop instead of the while.  The idea is that each individual patch is
    simpler and more obvious because it only contains a single logical
    change.

Reviewed-by: Bjorn Helgaas <bhelgaas@...gle.com>