Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1230920 > unrolled thread

Re: [RFC v2 09/18] mm/huge_page: Convert khugepaged() into kthread worker API

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC v2 09/18] mm/huge_page: Convert khugepaged() into kthread  worker API Tejun Heo <tj@kernel.org> - 2015-09-22 22:30 +0200
    Re: [RFC v2 09/18] mm/huge_page: Convert khugepaged() into kthread  worker API Petr Mladek <pmladek@suse.com> - 2015-09-23 12:00 +0200

#1230920 — Re: [RFC v2 09/18] mm/huge_page: Convert khugepaged() into kthread worker API

Hello,

On Mon, Sep 21, 2015 at 03:03:50PM +0200, Petr Mladek wrote:
> +static int khugepaged_has_work(void)
> +{
> +	return !list_empty(&khugepaged_scan.mm_head) &&
> +		khugepaged_enabled();
> +}

Hmmm... no biggie but this is a bit bothering.

> @@ -425,7 +447,10 @@ static ssize_t scan_sleep_millisecs_store(struct kobject *kobj,
>  		return -EINVAL;
>  
>  	khugepaged_scan_sleep_millisecs = msecs;
> -	wake_up_interruptible(&khugepaged_wait);
> +	if (khugepaged_has_work())
> +		mod_delayed_kthread_work(khugepaged_worker,
> +					 &khugepaged_do_scan_work,
> +					 0);

What's wrong with just doing the following?

	if (khugepaged_enabled())
		mod_delayed_kthread_work(...);

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1231296

On Tue 2015-09-22 16:26:04, Tejun Heo wrote:
> Hello,
> 
> On Mon, Sep 21, 2015 at 03:03:50PM +0200, Petr Mladek wrote:
> > +static int khugepaged_has_work(void)
> > +{
> > +	return !list_empty(&khugepaged_scan.mm_head) &&
> > +		khugepaged_enabled();
> > +}
> 
> Hmmm... no biggie but this is a bit bothering.

This function has been there even before and is used on more locations.
I have just moved the definition.

> > @@ -425,7 +447,10 @@ static ssize_t scan_sleep_millisecs_store(struct kobject *kobj,
> >  		return -EINVAL;
> >  
> >  	khugepaged_scan_sleep_millisecs = msecs;
> > -	wake_up_interruptible(&khugepaged_wait);
> > +	if (khugepaged_has_work())
> > +		mod_delayed_kthread_work(khugepaged_worker,
> > +					 &khugepaged_do_scan_work,
> > +					 0);
> 
> What's wrong with just doing the following?
> 
> 	if (khugepaged_enabled())
> 		mod_delayed_kthread_work(...);

It was just an optimization. It does not make sense to queue the work
if there is nothing to do.

Note that the timeout between the scans is there to throttle the work.
If all pages are scanned, the work stops re-queuing until
__khugepaged_enter() adds new job.

Thanks a lot for review. I am going to update the patchset according
to the other comments.

Best Regards,
Petr
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web