All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shaohua Li <shli@kernel.org>
To: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [patch]workqueue: copy attr with all fields.
Date: Wed, 31 Jul 2013 09:39:29 +0800	[thread overview]
Message-ID: <20130731013929.GB1504@kernel.org> (raw)
In-Reply-To: <20130730141419.GG12016@htj.dyndns.org>

On Tue, Jul 30, 2013 at 10:14:19AM -0400, Tejun Heo wrote:
> Hello, Shaohua.
> 
> On Tue, Jul 30, 2013 at 01:49:55PM +0800, Shaohua Li wrote:
> > > Note that copy_workqueue_attrs() is also used while creating worker
> > > pools and this change would mean that there will be worker pools with
> > > no_numa attribute assigned basically randomly depending on which wq
> > > created the pool for the first time which should be fine AFAICS but
> > > isn't very nice.  Given that there's only single place where a pool's
> > > attrs is set, maybe it'd be best to clear no_numa right after copy and
> > > explain what's going on there?
> > 
> > alloc_workqueue_attrs() always zero the new allocated attr.
> 
> Hmmm?  Yeah, sure, that's how it currently is always zero for all
> pools.  If you change the copy function, it'll now be set to whatever
> the input value was when a pool was created, which shouldn't break
> anything but is a bit confusing as no_numa doesn't have anything to do
> with pools directly.  Just in case, I'm talking about
> copy_workqueue_attrs() call in get_unbound_pool().

Hmm, I didn't agree it's more confusing to change copy_workqueue_attrs(), the
name of the function suggests it is a 'copy'. And clearing no_numa in
apply_workqueue_attrs() after copy_workqueue_attrs() looks like a hack to me.
But it depends on you, feel free to fix it by yourself.

Thanks,
Shaohua

  reply	other threads:[~2013-07-31  1:39 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-29  6:24 [patch]workqueue: copy attr with all fields Shaohua Li
2013-07-29 11:14 ` Tejun Heo
2013-07-29 12:52   ` Shaohua Li
2013-07-29 15:35     ` Tejun Heo
2013-07-30  5:49       ` Shaohua Li
2013-07-30 14:14         ` Tejun Heo
2013-07-31  1:39           ` Shaohua Li [this message]
2013-07-31 10:27             ` Tejun Heo
2013-08-01  1:56               ` Shaohua Li
2013-08-01 12:35                 ` [PATCH] workqueue: copy workqueue_attrs " Tejun Heo

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=20130731013929.GB1504@kernel.org \
    --to=shli@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.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: link
Be 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.