From: John Garry <john.garry@huawei.com> To: Arnd Bergmann <arnd@arndb.de> Cc: <James.Bottomley@hansenpartnership.com>, <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>, <linuxarm@huawei.com>, <zhangfei.gao@linaro.org>, <linux-scsi@vger.kernel.org>, <xuwei5@hisilicon.com>, <john.garry2@mail.dcu.ie>, <hare@suse.de> Subject: Re: [PATCH 13/25] scsi: hisi_sas: add path from phyup irq to SAS framework Date: Mon, 19 Oct 2015 15:55:47 +0100 [thread overview] Message-ID: <56250473.9090700@huawei.com> (raw) In-Reply-To: <4269975.xfCo9ut8KK@wuerfel> >>> I'd have to review more closely, but I think that's fine, as this >>> is how most work queues are used: you can queue the same function >>> multiple times, and it's guaranteed to run at least once after >>> the last queue, so if you queue it while it's already running, >>> it will be called again, otherwise it won't. >>> >> In the scenario I described the issue is not that the second call to >> queue the work function is lost. The problem is that when we setup the >> second call we may overwrite elements of the phy's hisi_sas_wq struct >> which may be still being referenced in the work function for the first call. > > Do you mean these members? > >> + wq->event = PHYUP; >> + wq->hisi_hba = hisi_hba; >> + wq->phy_no = phy_no; > Yes, these are the members I was referring to. However there is also an element "data" in hisi_sas_wq. This is used in control phy as follows: int hisi_sas_control_phy(struct asd_sas_phy *sas_phy, enum phy_func func, void *funcdata) { ... wq->event = CONTROL_PHY; wq->data = func; ... INIT_WORK(&wq->work_struct, hisi_sas_wq_process); queue_work(hisi_hba->wq, &wq->work_struct); } void hisi_sas_wq_process(struct work_struct *work) { struct hisi_sas_wq *wq = container_of(work, struct hisi_sas_wq, work_struct); switch (wq->event) { case CONTROL_PHY: hisi_sas_control_phy_work(hisi_hba, wq->data, phy_no); } static void hisi_sas_control_phy_work(struct hisi_hba *hisi_hba, int func, int phy_no) { switch (func) { case PHY_FUNC_HARD_RESET: hard_phy_reset_v1_hw(hisi_hba, phy_no) > Sorry for being unclear, I implied getting rid of them, by having a > work queue per phy. You can create a structure for each phy like > > struct hisi_sas_phy { > struct hisi_sas *dev; /* pointer to the device */ > unsigned int phy_no; > struct work_struct phyup_work; > }; > > There are probably other things you can put into the same structure. > When the phy is brought up, you then just start the phyup work for > that phy, which can retrieve its hisi_sas_phy structure from the > work_struct pointer, and from there get to the device. > > Arnd > > . > We could create a work_struct for each event, which would be fine. However we would sometimes still need some way of passing data to the event, like the phy control example.
WARNING: multiple messages have this Message-ID (diff)
From: John Garry <john.garry-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> To: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> Cc: James.Bottomley-JuX6DAaQMKPCXq6kfMZ53/egYHeGw8Jk@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linuxarm-hv44wF8Li93QT0dZR+AlfA@public.gmane.org, zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, xuwei5-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org, john.garry2-s/0ZXS5h9803lw97EnAbAg@public.gmane.org, hare-l3A5Bk7waGM@public.gmane.org Subject: Re: [PATCH 13/25] scsi: hisi_sas: add path from phyup irq to SAS framework Date: Mon, 19 Oct 2015 15:55:47 +0100 [thread overview] Message-ID: <56250473.9090700@huawei.com> (raw) In-Reply-To: <4269975.xfCo9ut8KK@wuerfel> >>> I'd have to review more closely, but I think that's fine, as this >>> is how most work queues are used: you can queue the same function >>> multiple times, and it's guaranteed to run at least once after >>> the last queue, so if you queue it while it's already running, >>> it will be called again, otherwise it won't. >>> >> In the scenario I described the issue is not that the second call to >> queue the work function is lost. The problem is that when we setup the >> second call we may overwrite elements of the phy's hisi_sas_wq struct >> which may be still being referenced in the work function for the first call. > > Do you mean these members? > >> + wq->event = PHYUP; >> + wq->hisi_hba = hisi_hba; >> + wq->phy_no = phy_no; > Yes, these are the members I was referring to. However there is also an element "data" in hisi_sas_wq. This is used in control phy as follows: int hisi_sas_control_phy(struct asd_sas_phy *sas_phy, enum phy_func func, void *funcdata) { ... wq->event = CONTROL_PHY; wq->data = func; ... INIT_WORK(&wq->work_struct, hisi_sas_wq_process); queue_work(hisi_hba->wq, &wq->work_struct); } void hisi_sas_wq_process(struct work_struct *work) { struct hisi_sas_wq *wq = container_of(work, struct hisi_sas_wq, work_struct); switch (wq->event) { case CONTROL_PHY: hisi_sas_control_phy_work(hisi_hba, wq->data, phy_no); } static void hisi_sas_control_phy_work(struct hisi_hba *hisi_hba, int func, int phy_no) { switch (func) { case PHY_FUNC_HARD_RESET: hard_phy_reset_v1_hw(hisi_hba, phy_no) > Sorry for being unclear, I implied getting rid of them, by having a > work queue per phy. You can create a structure for each phy like > > struct hisi_sas_phy { > struct hisi_sas *dev; /* pointer to the device */ > unsigned int phy_no; > struct work_struct phyup_work; > }; > > There are probably other things you can put into the same structure. > When the phy is brought up, you then just start the phyup work for > that phy, which can retrieve its hisi_sas_phy structure from the > work_struct pointer, and from there get to the device. > > Arnd > > . > We could create a work_struct for each event, which would be fine. However we would sometimes still need some way of passing data to the event, like the phy control example. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2015-10-19 14:56 UTC|newest] Thread overview: 91+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-10-12 15:20 [PATCH 00/25] HiSilicon SAS driver John Garry 2015-10-12 15:20 ` [PATCH 02/25] devicetree: bindings: scsi: HiSi SAS John Garry 2015-10-16 13:47 ` Rob Herring [not found] ` <CAL_JsqLk9p_YX2FCNiR4sOSU74asN0UrOSJ5gQfnyRhrFH8LgQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-10-19 10:48 ` John Garry 2015-10-19 14:51 ` zhangfei 2015-10-12 15:20 ` [PATCH 03/25] scsi: hisi_sas: add initial bare driver John Garry [not found] ` <1444663237-238302-4-git-send-email-john.garry-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> 2015-10-15 8:49 ` Xinwei Kong 2015-10-15 9:23 ` John Garry 2015-10-15 9:28 ` zhangfei [not found] ` <561F707C.1070305-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> 2015-10-15 12:07 ` Xinwei Kong 2015-10-12 15:20 ` [PATCH 04/25] scsi: hisi_sas: add scsi host registration John Garry 2015-10-12 15:21 ` Arnd Bergmann 2015-10-12 15:21 ` Arnd Bergmann 2015-10-13 9:16 ` John Garry 2015-10-13 9:16 ` John Garry 2015-10-13 12:18 ` Arnd Bergmann 2015-10-12 15:20 ` [PATCH 07/25] scsi: hisi_sas: add ioremap for device HW John Garry 2015-10-12 15:21 ` Arnd Bergmann 2015-10-13 9:47 ` zhangfei 2015-10-13 9:47 ` zhangfei 2015-10-13 12:20 ` Arnd Bergmann 2015-10-13 12:20 ` Arnd Bergmann 2015-10-13 15:09 ` zhangfei 2015-10-12 17:18 ` [RFC PATCH] scsi: hisi_sas: hisi_sas_ioremap() can be static kbuild test robot 2015-10-12 17:18 ` [PATCH 07/25] scsi: hisi_sas: add ioremap for device HW kbuild test robot 2015-10-12 15:20 ` [PATCH 08/25] scsi: hisi_sas: add cq structure initialization John Garry 2015-10-12 15:20 ` [PATCH 10/25] scsi: hisi_sas: add misc HBA initialization John Garry 2015-10-12 21:44 ` kbuild test robot 2015-10-12 15:20 ` [PATCH 12/25] scsi: hisi_sas: add v1 HW initialisation code John Garry 2015-10-12 18:46 ` Arnd Bergmann 2015-10-13 12:44 ` John Garry 2015-10-13 12:44 ` John Garry 2015-10-13 12:47 ` Arnd Bergmann 2015-10-12 15:20 ` [PATCH 13/25] scsi: hisi_sas: add path from phyup irq to SAS framework John Garry 2015-10-12 22:03 ` kbuild test robot 2015-10-13 0:27 ` Julian Calaby 2015-10-13 0:40 ` [lkp] " Fengguang Wu 2015-10-12 22:03 ` [RFC PATCH] scsi: hisi_sas: hisi_sas_bytes_dmaed() can be static kbuild test robot 2015-10-16 12:55 ` [PATCH 13/25] scsi: hisi_sas: add path from phyup irq to SAS framework Arnd Bergmann 2015-10-16 13:29 ` John Garry 2015-10-16 13:29 ` John Garry 2015-10-16 13:36 ` Arnd Bergmann 2015-10-19 14:11 ` John Garry 2015-10-19 14:11 ` John Garry 2015-10-19 14:26 ` Arnd Bergmann 2015-10-19 14:55 ` John Garry [this message] 2015-10-19 14:55 ` John Garry 2015-10-20 8:40 ` Arnd Bergmann 2015-10-20 9:09 ` John Garry 2015-10-20 9:09 ` John Garry [not found] ` <1444663237-238302-1-git-send-email-john.garry-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> 2015-10-12 15:20 ` [PATCH 01/25] [SCSI] sas: centralise ssp frame information units John Garry 2015-10-12 15:20 ` [PATCH 05/25] scsi: hisi_sas: allocate memories and create pools John Garry 2015-10-12 15:15 ` Arnd Bergmann 2015-10-12 15:15 ` Arnd Bergmann 2015-10-13 9:42 ` zhangfei [not found] ` <1444663237-238302-6-git-send-email-john.garry-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> 2015-10-13 1:05 ` kbuild test robot 2015-10-12 15:20 ` [PATCH 06/25] scsi: hisi_sas: add slot init code John Garry 2015-10-12 15:20 ` [PATCH 09/25] scsi: hisi_sas: add phy SAS ADDR initialization John Garry 2015-10-13 6:12 ` Hannes Reinecke [not found] ` <561CA0CB.5090802-l3A5Bk7waGM@public.gmane.org> 2015-10-13 17:14 ` John Garry 2015-10-14 8:40 ` Hannes Reinecke [not found] ` <561E1501.2070507-l3A5Bk7waGM@public.gmane.org> 2015-10-14 15:05 ` John Garry 2015-10-14 15:18 ` Arnd Bergmann 2015-10-15 3:36 ` zhangfei 2015-10-15 8:43 ` Arnd Bergmann 2015-10-12 15:20 ` [PATCH 11/25] scsi: hisi_sas: add v1 hardware register definitions John Garry 2015-10-12 15:20 ` [PATCH 14/25] scsi: hisi_sas: add ssp command function John Garry 2015-10-13 1:24 ` kbuild test robot 2015-10-12 15:20 ` [PATCH 20/25] scsi: hisi_sas: add smp protocol support John Garry 2015-10-12 15:20 ` [PATCH 22/25] scsi: hisi_sas: add tmf methods John Garry 2015-10-12 23:02 ` kbuild test robot [not found] ` <1444663237-238302-23-git-send-email-john.garry-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> 2015-10-12 23:02 ` [RFC PATCH] scsi: hisi_sas: hisi_sas_find_dev_phyno() can be static kbuild test robot 2015-10-12 15:20 ` [PATCH 23/25] scsi: hisi_sas: add control phy handler John Garry 2015-10-12 15:20 ` [PATCH 15/25] scsi: hisi_sas: add cq interrupt handler John Garry 2015-10-12 22:15 ` kbuild test robot 2015-10-12 15:20 ` [PATCH 16/25] scsi: hisi_sas: add dev_found and port_formed John Garry 2015-10-12 22:35 ` [RFC PATCH] scsi: hisi_sas: hisi_sas_alloc_dev() can be static kbuild test robot 2015-10-12 22:35 ` [PATCH 16/25] scsi: hisi_sas: add dev_found and port_formed kbuild test robot 2015-10-12 15:20 ` [PATCH 17/25] scsi: hisi_sas: add abnormal irq handler John Garry 2015-10-12 15:20 ` [PATCH 18/25] scsi: hisi_sas: add dev_gone and port_deformed John Garry [not found] ` <1444663237-238302-19-git-send-email-john.garry-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> 2015-10-12 22:49 ` kbuild test robot 2015-10-12 22:49 ` [RFC PATCH] scsi: hisi_sas: hisi_sas_do_release_task() can be static kbuild test robot 2015-10-12 15:20 ` [PATCH 19/25] scsi: hisi_sas: add bcast interrupt handler John Garry 2015-10-12 15:20 ` [PATCH 21/25] scsi: hisi_sas: add scan finished and start John Garry 2015-10-12 15:20 ` [PATCH 24/25] scsi: hisi_sas: add fatal irq handler John Garry 2015-10-12 15:20 ` [PATCH 25/25] MAINTAINERS: add maintainer for HiSi SAS driver John Garry 2015-10-19 8:47 ` [PATCH 00/25] HiSilicon " John Garry 2015-10-19 8:47 ` John Garry 2015-10-19 8:55 ` Hannes Reinecke 2015-10-19 10:40 ` John Garry 2015-10-19 10:40 ` John Garry
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=56250473.9090700@huawei.com \ --to=john.garry@huawei.com \ --cc=James.Bottomley@hansenpartnership.com \ --cc=arnd@arndb.de \ --cc=devicetree@vger.kernel.org \ --cc=hare@suse.de \ --cc=john.garry2@mail.dcu.ie \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=linuxarm@huawei.com \ --cc=xuwei5@hisilicon.com \ --cc=zhangfei.gao@linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.