From: Hayes Wang <hayeswang@realtek.com> To: David Miller <davem@davemloft.net> Cc: "sfeldma@gmail.com" <sfeldma@gmail.com>, "netdev@vger.kernel.org" <netdev@vger.kernel.org>, nic_swsd <nic_swsd@realtek.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org> Subject: RE: [PATCH net-next 1/7] r8152: adjust rx_bottom Date: Mon, 26 Jan 2015 07:14:06 +0000 [thread overview] Message-ID: <0835B3720019904CB8F7AA43166CEEB2EE77A2@RTITMBSV03.realtek.com.tw> (raw) In-Reply-To: <20150124.224342.425895080003524375.davem@davemloft.net> David Miller [mailto:davem@davemloft.net] > Sent: Sunday, January 25, 2015 2:44 PM [...] > What keeps rtl_start_rx() from running in parallel with > r8152_submit_rx(), or any other accessor of the RX agg->list? Forgive my poor English. I would try to describe them clearly. The steps about the rx agg->list would be 1. carrier on or autoresume occurs. 2. Call rtl_start_rx(). 3. Rx agg->list flows between device and tp->rx_done. 4. carrier off or autosuspend occurs. 5. call rtl_stop_rx(). The rtl_start_rx() would only be called when the linking status is changed from off to on or the auto resume occurs. And rtl_start_rx() would reinitialize the tp->rx_done and all of the rx agg->list. After step 2, the rx agg->list would flow between the usb host controller and the driver. If r8152_submit_rx() is success, the driver wouldn't own the rx agg->list until it is returned from the usb host controller. If r8152_submit_rx() is fail, the driver would still own the rx agg->list, and queue it to the tp->rx_done with spin lock for next try. If the status stays in step 3, only the rx_bottom() would submit the rx agg. The rtl_start_rx() wouldn't be called suddenly, unless the linking down or auto suspend occur first and linking on or auto resume occur again. If linking down or auto suspend occur, rtl_stop_rx() would be called (step 5). After this step, rx_bottom() wouldn't submit rx, and all rx agg->list would stop flowing. That is, the tp->rx_done and all rx agg->list wouldn't be changed until the next rtl_start_rx() is called. Therefore, the flow for each rx agg->list would be a. submittd by rtl_start_rx(). b. goto step c if success, otherwise goto step d. c. completed by usb host controller. d. queued to tp->rx_done with spin lock. e. dequeue from tp->rx_done with spin lock by rx_botoom(). f. goto step i if link down, otherwise goto step g. g. submitted by rx_botoom(). h. goto step b. i. goto step a if link on. And the patch change the step g to g1. g1. submitted by rx_botoom() if (!ret), otherwise goto step d. Best Regards, Hayes > > You also keep using different terminology from me when > discussing what lists do or do not need protection, and that > is going to make it difficult for anyone to follow our > conversation at all. > > We're talking specifically about RX agg->list objects and > whether access to them need synchronization or not.
WARNING: multiple messages have this Message-ID (diff)
From: Hayes Wang <hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org> To: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> Cc: "sfeldma-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <sfeldma-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, nic_swsd <nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> Subject: RE: [PATCH net-next 1/7] r8152: adjust rx_bottom Date: Mon, 26 Jan 2015 07:14:06 +0000 [thread overview] Message-ID: <0835B3720019904CB8F7AA43166CEEB2EE77A2@RTITMBSV03.realtek.com.tw> (raw) In-Reply-To: <20150124.224342.425895080003524375.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> David Miller [mailto:davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org] > Sent: Sunday, January 25, 2015 2:44 PM [...] > What keeps rtl_start_rx() from running in parallel with > r8152_submit_rx(), or any other accessor of the RX agg->list? Forgive my poor English. I would try to describe them clearly. The steps about the rx agg->list would be 1. carrier on or autoresume occurs. 2. Call rtl_start_rx(). 3. Rx agg->list flows between device and tp->rx_done. 4. carrier off or autosuspend occurs. 5. call rtl_stop_rx(). The rtl_start_rx() would only be called when the linking status is changed from off to on or the auto resume occurs. And rtl_start_rx() would reinitialize the tp->rx_done and all of the rx agg->list. After step 2, the rx agg->list would flow between the usb host controller and the driver. If r8152_submit_rx() is success, the driver wouldn't own the rx agg->list until it is returned from the usb host controller. If r8152_submit_rx() is fail, the driver would still own the rx agg->list, and queue it to the tp->rx_done with spin lock for next try. If the status stays in step 3, only the rx_bottom() would submit the rx agg. The rtl_start_rx() wouldn't be called suddenly, unless the linking down or auto suspend occur first and linking on or auto resume occur again. If linking down or auto suspend occur, rtl_stop_rx() would be called (step 5). After this step, rx_bottom() wouldn't submit rx, and all rx agg->list would stop flowing. That is, the tp->rx_done and all rx agg->list wouldn't be changed until the next rtl_start_rx() is called. Therefore, the flow for each rx agg->list would be a. submittd by rtl_start_rx(). b. goto step c if success, otherwise goto step d. c. completed by usb host controller. d. queued to tp->rx_done with spin lock. e. dequeue from tp->rx_done with spin lock by rx_botoom(). f. goto step i if link down, otherwise goto step g. g. submitted by rx_botoom(). h. goto step b. i. goto step a if link on. And the patch change the step g to g1. g1. submitted by rx_botoom() if (!ret), otherwise goto step d. Best Regards, Hayes > > You also keep using different terminology from me when > discussing what lists do or do not need protection, and that > is going to make it difficult for anyone to follow our > conversation at all. > > We're talking specifically about RX agg->list objects and > whether access to them need synchronization or not. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" 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-01-26 7:18 UTC|newest] Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-01-19 7:13 [PATCH net-next 0/7] r8152: adjust the code Hayes Wang 2015-01-19 7:13 ` [PATCH net-next 1/7] r8152: adjust rx_bottom Hayes Wang 2015-01-19 8:03 ` Scott Feldman 2015-01-19 21:13 ` David Miller 2015-01-19 21:13 ` David Miller 2015-01-20 2:48 ` Hayes Wang 2015-01-20 2:52 ` David Miller 2015-01-20 3:24 ` Hayes Wang 2015-01-25 6:43 ` David Miller 2015-01-26 7:14 ` Hayes Wang [this message] 2015-01-26 7:14 ` Hayes Wang 2015-02-02 2:38 ` Hayes Wang 2015-01-26 9:14 ` Scott Feldman 2015-01-26 9:14 ` Scott Feldman 2015-01-19 7:13 ` [PATCH net-next 2/7] r8152: adjust lpm timer Hayes Wang 2015-01-19 7:13 ` [PATCH net-next 3/7] r8152: check linking status with netif_carrier_ok Hayes Wang 2015-01-19 7:13 ` [PATCH net-next 4/7] r8152: check RTL8152_UNPLUG for rtl8152_close Hayes Wang 2015-01-19 7:13 ` [PATCH net-next 5/7] r8152: adjust the link feed for hw_features Hayes Wang 2015-01-19 7:13 ` [PATCH net-next 6/7] r8152: replace get_protocol with vlan_get_protocol Hayes Wang 2015-01-19 7:13 ` [PATCH net-next 7/7] r8152: use BIT macro Hayes Wang 2015-02-06 3:30 ` [PATCH net-next v2 0/7] r8152: adjust the code Hayes Wang 2015-02-06 3:30 ` [PATCH net-next v2 1/7] r8152: adjust rx_bottom Hayes Wang 2015-02-06 3:30 ` [PATCH net-next v2 2/7] r8152: adjust lpm timer Hayes Wang 2015-02-06 3:30 ` [PATCH net-next v2 3/7] r8152: check linking status with netif_carrier_ok Hayes Wang 2015-02-06 3:30 ` [PATCH net-next v2 4/7] r8152: check RTL8152_UNPLUG for rtl8152_close Hayes Wang 2015-02-06 3:30 ` [PATCH net-next v2 5/7] r8152: adjust the line feed for hw_features Hayes Wang 2015-02-06 3:30 ` [PATCH net-next v2 6/7] r8152: replace get_protocol with vlan_get_protocol Hayes Wang 2015-02-06 3:30 ` [PATCH net-next v2 7/7] r8152: use BIT macro Hayes Wang 2015-02-08 6:46 ` [PATCH net-next v2 0/7] r8152: adjust the code David Miller
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=0835B3720019904CB8F7AA43166CEEB2EE77A2@RTITMBSV03.realtek.com.tw \ --to=hayeswang@realtek.com \ --cc=davem@davemloft.net \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=netdev@vger.kernel.org \ --cc=nic_swsd@realtek.com \ --cc=sfeldma@gmail.com \ /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.