linux-kernel - Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread

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]

Date:	Tue, 02 Aug 2016 15:29:48 +0200
From:	Oliver Neukum <oneukum@...e.com>
To:	Michal Hocko <mhocko@...nel.org>
Cc:	Geliang Tang <geliangtang@....com>,
	Johannes Weiner <hannes@...xchg.org>,
	Bhaktipriya Shridhar <bhaktipriya96@...il.com>,
	"GeyslanG.Bem@...yakshetra" <geyslan@...il.com>,
	Masanari Iida <standby24x7@...il.com>,
	Tejun Heo <tj@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Alan Stern <stern@...land.harvard.edu>,
	Vlastimil Babka <vbabka@...e.cz>,
	Mel Gorman <mgorman@...hsingularity.net>,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
	Saurabh Karajgaonkar <skarajga@...teon.com>
Subject: Re: [RFC] usb: host: u132-hcd: Remove deprecated
 create_singlethread_workqueue

On Tue, 2016-08-02 at 14:48 +0200, Michal Hocko wrote:
> On Tue 02-08-16 13:44:48, Oliver Neukum wrote:
> > On Tue, 2016-08-02 at 13:34 +0200, Michal Hocko wrote:
> > > On Tue 02-08-16 12:03:23, Oliver Neukum wrote:
> > > > On Tue, 2016-08-02 at 10:18 +0200, Michal Hocko wrote:
> > > > > On Tue 02-08-16 10:06:12, Oliver Neukum wrote:
> > > > > > On Mon, 2016-08-01 at 10:20 -0400, Tejun Heo wrote:
> > > > > > > Hello,
> > > > > > 
> > > > > > > > If any real IO depends on those devices then this is not sufficient and
> > > > > > > > they need some form of guarantee for progress (aka mempool).
> > > > > > > 
> > > > > > > Oliver, Alan, what do you think?  If USB itself can't operate without
> > > > > > > allocating memory during transactions, whatever USB storage drivers
> > > > > > 
> > > > > > It cannot. The IO must be described to the hardware with a data
> > > > > > structure in memory.
> > > > > > 
> > > > > > > are doing isn't all that meaningful.  Can we proceed with the
> > > > > > > workqueue patches?  Also, it could be that the only thing GFP_NOIO and
> > > > > > > GFP_ATOMIC are doing is increasing the chance of IO failures under
> > > > > > > memory pressure.  Maybe it'd be a good idea to reconsider the
> > > > > > > approach?
> > > > > > 
> > > > > > We had actual deadlocks with GFP_KERNEL. It seems to me that the SCSI
> > > > > > layer can deal with IO that cannot be completed due to a lack of memory
> > > > > > at least somewhat, but a deadlock within a driver would obviously be
> > > > > > deadly. So I don't think that mempools would remove the need for
> > > > > > GFP_NOIO as there are places in usbcore we cannot enter the page
> > > > > > laundering path from. They are an additional need.
> > > > > 
> > > > > OK, I guess there is some misunderstanding here. I believe that Tejun
> > > > > wasn't arguing to drop GFP_NOIO. It might be really needed for the dead
> > > > > lock avoidance. No question about that. The whole point is that
> > > > > WQ_RECLAIM might be completely pointless because a rescuer wouldn't help
> > > > > much if the work item would do GFP_NOIO and get stuck in the page
> > > > > allocator.
> > > > 
> > > > But that can be a problem only if the items on the work queue are
> > > > actually run and without WQ_MEM_RECLAIM that guarantee cannot be made.
> > > > We can deal with failures of memory allocation. But the requests
> > > > must actually terminate.
> > > 
> > > I think you have missed my point. So let me ask differently. What is the
> > > difference between your work item not running at all or looping
> > > endlessly with GFP_NOIO inside the page allocator? If that particular
> > > work item is necessary for the further progress then the system is
> > > screwed one way or another.
> > 
> > The key difference is that I could give the right parameters to the
> > kmalloc() call. If it doesn't run, I am surely screwed. Thus I conclude
> > that WQ_RECLAIM needs to be set.
> 
> Just to make sure I understand. So you will never call kmalloc with
> __GFP_DIRECT_RECLAIM from the rescuer context, right?

Apparently if that is the requirement USB will have to define its own
set of flags to use in such contexts. But still the calls as currently
executed work. Taking away WQ_MEM_RECLAIM would create danger of
introducing a regression. The issue with __GFP_DIRECT_RECLAIM already
exists and can be fixed.

	Regards
		Oliver