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  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 12 Jan 2021 16:00:57 +0000
From:   John Garry <john.garry@...wei.com>
To:     "Ahmed S. Darwish" <a.darwish@...utronix.de>
CC:     "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Jason Yan <yanaijie@...wei.com>,
        Daniel Wagner <dwagner@...e.de>,
        Artur Paszkiewicz <artur.paszkiewicz@...el.com>,
        Jack Wang <jinpu.wang@...ud.ionos.com>,
        <linux-scsi@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "Sebastian A. Siewior" <bigeasy@...utronix.de>
Subject: Re: [PATCH v2 00/19] scsi: libsas: Remove in_interrupt() check

- intel-linux-scu@...el.com

On 12/01/2021 13:19, Ahmed S. Darwish wrote:
> On Tue, Jan 12, 2021 at 11:53:50AM +0000, John Garry wrote:
>> On 12/01/2021 11:06, Ahmed S. Darwish wrote:
>>> Hi,
>>>
>>> Changelog v2
>>> ------------
> ...
>>
>> I'll give this a spin today and help review also then.
>>

I boot-tested on my machines which have hisi_sas v2 and v3 hw, and it's 
ok. I will ask some guys to test a bit more.

And generally the changes look ok. But I just have a slight concern that 
we don't pass the gfp_flags all the way from the origin caller.

So we have some really long callchains, for example:

host.c: sci_controller_error_handler(): atomic, irq handler     (*)
OR host.c: sci_controller_completion_handler(), atomic, tasklet (*)
   -> sci_controller_process_completions()
     -> sci_controller_unsolicited_frame()
       -> phy.c: sci_phy_frame_handler()
         -> sci_change_state(SCI_PHY_SUB_AWAIT_SAS_POWER)
           -> sci_phy_starting_await_sas_power_substate_enter()
             -> host.c: sci_controller_power_control_queue_insert()
               -> phy.c: sci_phy_consume_power_handler()
                 -> sci_change_state(SCI_PHY_SUB_FINAL)
         -> sci_change_state(SCI_PHY_SUB_FINAL)
     -> sci_controller_event_completion()
       -> phy.c: sci_phy_event_handler()
         -> sci_phy_start_sata_link_training()
           -> sci_change_state(SCI_PHY_SUB_AWAIT_SATA_POWER)
             -> sci_phy_starting_await_sata_power_substate_enter
               -> host.c: sci_controller_power_control_queue_insert()
                 -> phy.c: sci_phy_consume_power_handler()
                   -> sci_change_state(SCI_PHY_SUB_FINAL)

So if someone rearranges the code later, adds new callchains, etc., it 
could be missed that the context may have changed than what we assume at 
the bottom. But then passing the flags everywhere is cumbersome, and all 
the libsas users see little or no significant changes anyway, apart from 
a couple.

Thanks,
John

Powered by blists - more mailing lists