2010-02-24 20:22:41

by Oleg Nesterov

Subject: [PATCH 2/3] workqueues: microoptimize set_wq_data()

The comment correctly states that the _PENDING bit must be set and
we even have the BUG_ON() check. But this means there is no need to
set WORK_STRUCT_PENDING explicitely and load work_data_bits() twice,
we can rely on WORK_STRUCT_FLAG_MASK which contains _PENDING.

Shaves 32 bytes from workqueue.o.

Signed-off-by: Oleg Nesterov <[email protected]>
---

kernel/workqueue.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

--- wq/kernel/workqueue.c~2_CLEANUP_SET_DATA 2010-02-24 20:55:53.000000000 +0100
+++ wq/kernel/workqueue.c 2010-02-24 20:58:37.000000000 +0100
@@ -220,12 +220,9 @@ struct cpu_workqueue_struct *wq_per_cpu(
static inline void set_wq_data(struct work_struct *work,
struct cpu_workqueue_struct *cwq)
{
- unsigned long new;
-
- BUG_ON(!work_pending(work));
-
- new = (unsigned long) cwq | (1UL << WORK_STRUCT_PENDING);
+ unsigned long new = (unsigned long)cwq;
new |= WORK_STRUCT_FLAG_MASK & *work_data_bits(work);
+ BUG_ON(!(new & (1UL << WORK_STRUCT_PENDING)));
atomic_long_set(&work->data, new);
}

2010-02-25 03:01:24

by Tejun Heo

Subject: Re: [PATCH 2/3] workqueues: microoptimize set_wq_data()

On 02/25/2010 05:20 AM, Oleg Nesterov wrote:
> The comment correctly states that the _PENDING bit must be set and
> we even have the BUG_ON() check. But this means there is no need to
> set WORK_STRUCT_PENDING explicitely and load work_data_bits() twice,
> we can rely on WORK_STRUCT_FLAG_MASK which contains _PENDING.
>
> Shaves 32 bytes from workqueue.o.
>
> Signed-off-by: Oleg Nesterov <[email protected]>
> ---
>
> kernel/workqueue.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> --- wq/kernel/workqueue.c~2_CLEANUP_SET_DATA 2010-02-24 20:55:53.000000000 +0100
> +++ wq/kernel/workqueue.c 2010-02-24 20:58:37.000000000 +0100
> @@ -220,12 +220,9 @@ struct cpu_workqueue_struct *wq_per_cpu(
> static inline void set_wq_data(struct work_struct *work,
> struct cpu_workqueue_struct *cwq)
> {
> - unsigned long new;
> -
> - BUG_ON(!work_pending(work));
> -
> - new = (unsigned long) cwq | (1UL << WORK_STRUCT_PENDING);
> + unsigned long new = (unsigned long)cwq;
> new |= WORK_STRUCT_FLAG_MASK & *work_data_bits(work);
> + BUG_ON(!(new & (1UL << WORK_STRUCT_PENDING)));
> atomic_long_set(&work->data, new);

Will apply under cmwq patches for the next merge window.

Thanks.

--
tejun

2010-02-25 10:25:03

by Tejun Heo

Subject: Re: [PATCH 2/3] workqueues: microoptimize set_wq_data()

Hello, again.

On 02/25/2010 12:10 PM, Tejun Heo wrote:
>> @@ -220,12 +220,9 @@ struct cpu_workqueue_struct *wq_per_cpu(
>> static inline void set_wq_data(struct work_struct *work,
>> struct cpu_workqueue_struct *cwq)
>> {
>> - unsigned long new;
>> -
>> - BUG_ON(!work_pending(work));
>> -
>> - new = (unsigned long) cwq | (1UL << WORK_STRUCT_PENDING);
>> + unsigned long new = (unsigned long)cwq;
>> new |= WORK_STRUCT_FLAG_MASK & *work_data_bits(work);
>> + BUG_ON(!(new & (1UL << WORK_STRUCT_PENDING)));
>> atomic_long_set(&work->data, new);
>
> Will apply under cmwq patches for the next merge window.

Turns out I already have a patch which kills the second
work_data_bits() dereferencing in the series. The first one is now in
the cmwq series which is about to be posted again.

Thanks.

--
tejun

2010-02-25 13:56:53

by Oleg Nesterov

Subject: Re: [PATCH 2/3] workqueues: microoptimize set_wq_data()

Hi,

On 02/25, Tejun Heo wrote:
>
> Hello, again.
>
> On 02/25/2010 12:10 PM, Tejun Heo wrote:
> >> @@ -220,12 +220,9 @@ struct cpu_workqueue_struct *wq_per_cpu(
> >> static inline void set_wq_data(struct work_struct *work,
> >> struct cpu_workqueue_struct *cwq)
> >> {
> >> - unsigned long new;
> >> -
> >> - BUG_ON(!work_pending(work));
> >> -
> >> - new = (unsigned long) cwq | (1UL << WORK_STRUCT_PENDING);
> >> + unsigned long new = (unsigned long)cwq;
> >> new |= WORK_STRUCT_FLAG_MASK & *work_data_bits(work);
> >> + BUG_ON(!(new & (1UL << WORK_STRUCT_PENDING)));
> >> atomic_long_set(&work->data, new);
> >
> > Will apply under cmwq patches for the next merge window.
>
> Turns out I already have a patch which kills the second
> work_data_bits() dereferencing in the series. The first one is now in
> the cmwq series which is about to be posted again.

OK, good.

Tejun, where can I find your cmwq patches? I'd like to take a look.

Thanks,

Oleg.

2010-02-25 15:13:24

by Tejun Heo

Subject: Re: [PATCH 2/3] workqueues: microoptimize set_wq_data()

Hello,

On 02/25/2010 10:55 PM, Oleg Nesterov wrote:
>> Turns out I already have a patch which kills the second
>> work_data_bits() dereferencing in the series. The first one is now in
>> the cmwq series which is about to be posted again.
>
> OK, good.
>
> Tejun, where can I find your cmwq patches? I'd like to take a look.

The third take was about a month ago.

http://thread.gmane.org/gmane.linux.kernel/939353

The fourth take is just around the corner.

Thanks.

--
tejun