* Re: [PATCH UPDATED 1/3] vhost: replace vhost_workqueue with per-vhost kthread
From: Tejun Heo @ 2010-07-26 19:04 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Oleg Nesterov, Sridhar Samudrala, netdev, lkml,
kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev, Jiri Kosina,
Thomas Gleixner, Ingo Molnar, Andi Kleen
In-Reply-To: <20100726162346.GD26412@redhat.com>
On 07/26/2010 06:23 PM, Michael S. Tsirkin wrote:
>> * Can you please keep the outer goto repeat loop? I just don't like
>> outermost for (;;).
>
> Okay ... can we put the code in a {} scope to make it clear
> where does the loop starts and ends?
If we're gonna do that, it would be better to put it inside a loop
construct. The reason why I don't like it is that loops like that
don't really help read/writeability much while indenting the whole
logic unnecessarily and look more like a result of obsession against
goto rather than any practical reason. It's just a cosmetic
preference and I might as well be the weirdo here, so if you feel
strong about it, please feel free to put everything in a loop.
>> * Placing try_to_freeze() could be a bit annoying. It shouldn't be
>> executed when there's a work to flush.
>
> It currently seems to be executed when there is work to flush.
> Is this wrong?
Oh, does it? As I wrote in the other mail, things like that wouldn't
necessarily break correctness but I think it would be better to avoid
surprises in the generic code if not too difficult.
>> * I think A - B <= 0 test would be more familiar. At least
>> time_before/after() are implemented that way.
>
> I am concerned that this overflows a signed integer -
> which I seem to remeber that C99 disallows.
Really? Overflows of pointer isn't expected and that's why we have
weird RELOC_HIDE() macro for such calculations but integers not
expected to overflow is a news to me. Are you sure? That basically
means time_before/after() aren't safe either.
> timer macros are on data path so might be worth the risk there,
> but flush is slow path so better be safe?
I don't think performance matters much here. I just think the sign
test is clearer / more familiar for the logic.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH UPDATED 1/3] vhost: replace vhost_workqueue with per-vhost kthread
From: Tejun Heo @ 2010-07-26 18:51 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Oleg Nesterov, Sridhar Samudrala, netdev, lkml,
kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev, Jiri Kosina,
Thomas Gleixner, Ingo Molnar, Andi Kleen
In-Reply-To: <20100726163108.GE26412@redhat.com>
Hello,
On 07/26/2010 06:31 PM, Michael S. Tsirkin wrote:
>> On 07/26/2010 06:05 PM, Tejun Heo wrote:
>>> * Placing try_to_freeze() could be a bit annoying. It shouldn't be
>>> executed when there's a work to flush.
>
> BTW why is this important?
> We could always get another work and flush right after
> try_to_freeze, and then flush would block for a long time.
>
> BTW the vhost patch you sent does not do this at all.
> I am guessing it is because our thread is not freezable?
Yeap, I think so.
>> * Similar issue exists for kthread_stop(). The kthread shouldn't exit
>> while there's a work to flush (please note that kthread_worker
>> interface allows detaching / attaching worker kthread during
>> operation, so it should remain in consistent state with regard to
>> flushing).
>
> Not sure I agree here. Users must synchronise flush and stop calls.
> Otherwise a work might get queued after stop is called, and
> you won't be able to flush it.
For freeze, it probably is okay but for stop, I think it's better to
keep the semantics straight forward. It may be okay to do otherwise
but having such oddity in generic interface is nasty and may lead to
surprises which can be pretty difficult to track down later on. It's
just a bit more of annoyance while writing the generic code, so...
Thanks.
--
tejun
^ permalink raw reply
* Re: tcpdump <-> FCoE support
From: Loke, Chetan @ 2010-07-26 18:27 UTC (permalink / raw)
To: Joe Eykholt; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devel-s9riP+hp16TNLxjTenLetw
In-Reply-To: <4C4DCCA6.5060206-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> From: Joe Eykholt [mailto:jeykholt-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org]
>
> Good question. There's no effort in progress that I know of.
> I find wireshark very handy and do tend to capture with tcpdump -w
> and then read the file with wireshark. tshark is handy if you
> want a CLI mode.
>
> If tcpdump interpreted FCoE frames, then we would want FC, FC-ELS,
> and FCP. Does it already dissect any SCSI frames (e.g., iSCSI)?
> I'm not sure how much work it would be, but given tshark
> is around, I don't think it's that important.
>
Oki thanks. I'll play with tshark. If tshark handles the FIP/FC
discovery business then that's perfect. I don't have a FCoE target
configured yet. I just sent a raw fcoe-frame to check how tcpdump
responds.
> It could at least be changed to print out FCoE instead of 0x8906,
> and FIP instead of 0x8914.
>
Agreed.
> Joe
Chetan Loke
^ permalink raw reply
* Re: [PATCH 4/5] wireless: use newly introduced hex_to_bin()
From: John W. Linville @ 2010-07-26 18:27 UTC (permalink / raw)
To: Andy Shevchenko
Cc: David S. Miller, netdev, linux-kernel, Corey Thomas,
linux-wireless
In-Reply-To: <9d7821ff7791c30772667681c0eab829e6e93e59.1279890832.git.andy.shevchenko@gmail.com>
On Fri, Jul 23, 2010 at 04:18:09PM +0300, Andy Shevchenko wrote:
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Corey Thomas <coreythomas@charter.net>
> Cc: "John W. Linville" <linville@tuxdriver.com>
> Cc: linux-wireless@vger.kernel.org
Acked-by: John W. Linville <linville@tuxdriver.com>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [PATCH V3] Export SMBIOS provided firmware instance and label to sysfs
From: Greg KH @ 2010-07-26 18:20 UTC (permalink / raw)
To: Matt Domsch
Cc: Narendra K, netdev, linux-hotplug, linux-pci, charles_rose,
jordan_hargrave, vijay_nijhawan
In-Reply-To: <20100723145809.GA7629@auslistsprd01.us.dell.com>
On Fri, Jul 23, 2010 at 09:58:09AM -0500, Matt Domsch wrote:
> On Fri, Jul 23, 2010 at 06:55:57AM -0700, Greg KH wrote:
> > On Fri, Jul 23, 2010 at 08:34:56AM -0500, Narendra K wrote:
> > > --- a/Documentation/ABI/testing/sysfs-bus-pci
> > > +++ b/Documentation/ABI/testing/sysfs-bus-pci
> > > @@ -179,3 +179,30 @@ Contact: linux-pci@vger.kernel.org
> > > Description:
> > > This symbolic link points to the PCI hotplug controller driver
> > > module that manages the hotplug slot.
> > > +
> > > +What: /sys/bus/pci/devices/.../label
> > > +Date: July 2010
> > > +Contact: linux-bugs@dell.com
> >
> > that's not your email address. Please don't hide behind some random
> > address, Linux is about contacting developers directly were ever
> > possible.
>
> That's actually the public email address for the whole Linux
> engineering team (including engineers and managers) at Dell, of which
> Narendra is a part. It's the address we publish for people to include
> on the cc: list of bugzilla issues on the kernel.org, Novell, and Red
> Hat bugzillas. This ensures someone on the team will see bug reports
> or patches and act accordingly, likely Narendra, but could be anyone
> in the future once Narendra is promoted or changes responsibilities
> (or even employers).
Ok, I don't really like it, but if that's what Dell wants to do, I guess
it's acceptable.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] Driver-core: Fix bluetooth network device rename regression
From: Greg KH @ 2010-07-26 18:09 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Greg KH, Johannes Berg, Kay Sievers, Andrew Morton,
Rafael J. Wysocki, Maciej W. Rozycki, netdev
In-Reply-To: <m1eievugon.fsf@fess.ebiederm.org>
On Thu, Jul 22, 2010 at 06:31:52PM -0700, Eric W. Biederman wrote:
> Greg KH <gregkh@suse.de> writes:
>
> > On Thu, Jul 22, 2010 at 08:36:41PM +0200, Johannes Berg wrote:
> >> On Thu, 2010-07-22 at 11:28 -0700, Greg KH wrote:
> >>
> >> > It worked only because no one realized that it was broken with the
> >> > DEPRECATED option enabled. When that is enabled, it is broken, right?
> >>
> >> I'm pretty sure I always had that enabled, and never had issues. Can't
> >> test right now since I don't have that option back yet in the tree I'm
> >> using.
> >>
> >> > Eric's changes to sysfs to add namespace support exposed this breakage.
> >> > That's not a reason to paper over the problem, but it should be driving
> >> > someone to fix it correctly, as has been pointed out a number of times
> >> > already.
> >>
> >> I'm just contesting that that someone should be me. I don't think you
> >> get to blame driver developers for doing something that worked and
> >> solved the problem they needed to solve. sysfs is largely opaque to most
> >> of us already, and it now sure feels like Kay decided to change the
> >> rules underneath the code in saying "this was wrong all along".
> >
> > Well, if it worked before, and it doesn't now, that's due to Eric's
> > changes, nothing Kay and I did here :)
>
> It mostly worked before, and it works with CONFIG_SYSFS_DEPRECATED=y
> (After I my stupid bug is fixed).
>
> With CONFIG_SYSFS_DEPRECATED=y we have in sysfs:
> ........./net:bnep0 -> /sys/class/net/bnep0
> /sys/class/net/bnep0 is a directory.
>
> With CONFIG_SYSFS_DEPRECATED=n we have in sysfs:
> ........./bnep0
> /sys/class/net/bnep0 -> ......./bnep0
>
> Where for a normal network device we have:
> With CONFIG_SYSFS_DEPRECATED=y we have in sysfs:
> ........./net:eth0 -> /sys/class/net/eth0
> /sys/class/net/bnep0 is a directory.
>
> With CONFIG_SYSFS_DEPRECATED=n we have in sysfs:
> ........./net/eth0
> /sys/class/net/eth0 -> ......./net/eth0
>
> The new sysfs layout loses the net namespace separator for bluetooth
> devices.
>
> > But, in looking at it closer, it does seem that the code is doing things
> > that was not expected to work at all previously, and It's amazing that
> > it did. I thought Kay offered to help fix it all up, and provided 2
> > different ways to do it. I know they aren't trivial, but then again,
> > your usage of sysfs is not trivial either...
>
> I don't expect much just the upper levels to work correctly. The fact
> this works on all but a handful of drivers is what I find dist
>
> Does this version of the change look less bleh worthy? The effect is
> the same as my previous patch but the test is more abstract so the
> effect is not strictly limited to /sys/class/net.
What other class type has a namespace at this point in time?
Essentially this is the same exact thing, just in a different format
that obfuscates what you are really doing here.
As you have now sent this to Linus and he took it, well, it seems moot,
right?
kids these days...
greg k-h
^ permalink raw reply
* Re: [PATCH] Driver-core: Always create class directories for classses that support namespaces.
From: Greg KH @ 2010-07-26 18:07 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Linus Torvalds, Johannes Berg, Kay Sievers, Andrew Morton,
Rafael J. Wysocki, Maciej W. Rozycki, netdev,
<linux-kernel@vger.kernel.org> Marcel Holtmann,
linux-bluetooth, Greg KH
In-Reply-To: <m1tynonmk8.fsf_-_@fess.ebiederm.org>
On Sat, Jul 24, 2010 at 10:43:35PM -0700, Eric W. Biederman wrote:
> p.s. Linus my apologies for sending this directly but I have gotten an
> incredible amount of flak trying to fix this problem, and I would like
> not to leave an accidental regression whose cause is well known in
> 2.6.35 if I can help it.
"flak"?
{sigh}
No, it's just been an issue of fixing the issue in the correct manner.
You sent this patch a mere 2 days prior, and I've been away since then
and haven't even _had_ the chance to apply it to my tree and run it
through any testing.
Heck, it hasn't even been in the linux-next tree, and seen if it blows
up Andrew's notorious machine.
Next time, give me a chance to at least build your patch before getting
upset and routing around me. The other 2 patches you have sent me have
had testing in linux-next and will go to Linus later today, and I have
half-a-mind to revert this one, but I just know you will be willing to
fix up any problems found in it now :)
What a nice way to return from a 3day vacation...
greg k-h
^ permalink raw reply
* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Oleg Nesterov @ 2010-07-26 18:08 UTC (permalink / raw)
To: Sridhar Samudrala
Cc: Michael S. Tsirkin, Peter Zijlstra, Tejun Heo, Ingo Molnar,
netdev, lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <1280166688.3375.5.camel@localhost>
On 07/26, Sridhar Samudrala wrote:
>
> I have been testing out a similar patch that uses kernel_thread() without CLONE_FILES
> flag rather than create_kthread() and then closing the files.
!CLONE_FILES can't help. copy_files() does dup_fd() in this case.
The child still inherits the files.
> Either version should be fine.
I think neither version is fine ;)
exit_files() is not enough too. How about the signals, reparenting?
I already forgot all details, probably I missed somethig. But it
seems to me that it is better to just export get/set affinity and
forget about all complications.
Oleg.
^ permalink raw reply
* Re: tcpdump <-> FCoE support
From: Joe Eykholt @ 2010-07-26 17:57 UTC (permalink / raw)
To: Loke, Chetan; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devel-s9riP+hp16TNLxjTenLetw
In-Reply-To: <D3F292ADF945FB49B35E96C94C2061B90C6F53AE-2s2rCY1e8UXHBhWB4kaBDUEOCMrvLtNR@public.gmane.org>
On 7/26/10 10:30 AM, Loke, Chetan wrote:
> 13:08:21.907971 00:yy:yy:yy:yy:yy (oui Unknown)> 00:xx:xx:xx:xx:xx (oui
> Unknown), ethertype Unknown (0x8906)
>
> On: tcpdump-4.0.0-3, tcpdump doesn't seem to have FCoE support(unless I
> capture it and pipe it to wireshark). Is there any effort currently in
> progress on adding FCoE parsers?
>
>
> Chetan Loke
Good question. There's no effort in progress that I know of.
I find wireshark very handy and do tend to capture with tcpdump -w
and then read the file with wireshark. tshark is handy if you
want a CLI mode.
If tcpdump interpreted FCoE frames, then we would want FC, FC-ELS,
and FCP. Does it already dissect any SCSI frames (e.g., iSCSI)?
I'm not sure how much work it would be, but given tshark
is around, I don't think it's that important.
It could at least be changed to print out FCoE instead of 0x8906,
and FIP instead of 0x8914.
Joe
^ permalink raw reply
* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Sridhar Samudrala @ 2010-07-26 17:51 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Oleg Nesterov, Peter Zijlstra, Tejun Heo, Ingo Molnar, netdev,
lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <20100726171230.GA27644@redhat.com>
On Mon, 2010-07-26 at 20:12 +0300, Michael S. Tsirkin wrote:
> On Fri, Jul 02, 2010 at 11:06:37PM +0200, Oleg Nesterov wrote:
> > On 07/02, Peter Zijlstra wrote:
> > >
> > > On Fri, 2010-07-02 at 11:01 -0700, Sridhar Samudrala wrote:
> > > >
> > > > Does it (Tejun's kthread_clone() patch) also inherit the
> > > > cgroup of the caller?
> > >
> > > Of course, its a simple do_fork() which inherits everything just as you
> > > would expect from a similar sys_clone()/sys_fork() call.
> >
> > Yes. And I'm afraid it can inherit more than we want. IIUC, this is called
> > from ioctl(), right?
> >
> > Then the new thread becomes the natural child of the caller, and it shares
> > ->mm with the parent. And files, dup_fd() without CLONE_FS.
> >
> > Signals. Say, if you send SIGKILL to this new thread, it can't sleep in
> > TASK_INTERRUPTIBLE or KILLABLE after that. And this SIGKILL can be sent
> > just because the parent gets SIGQUIT or abother coredumpable signal.
> > Or the new thread can recieve SIGSTOP via ^Z.
> >
> > Perhaps this is OK, I do not know. Just to remind that kernel_thread()
> > is merely clone(CLONE_VM).
> >
> > Oleg.
>
> With some machinery to stop it later, yes.
> Oleg, how does the below look to you?
>
> Here I explicitly drop the fds so we don't share them.
> CLONE_VM takes care of sharing the mm I think.
> About signals - for the vhost-net use this is OK as we use
> uninterruptible sleep anyway (like the new kthread_worker does).
>
> This code seems to work fine for me so far - any comments?
>
> ---
>
> diff --git a/include/linux/kthread.h b/include/linux/kthread.h
> index aabc8a1..72c7b17 100644
> --- a/include/linux/kthread.h
> +++ b/include/linux/kthread.h
> @@ -9,6 +9,11 @@ struct task_struct *kthread_create(int (*threadfn)(void *data),
> const char namefmt[], ...)
> __attribute__((format(printf, 3, 4)));
>
> +struct task_struct *kthread_create_inherit(int (*threadfn)(void *data),
> + void *data,
> + const char namefmt[], ...)
> + __attribute__((format(printf, 3, 4)));
> +
> /**
> * kthread_run - create and wake a thread.
> * @threadfn: the function to run until signal_pending(current).
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index 83911c7..b81588c 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -149,6 +149,38 @@ struct task_struct *kthread_create(int (*threadfn)(void *data),
> }
> EXPORT_SYMBOL(kthread_create);
>
> +/* Same as kthread_create, but inherit attributes (cgroups, priority, CPU mask)
> + * from current. */
> +struct task_struct *kthread_create_inherit(int (*threadfn)(void *data),
> + void *data,
> + const char namefmt[],
> + ...)
> +{
> + struct kthread_create_info create;
> +
> + create.threadfn = threadfn;
> + create.data = data;
> + init_completion(&create.done);
> +
> + create_kthread(&create);
> + wait_for_completion(&create.done);
> +
> + if (!IS_ERR(create.result)) {
> + va_list args;
> +
> + /* Don't share files with parent as drivers use release for
> + * close on exit, etc. */
> + exit_files(create.result);
> +
> + va_start(args, namefmt);
> + vsnprintf(create.result->comm, sizeof(create.result->comm),
> + namefmt, args);
> + va_end(args);
> + }
> + return create.result;
> +}
> +EXPORT_SYMBOL(kthread_create_inherit);
> +
> /**
> * kthread_bind - bind a just-created kthread to a cpu.
> * @p: thread created by kthread_create().
I have been testing out a similar patch that uses kernel_thread() without CLONE_FILES
flag rather than create_kthread() and then closing the files.
Either version should be fine.
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index aabc8a1..634eaf7 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -9,6 +9,11 @@ struct task_struct *kthread_create(int (*threadfn)(void *data),
const char namefmt[], ...)
__attribute__((format(printf, 3, 4)));
+struct task_struct *kthread_clone(int (*threadfn)(void *data),
+ void *data,
+ const char namefmt[], ...)
+ __attribute__((format(printf, 3, 4)));
+
/**
* kthread_run - create and wake a thread.
* @threadfn: the function to run until signal_pending(current).
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 83911c7..806dae5 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -149,6 +149,38 @@ struct task_struct *kthread_create(int (*threadfn)(void *data),
}
EXPORT_SYMBOL(kthread_create);
+struct task_struct *kthread_clone(int (*threadfn)(void *data),
+ void *data,
+ const char namefmt[],
+ ...)
+{
+ struct kthread_create_info create;
+ int pid;
+
+ create.threadfn = threadfn;
+ create.data = data;
+ init_completion(&create.done);
+ INIT_LIST_HEAD(&create.list);
+
+ pid = kernel_thread(kthread, &create, CLONE_FS);
+ if (pid < 0) {
+ create.result = ERR_PTR(pid);
+ complete(&create.done);
+ }
+ wait_for_completion(&create.done);
+
+ if (!IS_ERR(create.result)) {
+ va_list args;
+ va_start(args, namefmt);
+ vsnprintf(create.result->comm, sizeof(create.result->comm),
+ namefmt, args);
+ va_end(args);
+ }
+
+ return create.result;
+}
+EXPORT_SYMBOL(kthread_clone);
+
/**
* kthread_bind - bind a just-created kthread to a cpu.
* @p: thread created by kthread_create().
^ permalink raw reply related
* tcpdump <-> FCoE support
From: Loke, Chetan @ 2010-07-26 17:30 UTC (permalink / raw)
To: devel-s9riP+hp16TNLxjTenLetw, netdev-u79uwXL29TY76Z2rM5mHXA
Folks,
13:08:21.907971 00:yy:yy:yy:yy:yy (oui Unknown) > 00:xx:xx:xx:xx:xx (oui
Unknown), ethertype Unknown (0x8906)
On: tcpdump-4.0.0-3, tcpdump doesn't seem to have FCoE support(unless I
capture it and pipe it to wireshark). Is there any effort currently in
progress on adding FCoE parsers?
Chetan Loke
^ permalink raw reply
* Re: [PATCH UPDATED 1/3] vhost: replace vhost_workqueue with per-vhost kthread
From: Michael S. Tsirkin @ 2010-07-26 16:23 UTC (permalink / raw)
To: Tejun Heo
Cc: Oleg Nesterov, Sridhar Samudrala, netdev, lkml,
kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev, Jiri Kosina,
Thomas Gleixner, Ingo Molnar, Andi Kleen
In-Reply-To: <4C4DB247.9060709@kernel.org>
On Mon, Jul 26, 2010 at 06:05:27PM +0200, Tejun Heo wrote:
> Hello,
>
> On 07/26/2010 05:50 PM, Michael S. Tsirkin wrote:
> >> Hmmm... I'm not quite sure whether it's an optimization. I thought
> >> the patch was due to feeling uncomfortable about using barriers?
> >
> > Oh yes. But getting rid of barriers is what motivated me originally.
>
> Yeah, getting rid of barriers is always good. :-)
>
> > Is there a git tree with kthread_worker applied?
> > I might do this just for fun ...
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-next
>
> For the original implementaiton, please take a look at commit
> b56c0d8937e665a27d90517ee7a746d0aa05af46.
>
> * Can you please keep the outer goto repeat loop? I just don't like
> outermost for (;;).
Okay ... can we put the code in a {} scope to make it clear
where does the loop starts and ends?
> * Placing try_to_freeze() could be a bit annoying. It shouldn't be
> executed when there's a work to flush.
It currently seems to be executed when there is work to flush.
Is this wrong?
> * I think A - B <= 0 test would be more familiar. At least
> time_before/after() are implemented that way.
I am concerned that this overflows a signed integer -
which I seem to remeber that C99 disallows.
timer macros are on data path so might be worth the risk there,
but flush is slow path so better be safe?
> Thanks.
>
> --
> tejun
^ permalink raw reply
* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Michael S. Tsirkin @ 2010-07-26 17:12 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Peter Zijlstra, Sridhar Samudrala, Tejun Heo, Ingo Molnar, netdev,
lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <20100702210637.GA12433@redhat.com>
On Fri, Jul 02, 2010 at 11:06:37PM +0200, Oleg Nesterov wrote:
> On 07/02, Peter Zijlstra wrote:
> >
> > On Fri, 2010-07-02 at 11:01 -0700, Sridhar Samudrala wrote:
> > >
> > > Does it (Tejun's kthread_clone() patch) also inherit the
> > > cgroup of the caller?
> >
> > Of course, its a simple do_fork() which inherits everything just as you
> > would expect from a similar sys_clone()/sys_fork() call.
>
> Yes. And I'm afraid it can inherit more than we want. IIUC, this is called
> from ioctl(), right?
>
> Then the new thread becomes the natural child of the caller, and it shares
> ->mm with the parent. And files, dup_fd() without CLONE_FS.
>
> Signals. Say, if you send SIGKILL to this new thread, it can't sleep in
> TASK_INTERRUPTIBLE or KILLABLE after that. And this SIGKILL can be sent
> just because the parent gets SIGQUIT or abother coredumpable signal.
> Or the new thread can recieve SIGSTOP via ^Z.
>
> Perhaps this is OK, I do not know. Just to remind that kernel_thread()
> is merely clone(CLONE_VM).
>
> Oleg.
With some machinery to stop it later, yes.
Oleg, how does the below look to you?
Here I explicitly drop the fds so we don't share them.
CLONE_VM takes care of sharing the mm I think.
About signals - for the vhost-net use this is OK as we use
uninterruptible sleep anyway (like the new kthread_worker does).
This code seems to work fine for me so far - any comments?
---
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index aabc8a1..72c7b17 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -9,6 +9,11 @@ struct task_struct *kthread_create(int (*threadfn)(void *data),
const char namefmt[], ...)
__attribute__((format(printf, 3, 4)));
+struct task_struct *kthread_create_inherit(int (*threadfn)(void *data),
+ void *data,
+ const char namefmt[], ...)
+ __attribute__((format(printf, 3, 4)));
+
/**
* kthread_run - create and wake a thread.
* @threadfn: the function to run until signal_pending(current).
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 83911c7..b81588c 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -149,6 +149,38 @@ struct task_struct *kthread_create(int (*threadfn)(void *data),
}
EXPORT_SYMBOL(kthread_create);
+/* Same as kthread_create, but inherit attributes (cgroups, priority, CPU mask)
+ * from current. */
+struct task_struct *kthread_create_inherit(int (*threadfn)(void *data),
+ void *data,
+ const char namefmt[],
+ ...)
+{
+ struct kthread_create_info create;
+
+ create.threadfn = threadfn;
+ create.data = data;
+ init_completion(&create.done);
+
+ create_kthread(&create);
+ wait_for_completion(&create.done);
+
+ if (!IS_ERR(create.result)) {
+ va_list args;
+
+ /* Don't share files with parent as drivers use release for
+ * close on exit, etc. */
+ exit_files(create.result);
+
+ va_start(args, namefmt);
+ vsnprintf(create.result->comm, sizeof(create.result->comm),
+ namefmt, args);
+ va_end(args);
+ }
+ return create.result;
+}
+EXPORT_SYMBOL(kthread_create_inherit);
+
/**
* kthread_bind - bind a just-created kthread to a cpu.
* @p: thread created by kthread_create().
^ permalink raw reply related
* Re: [PATCH UPDATED 1/3] vhost: replace vhost_workqueue with per-vhost kthread
From: Michael S. Tsirkin @ 2010-07-26 16:57 UTC (permalink / raw)
To: Tejun Heo
Cc: Oleg Nesterov, Sridhar Samudrala, netdev, lkml,
kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev, Jiri Kosina,
Thomas Gleixner, Ingo Molnar, Andi Kleen
In-Reply-To: <4C4DB466.6000409@kernel.org>
Here's an untested patch forward-ported from vhost
(works fine for vhost).
kthread_worker: replace barriers+atomics with a lock
We can save some cycles and make code simpler by
reusing worker lock for flush, instead of atomics.
flush_kthread_work needs to get worker pointer for
this to work.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index 685ea65..19ae9f2 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -58,7 +58,7 @@ struct kthread_work {
struct list_head node;
kthread_work_func_t func;
wait_queue_head_t done;
- atomic_t flushing;
+ int flushing;
int queue_seq;
int done_seq;
};
@@ -72,7 +72,7 @@ struct kthread_work {
.node = LIST_HEAD_INIT((work).node), \
.func = (fn), \
.done = __WAIT_QUEUE_HEAD_INITIALIZER((work).done), \
- .flushing = ATOMIC_INIT(0), \
+ .flushing = 0, \
}
#define DEFINE_KTHREAD_WORKER(worker) \
@@ -96,7 +96,8 @@ int kthread_worker_fn(void *worker_ptr);
bool queue_kthread_work(struct kthread_worker *worker,
struct kthread_work *work);
-void flush_kthread_work(struct kthread_work *work);
+void flush_kthread_work(struct kthread_worker *worker,
+ struct kthread_work *work);
void flush_kthread_worker(struct kthread_worker *worker);
#endif /* _LINUX_KTHREAD_H */
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 2dc3786..461f58d 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -283,10 +283,12 @@ int kthreadd(void *unused)
int kthread_worker_fn(void *worker_ptr)
{
struct kthread_worker *worker = worker_ptr;
- struct kthread_work *work;
+ struct kthread_work *work = NULL;
+ spin_lock_irq(&worker->lock);
WARN_ON(worker->task);
worker->task = current;
+ spin_unlock_irq(&worker->lock);
repeat:
set_current_state(TASK_INTERRUPTIBLE); /* mb paired w/ kthread_stop */
@@ -298,23 +300,23 @@ repeat:
return 0;
}
- work = NULL;
spin_lock_irq(&worker->lock);
+ if (work) {
+ work->done_seq = work->queue_seq;
+ if (work->flushing)
+ wake_up_all(&work->done);
+ }
if (!list_empty(&worker->work_list)) {
work = list_first_entry(&worker->work_list,
struct kthread_work, node);
list_del_init(&work->node);
- }
+ } else
+ work = NULL;
spin_unlock_irq(&worker->lock);
if (work) {
__set_current_state(TASK_RUNNING);
work->func(work);
- smp_wmb(); /* wmb worker-b0 paired with flush-b1 */
- work->done_seq = work->queue_seq;
- smp_mb(); /* mb worker-b1 paired with flush-b0 */
- if (atomic_read(&work->flushing))
- wake_up_all(&work->done);
} else if (!freezing(current))
schedule();
@@ -353,31 +355,33 @@ EXPORT_SYMBOL_GPL(queue_kthread_work);
/**
* flush_kthread_work - flush a kthread_work
+ * @worker: where work might be running
* @work: work to flush
*
* If @work is queued or executing, wait for it to finish execution.
*/
-void flush_kthread_work(struct kthread_work *work)
+void flush_kthread_work(struct kthread_worker *worker,
+ struct kthread_work *work)
{
- int seq = work->queue_seq;
+ int seq
- atomic_inc(&work->flushing);
-
- /*
- * mb flush-b0 paired with worker-b1, to make sure either
- * worker sees the above increment or we see done_seq update.
- */
- smp_mb__after_atomic_inc();
+ spin_lock_irq(&worker->lock);
+ seq = work->queue_seq;
+ ++work->flushing;
+ spin_unlock_irq(&worker->lock);
/* A - B <= 0 tests whether B is in front of A regardless of overflow */
- wait_event(work->done, seq - work->done_seq <= 0);
- atomic_dec(&work->flushing);
-
- /*
- * rmb flush-b1 paired with worker-b0, to make sure our caller
- * sees every change made by work->func().
- */
- smp_mb__after_atomic_dec();
+ wait_event(work->done,
+ ({
+ int done;
+ spin_lock_irq(&worker->lock);
+ delta = seq - work->done_seq <= 0;
+ spin_unlock_irq(&worker->lock);
+ done;
+ });
+ spin_lock_irq(&worker->lock);
+ --work->flushing;
+ spin_unlock_irq(&worker->lock);
}
EXPORT_SYMBOL_GPL(flush_kthread_work);
^ permalink raw reply related
* Re: [PATCH UPDATED 1/3] vhost: replace vhost_workqueue with per-vhost kthread
From: Michael S. Tsirkin @ 2010-07-26 16:51 UTC (permalink / raw)
To: Tejun Heo
Cc: Oleg Nesterov, Sridhar Samudrala, netdev, lkml,
kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev, Jiri Kosina,
Thomas Gleixner, Ingo Molnar, Andi Kleen
In-Reply-To: <4C4DB466.6000409@kernel.org>
On Mon, Jul 26, 2010 at 06:14:30PM +0200, Tejun Heo wrote:
> Just one more thing.
I noticed that with vhost, flush_work was getting the worker
pointer as well. Can we live with this API change?
--
MST
^ permalink raw reply
* Re: [PATCH] ip: correctly report 802.15.4 link type
From: Stephen Hemminger @ 2010-07-26 16:35 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: stephen.hemminger, netdev
In-Reply-To: <1280089683-12020-2-git-send-email-jengelh@medozas.de>
On Sun, 25 Jul 2010 22:28:02 +0200
Jan Engelhardt <jengelh@medozas.de> wrote:
> Up until now, the "hardwpan" devices were displayed as link/[804].
>
> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
> ---
> lib/ll_types.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/lib/ll_types.c b/lib/ll_types.c
> index 846cdb0..1cc46b6 100644
> --- a/lib/ll_types.c
> +++ b/lib/ll_types.c
> @@ -125,6 +125,9 @@ __PF(IEEE80211_PRISM,ieee802.11/prism)
> #ifdef ARPHRD_IEEE80211_RADIOTAP
> __PF(IEEE80211_RADIOTAP,ieee802.11/radiotap)
> #endif
> +#ifdef ARPHRD_IEEE802154
> +__PF(IEEE802154, ieee802.15.4)
> +#endif
> #ifdef ARPHRD_NONE
> __PF(NONE, none)
> #endif
Already fixed.
--
commit 4dbda0f482b8947d064b3f82992394033de6616c
Author: Stephen Hemminger <stephen.hemminger@vyatta.com>
Date: Fri Jul 23 13:12:12 2010 -0700
Update ARP header type table
Add all current values. Since if_arp.h is included, get rid
of ifdefs'. Make table constant.
^ permalink raw reply
* Re: [PATCH UPDATED 1/3] vhost: replace vhost_workqueue with per-vhost kthread
From: Michael S. Tsirkin @ 2010-07-26 16:31 UTC (permalink / raw)
To: Tejun Heo
Cc: Oleg Nesterov, Sridhar Samudrala, netdev, lkml,
kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev, Jiri Kosina,
Thomas Gleixner, Ingo Molnar, Andi Kleen
In-Reply-To: <4C4DB466.6000409@kernel.org>
On Mon, Jul 26, 2010 at 06:14:30PM +0200, Tejun Heo wrote:
> Just one more thing.
>
> On 07/26/2010 06:05 PM, Tejun Heo wrote:
> > * Placing try_to_freeze() could be a bit annoying. It shouldn't be
> > executed when there's a work to flush.
BTW why is this important?
We could always get another work and flush right after
try_to_freeze, and then flush would block for a long time.
BTW the vhost patch you sent does not do this at all.
I am guessing it is because our thread is not freezable?
> * Similar issue exists for kthread_stop(). The kthread shouldn't exit
> while there's a work to flush (please note that kthread_worker
> interface allows detaching / attaching worker kthread during
> operation, so it should remain in consistent state with regard to
> flushing).
>
> Thanks.
Not sure I agree here. Users must synchronise flush and stop calls.
Otherwise a work might get queued after stop is called, and
you won't be able to flush it.
> --
> tejun
^ permalink raw reply
* Re: [patch] usbnet: fix 100% CPU use on suspended device
From: Oliver Neukum @ 2010-07-26 16:21 UTC (permalink / raw)
To: Alan Stern
Cc: Elly Jones, David Miller, netdev-u79uwXL29TY76Z2rM5mHXA, USB list
In-Reply-To: <Pine.LNX.4.44L0.1007261106350.1550-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
Am Montag, 26. Juli 2010, 17:13:23 schrieb Alan Stern:
> On Mon, 26 Jul 2010, Elly Jones wrote:
>
> > > This isn't right. The problem should be fixed some other way. Under
> > > what circumstances are URBs submitted incorrectly?
> >
> > When the device is autosuspended. What is the proper thing for a
> > device to do here?
>
> From looking at the code, it appears that the EVENT_DEV_ASLEEP flag
> should be tested in usbnet_bh() the way it is in rx_submit(). But I'm
> not an expert on usbnet; we should ask someone who is, like Oliver.
Sorry, I didn't notice this thread.
The correct way to check for autosuspend in usbnet is to look
at EVENT_DEV_ASLEEP under txq.lock. That being said, usbnet_bh()
uses rx_submit() which does the correct check. The bug seems to be
a lack of error handling in usbnet_bh() regarding the return of rx_submit()
Regards
Oliver
--
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
^ permalink raw reply
* Re: [PATCH UPDATED 1/3] vhost: replace vhost_workqueue with per-vhost kthread
From: Tejun Heo @ 2010-07-26 16:14 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Oleg Nesterov, Sridhar Samudrala, netdev, lkml,
kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev, Jiri Kosina,
Thomas Gleixner, Ingo Molnar, Andi Kleen
In-Reply-To: <4C4DB247.9060709@kernel.org>
Just one more thing.
On 07/26/2010 06:05 PM, Tejun Heo wrote:
> * Placing try_to_freeze() could be a bit annoying. It shouldn't be
> executed when there's a work to flush.
* Similar issue exists for kthread_stop(). The kthread shouldn't exit
while there's a work to flush (please note that kthread_worker
interface allows detaching / attaching worker kthread during
operation, so it should remain in consistent state with regard to
flushing).
Thanks.
--
tejun
^ permalink raw reply
* RE: e1000e crashes with 2.6.34.x and ThinkPad T60
From: Allan, Bruce W @ 2010-07-26 16:13 UTC (permalink / raw)
To: Marc Haber, Linux Kernel Developers,
Linux Kernel Network Developers, "e1000-devel@lists.sourcef
In-Reply-To: <20100724092644.GA13353@torres.zugschlus.de>
On Saturday, July 24, 2010 2:27 AM, Marc Haber wrote:
> Hi,
>
> I have a new notebook, a Thinkpad T60, which is freezing in random
> intervals (like 30 minutes to two days) as long as I am using the
> on-board wired ethernet interface, which is an e1000e, [8086:109a]. As
> long as I keep using the WLAN, the system runs for weeks despite
> frequent suspend/resume cycles etc. The crashes seem really to be tied
> to using the wired ethernet. This is a hard freeze, with nothing
> happening on the system, only a long push on the power button helps.
>
> Additionally, sometimes, probably after suspend/resume, the wired
> ethernet does not come up properly again, ip addr claims "NO CARRIER"
> even if the LEDs on the interface and on the switch claim that there
> was a link. No packets are received by the interface when it's at this
> stage.
>
> Both issues appear with 2.6.34 and 2.6.34.1. I didn't try any of these
> issues with an older kernel, 2.6.34 was already out when I started
> using the T60.
>
> To rule out defective hardware, I have tried with a second T60, with
> the same results.
>
> Full dmesg and lspci-nn attached, please say if you need more.
>
> Greetings
> Marc
>
> P.S.: Please Cc: me on replies, both linux-kernel and netdev are too
> big for me to timely follow. I am subscribed to both lists, but a Cc
> helps in getting a faster reply.
>
> --
> -----------------------------------------------------------------------------
> Marc Haber | "I don't trust Computers. They | Mailadresse im
> Header Mannheim, Germany | lose things." Winona Ryder | Fon: *49
> 621 72739834 Nordisch by Nature | How to make an American Quilt |
> Fax: *49 3221 2323190
Adding e1000-devel (the Intel LAN developers list).
Please supply the full dmesg you meant to attach with the original report, as well as the output of lspci -vvv.
Thanks,
Bruce.
^ permalink raw reply
* Re: [PATCH UPDATED 1/3] vhost: replace vhost_workqueue with per-vhost kthread
From: Tejun Heo @ 2010-07-26 16:05 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Oleg Nesterov, Sridhar Samudrala, netdev, lkml,
kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev, Jiri Kosina,
Thomas Gleixner, Ingo Molnar, Andi Kleen
In-Reply-To: <20100726155014.GA26412@redhat.com>
Hello,
On 07/26/2010 05:50 PM, Michael S. Tsirkin wrote:
>> Hmmm... I'm not quite sure whether it's an optimization. I thought
>> the patch was due to feeling uncomfortable about using barriers?
>
> Oh yes. But getting rid of barriers is what motivated me originally.
Yeah, getting rid of barriers is always good. :-)
> Is there a git tree with kthread_worker applied?
> I might do this just for fun ...
git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-next
For the original implementaiton, please take a look at commit
b56c0d8937e665a27d90517ee7a746d0aa05af46.
* Can you please keep the outer goto repeat loop? I just don't like
outermost for (;;).
* Placing try_to_freeze() could be a bit annoying. It shouldn't be
executed when there's a work to flush.
* I think A - B <= 0 test would be more familiar. At least
time_before/after() are implemented that way.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH UPDATED 1/3] vhost: replace vhost_workqueue with per-vhost kthread
From: Michael S. Tsirkin @ 2010-07-26 15:51 UTC (permalink / raw)
To: Tejun Heo
Cc: Oleg Nesterov, Sridhar Samudrala, netdev, lkml,
kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev, Jiri Kosina,
Thomas Gleixner, Ingo Molnar, Andi Kleen
In-Reply-To: <4C4DADD6.90507@kernel.org>
On Mon, Jul 26, 2010 at 05:46:30PM +0200, Tejun Heo wrote:
> On 07/26/2010 05:34 PM, Tejun Heo wrote:
> > Hello,
> >
> > On 07/26/2010 05:25 PM, Michael S. Tsirkin wrote:
> >> BTW, kthread_worker would benefit from the optimization I implemented
> >> here as well.
> >
> > Hmmm... I'm not quite sure whether it's an optimization. I thought
> > the patch was due to feeling uncomfortable about using barriers? Is
> > it an optimization?
>
> Yeah, one less smp_mb() in execution path. The lock dancing in
> flush() is ugly but then again mucking with barriers could be harder
> to understand. Care to send a patch against wq#for-next tree?
>
> Thanks.
Sure. Where's that, exactly?
> --
> tejun
^ permalink raw reply
* Re: [PATCH UPDATED 1/3] vhost: replace vhost_workqueue with per-vhost kthread
From: Michael S. Tsirkin @ 2010-07-26 15:50 UTC (permalink / raw)
To: Tejun Heo
Cc: Oleg Nesterov, Sridhar Samudrala, netdev, lkml,
kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev, Jiri Kosina,
Thomas Gleixner, Ingo Molnar, Andi Kleen
In-Reply-To: <4C4DAB14.5050809@kernel.org>
On Mon, Jul 26, 2010 at 05:34:44PM +0200, Tejun Heo wrote:
> Hello,
>
> On 07/26/2010 05:25 PM, Michael S. Tsirkin wrote:
> > BTW, kthread_worker would benefit from the optimization I implemented
> > here as well.
>
> Hmmm... I'm not quite sure whether it's an optimization. I thought
> the patch was due to feeling uncomfortable about using barriers?
Oh yes. But getting rid of barriers is what motivated me originally.
> Is it an optimization?
>
> Thanks.
Yes, sure. This removes atomic read and 2 barrier operations on data path. And
it does not add any new synchronization: instead, we reuse the lock that we
take anyway. The relevant part is:
+ if (work) {
+ __set_current_state(TASK_RUNNING);
+ work->fn(work);
+ } else
+ schedule();
- if (work) {
- __set_current_state(TASK_RUNNING);
- work->fn(work);
- smp_wmb(); /* wmb worker-b0 paired with flush-b1 */
- work->done_seq = work->queue_seq;
- smp_mb(); /* mb worker-b1 paired with flush-b0 */
- if (atomic_read(&work->flushing))
- wake_up_all(&work->done);
- } else
- schedule();
-
- goto repeat;
Is there a git tree with kthread_worker applied?
I might do this just for fun ...
> --
> tejun
^ permalink raw reply
* Re: [PATCH] sky2: Fix most checkpatch errors
From: Stephen Hemminger @ 2010-07-26 15:46 UTC (permalink / raw)
To: Mike McCormack; +Cc: netdev
In-Reply-To: <4C4D8D67.7080008@ring3k.org>
On Mon, 26 Jul 2010 22:28:07 +0900
Mike McCormack <mikem@ring3k.org> wrote:
> Tidy up whitespace and formatting to reduce from
> 38 errors to 4 and 20 fewer warnings.
>
> Signed-off-by: Mike McCormack <mikem@ring3k.org>
Please don't bother fixing the stupid checkpatch whitespace noise.
Any patch that just substitutes space for tab (or vice versa) with
no human visible impact is just stupid.
But there are some useful tidbits in there.
--- a/drivers/net/sky2.c 2010-07-26 08:40:37.554612692 -0700
+++ b/drivers/net/sky2.c 2010-07-26 08:44:18.683011839 -0700
@@ -79,7 +79,7 @@
#define SKY2_EEPROM_MAGIC 0x9955aabb
-#define RING_NEXT(x,s) (((x)+1) & ((s)-1))
+#define RING_NEXT(x, s) (((x)+1) & ((s)-1))
static const u32 default_msg =
NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_LINK
@@ -172,7 +172,7 @@ static int gm_phy_write(struct sky2_hw *
udelay(10);
}
- dev_warn(&hw->pdev->dev,"%s: phy write timeout\n", hw->dev[port]->name);
+ dev_warn(&hw->pdev->dev, "%s: phy write timeout\n", hw->dev[port]->name);
return -ETIMEDOUT;
io_error:
@@ -1067,7 +1067,7 @@ static inline struct sky2_rx_le *sky2_ne
return le;
}
-static unsigned sky2_get_rx_threshold(struct sky2_port* sky2)
+static unsigned sky2_get_rx_threshold(struct sky2_port *sky2)
{
unsigned size;
@@ -1078,7 +1078,7 @@ static unsigned sky2_get_rx_threshold(st
return (size - 8) / sizeof(u32);
}
-static unsigned sky2_get_rx_data_size(struct sky2_port* sky2)
+static unsigned sky2_get_rx_data_size(struct sky2_port *sky2)
{
struct rx_ring_info *re;
unsigned size;
@@ -3014,7 +3014,7 @@ static int __devinit sky2_init(struct sk
hw->chip_id = sky2_read8(hw, B2_CHIP_ID);
hw->chip_rev = (sky2_read8(hw, B2_MAC_CFG) & CFG_CHIP_R_MSK) >> 4;
- switch(hw->chip_id) {
+ switch (hw->chip_id) {
case CHIP_ID_YUKON_XL:
hw->flags = SKY2_HW_GIGABIT | SKY2_HW_NEWER_PHY;
if (hw->chip_rev < CHIP_REV_YU_XL_A2)
@@ -3685,7 +3685,7 @@ static int sky2_set_mac_address(struct n
return 0;
}
-static void inline sky2_add_filter(u8 filter[8], const u8 *addr)
+static inline void sky2_add_filter(u8 filter[8], const u8 *addr)
{
u32 bit;
@@ -3911,7 +3911,7 @@ static int sky2_set_coalesce(struct net_
return -EINVAL;
if (ecmd->rx_max_coalesced_frames > RX_MAX_PENDING)
return -EINVAL;
- if (ecmd->rx_max_coalesced_frames_irq >RX_MAX_PENDING)
+ if (ecmd->rx_max_coalesced_frames_irq > RX_MAX_PENDING)
return -EINVAL;
if (ecmd->tx_coalesce_usecs == 0)
@@ -4372,7 +4372,7 @@ static int sky2_debug_show(struct seq_fi
seq_printf(seq, "%u:", idx);
sop = 0;
- switch(le->opcode & ~HW_OWNER) {
+ switch (le->opcode & ~HW_OWNER) {
case OP_ADDR64:
seq_printf(seq, " %#x:", a);
break;
@@ -4441,7 +4441,7 @@ static int sky2_device_event(struct noti
if (dev->netdev_ops->ndo_open != sky2_up || !sky2_debug)
return NOTIFY_DONE;
- switch(event) {
+ switch (event) {
case NETDEV_CHANGENAME:
if (sky2->debugfs) {
sky2->debugfs = debugfs_rename(sky2_debug, sky2->debugfs,
@@ -4636,7 +4636,7 @@ static int __devinit sky2_test_msi(struc
struct pci_dev *pdev = hw->pdev;
int err;
- init_waitqueue_head (&hw->msi_wait);
+ init_waitqueue_head(&hw->msi_wait);
sky2_write32(hw, B0_IMSK, Y2_IS_IRQ_SW);
^ permalink raw reply
* Re: [PATCH UPDATED 1/3] vhost: replace vhost_workqueue with per-vhost kthread
From: Tejun Heo @ 2010-07-26 15:46 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Oleg Nesterov, Sridhar Samudrala, netdev, lkml,
kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev, Jiri Kosina,
Thomas Gleixner, Ingo Molnar, Andi Kleen
In-Reply-To: <4C4DAB14.5050809@kernel.org>
On 07/26/2010 05:34 PM, Tejun Heo wrote:
> Hello,
>
> On 07/26/2010 05:25 PM, Michael S. Tsirkin wrote:
>> BTW, kthread_worker would benefit from the optimization I implemented
>> here as well.
>
> Hmmm... I'm not quite sure whether it's an optimization. I thought
> the patch was due to feeling uncomfortable about using barriers? Is
> it an optimization?
Yeah, one less smp_mb() in execution path. The lock dancing in
flush() is ugly but then again mucking with barriers could be harder
to understand. Care to send a patch against wq#for-next tree?
Thanks.
--
tejun
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox