linux-kernel - RE: [PATCH v3 0/6] usb: hub: convert khubd into workqueue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <A2CA0424C0A6F04399FB9E1CD98E0304844D9FFC@US01WEMBX2.internal.synopsys.com>
Date:	Sat, 20 Sep 2014 20:41:21 +0000
From:	Paul Zimmerman <Paul.Zimmerman@...opsys.com>
To:	Alan Stern <stern@...land.harvard.edu>,
	Petr Mladek <pmladek@...e.cz>
CC:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Tejun Heo <tj@...nel.org>,
	Joe Lawrence <joe.lawrence@...atus.com>,
	Jiri Kosina <jkosina@...e.cz>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3 0/6] usb: hub: convert khubd into workqueue

> From: linux-usb-owner@...r.kernel.org [mailto:linux-usb-owner@...r.kernel.org] On Behalf Of Alan Stern
> Sent: Friday, September 19, 2014 12:39 PM
> 
> On Fri, 19 Sep 2014, Petr Mladek wrote:
> 
> > The 3rd version of the patchset is slightly reordered and refactored
> > as suggested earlier. See below for more details.
> >
> > IMHO, the result is more clean. But feel free to ask me to revert it.
> > I do not want to make the review more complicated. Well, I double checked
> > the diff of patches files between v2 and v3. It is pretty small and only
> > the expected changes are there.
> >
> > Changes in:
> >     + v3:
> > 	+ split the optimization of hub->hdev reference counting
> > 	  into separate patch; usable standalone (1st patch)
> >
> > 	+ switch order of the two patches that convert the kthread
> > 	  and that remove the while cycle in hub_event(); it is
> > 	  cleaner (2nd and 3rd patch)
> >
> > 	+ use ordered workqueue by default; it makes it compatible
> > 	  with the original kthread solution (3rd patch)
> >
> >         + added optional patch that allows to process events
> > 	  in parallel (6th patch)
> >
> > 	+ fixed the suggested stylistic problems (2nd and 3rd patch)
> >
> >
> >     + v2:
> > 	+ solved potential races:
> > 	  	 + get hub->kref in kick_hub_wq()
> > 		 + call usb_get_dev(hdev) in hub_probe()
> > 		   and 	usb_put_dev(hdev) in hub_release()
> > 		 + INIT_WORK only once in hub_probe()
> >
> > 	 + do not call cancel_work_sync() in hub_disconnect()
> > 	   to keep it fast
> >
> > 	 + rename kick_khubd() to kick_hub_wq() already
> > 	   in the first patch; IMHO, it is cleaner while
> > 	   adding only very few changes
> >
> >
> > The workqueue API is well defined and tested. It has many options
> > that could be used to tune the scheduling. The code is usually
> > easier and thus more safe. It allows to avoid the extra thread
> > in most cases. It has has clearly defined behavior vrt. system suspend.
> >
> > This patchset converts khubd into the workqueue. It saves one thread,
> > lock, and list.
> >
> > It  looks huge but the main change is in the first patch. The rest is
> > simple renaming of functions, comments and documentation.
> >
> >
> > Thanks a lot Alan Stern and Tejun Heo for hints and guidance.
> >
> > The patches can be applied either against Linus' tree or linux-next.
> 
> Very nice work.  I haven't tried it out, but it all looks good.  No
> doubt we will be able to sort out unforeseen troubles if any arise,
> without much difficulty.

Hmm. What are the odds that some old kernel monitoring program, in an
ancient Fedora distribution let's say, will be checking for 'khubd' in
the kernel process list? Pretty high I would think.

I guess we'll find out once this hits mainline ;)

-- 
Paul

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