* [PATCH] net/mlx5_core/pagealloc: Remove deprecated create_singlethread_workqueue @ 2016-07-28 8:19 Bhaktipriya Shridhar 2016-07-28 9:37 ` Leon Romanovsky 2016-07-28 22:44 ` Saeed Mahameed 0 siblings, 2 replies; 8+ messages in thread From: Bhaktipriya Shridhar @ 2016-07-28 8:19 UTC (permalink / raw) To: Matan Barak, Leon Romanovsky; +Cc: netdev, linux-rdma, linux-kernel, Tejun Heo A dedicated workqueue has been used since the work items are being used on a memory reclaim path. WQ_MEM_RECLAIM has been set to guarantee forward progress under memory pressure. The workqueue has a single work item. Hence, alloc_workqueue() is used instead of alloc_ordered_workqueue() since ordering is unnecessary when there's only one work item. Explicit concurrency limit is unnecessary here since there are only a fixed number of work items. Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com> --- drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c index 9eeee05..7c85262 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c @@ -552,7 +552,8 @@ void mlx5_pagealloc_cleanup(struct mlx5_core_dev *dev) int mlx5_pagealloc_start(struct mlx5_core_dev *dev) { - dev->priv.pg_wq = create_singlethread_workqueue("mlx5_page_allocator"); + dev->priv.pg_wq = alloc_workqueue("mlx5_page_allocator", + WQ_MEM_RECLAIM, 0); if (!dev->priv.pg_wq) return -ENOMEM; -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] net/mlx5_core/pagealloc: Remove deprecated create_singlethread_workqueue 2016-07-28 8:19 [PATCH] net/mlx5_core/pagealloc: Remove deprecated create_singlethread_workqueue Bhaktipriya Shridhar @ 2016-07-28 9:37 ` Leon Romanovsky 2016-07-28 22:45 ` Saeed Mahameed 2016-07-29 12:22 ` Tejun Heo 2016-07-28 22:44 ` Saeed Mahameed 1 sibling, 2 replies; 8+ messages in thread From: Leon Romanovsky @ 2016-07-28 9:37 UTC (permalink / raw) To: Bhaktipriya Shridhar Cc: Matan Barak, netdev, linux-rdma, linux-kernel, Tejun Heo, Saeed Mahameed [-- Attachment #1: Type: text/plain, Size: 1905 bytes --] On Thu, Jul 28, 2016 at 01:49:49PM +0530, Bhaktipriya Shridhar wrote: > A dedicated workqueue has been used since the work items are being used > on a memory reclaim path. WQ_MEM_RECLAIM has been set to guarantee forward > progress under memory pressure. > > The workqueue has a single work item. Hence, alloc_workqueue() is used > instead of alloc_ordered_workqueue() since ordering is unnecessary when > there's only one work item. > > Explicit concurrency limit is unnecessary here since there are only a > fixed number of work items. > > Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com> > --- > drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Hi Bhaktipriya, First of all, I would like to thank you for your work and invite you to continue, but can you please submit ONE patch SERIES which changes all similar places? BTW, Did you test this patch? Did you notice the memory reclaim path nature of this work? Thanks > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c > index 9eeee05..7c85262 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c > @@ -552,7 +552,8 @@ void mlx5_pagealloc_cleanup(struct mlx5_core_dev *dev) > > int mlx5_pagealloc_start(struct mlx5_core_dev *dev) > { > - dev->priv.pg_wq = create_singlethread_workqueue("mlx5_page_allocator"); > + dev->priv.pg_wq = alloc_workqueue("mlx5_page_allocator", > + WQ_MEM_RECLAIM, 0); > if (!dev->priv.pg_wq) > return -ENOMEM; > > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net/mlx5_core/pagealloc: Remove deprecated create_singlethread_workqueue 2016-07-28 9:37 ` Leon Romanovsky @ 2016-07-28 22:45 ` Saeed Mahameed 2016-07-29 12:22 ` Tejun Heo 1 sibling, 0 replies; 8+ messages in thread From: Saeed Mahameed @ 2016-07-28 22:45 UTC (permalink / raw) To: Leon Romanovsky Cc: Bhaktipriya Shridhar, Matan Barak, Linux Netdev List, linux-rdma, linux-kernel, Tejun Heo, Saeed Mahameed On Thu, Jul 28, 2016 at 12:37 PM, Leon Romanovsky <leonro@mellanox.com> wrote: > On Thu, Jul 28, 2016 at 01:49:49PM +0530, Bhaktipriya Shridhar wrote: >> A dedicated workqueue has been used since the work items are being used >> on a memory reclaim path. WQ_MEM_RECLAIM has been set to guarantee forward >> progress under memory pressure. >> >> The workqueue has a single work item. Hence, alloc_workqueue() is used >> instead of alloc_ordered_workqueue() since ordering is unnecessary when >> there's only one work item. >> >> Explicit concurrency limit is unnecessary here since there are only a >> fixed number of work items. >> >> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com> >> --- >> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) > > Hi Bhaktipriya, > > First of all, I would like to thank you for your work and invite you to > continue, but can you please submit ONE patch SERIES which changes all > similar places? > I agree with Leon, please push one series for all mlx5 patches and add some explanation in the cover letter regarding the motivation of this work. > BTW, > Did you test this patch? Did you notice the memory reclaim path nature > of this work? > > Thanks > >> >> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c >> index 9eeee05..7c85262 100644 >> --- a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c >> +++ b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c >> @@ -552,7 +552,8 @@ void mlx5_pagealloc_cleanup(struct mlx5_core_dev *dev) >> >> int mlx5_pagealloc_start(struct mlx5_core_dev *dev) >> { >> - dev->priv.pg_wq = create_singlethread_workqueue("mlx5_page_allocator"); >> + dev->priv.pg_wq = alloc_workqueue("mlx5_page_allocator", >> + WQ_MEM_RECLAIM, 0); >> if (!dev->priv.pg_wq) >> return -ENOMEM; >> >> -- >> 2.1.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net/mlx5_core/pagealloc: Remove deprecated create_singlethread_workqueue 2016-07-28 9:37 ` Leon Romanovsky 2016-07-28 22:45 ` Saeed Mahameed @ 2016-07-29 12:22 ` Tejun Heo 2016-07-31 6:35 ` Leon Romanovsky 1 sibling, 1 reply; 8+ messages in thread From: Tejun Heo @ 2016-07-29 12:22 UTC (permalink / raw) To: Leon Romanovsky Cc: Bhaktipriya Shridhar, Matan Barak, netdev, linux-rdma, linux-kernel, Saeed Mahameed Hello, On Thu, Jul 28, 2016 at 12:37:35PM +0300, Leon Romanovsky wrote: > Did you test this patch? Did you notice the memory reclaim path nature > of this work? The conversion uses WQ_MEM_RECLAIM, which is standard for all workqueues which can stall packet processing if stalled. The requirement comes from nfs or block devices over network. Thanks. -- tejun ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net/mlx5_core/pagealloc: Remove deprecated create_singlethread_workqueue 2016-07-29 12:22 ` Tejun Heo @ 2016-07-31 6:35 ` Leon Romanovsky 2016-08-01 15:11 ` Tejun Heo 0 siblings, 1 reply; 8+ messages in thread From: Leon Romanovsky @ 2016-07-31 6:35 UTC (permalink / raw) To: Tejun Heo Cc: Bhaktipriya Shridhar, Matan Barak, netdev, linux-rdma, linux-kernel, Saeed Mahameed [-- Attachment #1: Type: text/plain, Size: 1294 bytes --] On Fri, Jul 29, 2016 at 08:22:37AM -0400, Tejun Heo wrote: > Hello, > > On Thu, Jul 28, 2016 at 12:37:35PM +0300, Leon Romanovsky wrote: > > Did you test this patch? Did you notice the memory reclaim path nature > > of this work? > > The conversion uses WQ_MEM_RECLAIM, which is standard for all > workqueues which can stall packet processing if stalled. The > requirement comes from nfs or block devices over network. The title stays "remove deprecated create_singlethread_workqueue" and if we put aside the word "deprecated", the code moves from ordered workqueue to unordered workqueue and changes max_active count which isn't expressed in commit message. For reclaim paths, I want to be extra caution and want to see justification for doing that along with understanding if it is tested or not. Right now, I'm feeling that I'm participating in soapie where one sends patch for every line, waits and sends the same patch for another file. It is worth to send one patch set and let us to test it all in once. Thanks. > > Thanks. > > -- > tejun > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net/mlx5_core/pagealloc: Remove deprecated create_singlethread_workqueue 2016-07-31 6:35 ` Leon Romanovsky @ 2016-08-01 15:11 ` Tejun Heo 2016-08-02 5:56 ` Leon Romanovsky 0 siblings, 1 reply; 8+ messages in thread From: Tejun Heo @ 2016-08-01 15:11 UTC (permalink / raw) To: Leon Romanovsky Cc: Bhaktipriya Shridhar, Matan Barak, netdev, linux-rdma, linux-kernel, Saeed Mahameed Hello, Leon. On Sun, Jul 31, 2016 at 09:35:13AM +0300, Leon Romanovsky wrote: > > The conversion uses WQ_MEM_RECLAIM, which is standard for all > > workqueues which can stall packet processing if stalled. The > > requirement comes from nfs or block devices over network. > > The title stays "remove deprecated create_singlethread_workqueue" and if > we put aside the word "deprecated", the code moves from ordered > workqueue to unordered workqueue and changes max_active count which > isn't expressed in commit message. Maybe it was too compressed but the description says "The workqueue has a single work item. Hence, alloc_workqueue() is used instead of alloc_ordered_workqueue() since ordering is unnecessary when there's only one work item." There's only one work item so whether the workqueue is ordered and its concurrency level don't make any difference. The reason why people used to use create_singlethread_workqueue() in these situations was because per-cpu workqueues used to be very expensive which isn't true anymore. > For reclaim paths, I want to be extra caution and want to see > justification for doing that along with understanding if it is tested or > not. WQ_MEM_RECLAIM maintains the behavior of the existing code and the requirement comes from network interfaces being used for nfs and transport for block devices and is universial to everything in network layer. The only test we can do here is removing it and pushing it really hard and see whether we can actually trigger deadlock, which it will under the right circumstances which may or may not be easy to replicate in test environments. I don't see many merits in doing this especially when the behavior doesn't change from the existing code. > Right now, I'm feeling that I'm participating in soapie where one sends > patch for every line, waits and sends the same patch for another file. > It is worth to send one patch set and let us to test it all in once. Yeah, I guess it can be a bit annoying. Bhaktipriya, can you please group patches for meallnox? Thanks. -- tejun ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net/mlx5_core/pagealloc: Remove deprecated create_singlethread_workqueue 2016-08-01 15:11 ` Tejun Heo @ 2016-08-02 5:56 ` Leon Romanovsky 0 siblings, 0 replies; 8+ messages in thread From: Leon Romanovsky @ 2016-08-02 5:56 UTC (permalink / raw) To: Tejun Heo Cc: Bhaktipriya Shridhar, Matan Barak, netdev, linux-rdma, linux-kernel, Saeed Mahameed [-- Attachment #1: Type: text/plain, Size: 660 bytes --] On Mon, Aug 01, 2016 at 11:11:19AM -0400, Tejun Heo wrote: > > Right now, I'm feeling that I'm participating in soapie where one sends > > patch for every line, waits and sends the same patch for another file. > > It is worth to send one patch set and let us to test it all in once. > > Yeah, I guess it can be a bit annoying. Bhaktipriya, can you please > group patches for meallnox? Please do. Thanks. > > Thanks. > > -- > tejun > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net/mlx5_core/pagealloc: Remove deprecated create_singlethread_workqueue 2016-07-28 8:19 [PATCH] net/mlx5_core/pagealloc: Remove deprecated create_singlethread_workqueue Bhaktipriya Shridhar 2016-07-28 9:37 ` Leon Romanovsky @ 2016-07-28 22:44 ` Saeed Mahameed 1 sibling, 0 replies; 8+ messages in thread From: Saeed Mahameed @ 2016-07-28 22:44 UTC (permalink / raw) To: Bhaktipriya Shridhar Cc: Matan Barak, Leon Romanovsky, Linux Netdev List, linux-rdma, linux-kernel, Tejun Heo On Thu, Jul 28, 2016 at 11:19 AM, Bhaktipriya Shridhar <bhaktipriya96@gmail.com> wrote: > A dedicated workqueue has been used since the work items are being used > on a memory reclaim path. WQ_MEM_RECLAIM has been set to guarantee forward > progress under memory pressure. > > The workqueue has a single work item. Hence, alloc_workqueue() is used > instead of alloc_ordered_workqueue() since ordering is unnecessary when > there's only one work item. let's keep the current behavior (ST WQ) because at the moment we don't know how this WQ will evolve in the future, the original author had something in mind .. let's keep. > > Explicit concurrency limit is unnecessary here since there are only a > fixed number of work items. > > Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com> > --- > drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c > index 9eeee05..7c85262 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c > @@ -552,7 +552,8 @@ void mlx5_pagealloc_cleanup(struct mlx5_core_dev *dev) > > int mlx5_pagealloc_start(struct mlx5_core_dev *dev) > { > - dev->priv.pg_wq = create_singlethread_workqueue("mlx5_page_allocator"); > + dev->priv.pg_wq = alloc_workqueue("mlx5_page_allocator", > + WQ_MEM_RECLAIM, 0); > if (!dev->priv.pg_wq) > return -ENOMEM; > > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-08-02 5:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-07-28 8:19 [PATCH] net/mlx5_core/pagealloc: Remove deprecated create_singlethread_workqueue Bhaktipriya Shridhar 2016-07-28 9:37 ` Leon Romanovsky 2016-07-28 22:45 ` Saeed Mahameed 2016-07-29 12:22 ` Tejun Heo 2016-07-31 6:35 ` Leon Romanovsky 2016-08-01 15:11 ` Tejun Heo 2016-08-02 5:56 ` Leon Romanovsky 2016-07-28 22:44 ` Saeed Mahameed
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).