Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-25 Thread Oliver Neukum
On Wednesday 24 April 2013 07:31:25 Dan Williams wrote: > So how do we go forward from here... I'm happy to update the patchset > with anything needed to get davem to apply it. Have we decided to keep > the memflags for now, and thus should I just resubmit with a summary of > the discussion unde

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-24 Thread Dan Williams
On Fri, 2013-04-19 at 10:25 +0200, Oliver Neukum wrote: > On Thursday 18 April 2013 11:51:25 Ming Lei wrote: > > On Wed, Apr 17, 2013 at 2:55 PM, Oliver Neukum wrote: > > > > > If we have a function for starting a status URB we want to use it whenever > > > it applies, that is also when we need to

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-19 Thread Oliver Neukum
On Thursday 18 April 2013 11:51:25 Ming Lei wrote: > On Wed, Apr 17, 2013 at 2:55 PM, Oliver Neukum wrote: > > > If we have a function for starting a status URB we want to use it whenever > > it applies, that is also when we need to poll status for internal reason > > while > > an interface is up

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-17 Thread Ming Lei
On Wed, Apr 17, 2013 at 2:55 PM, Oliver Neukum wrote: > On Wednesday 17 April 2013 09:56:33 Ming Lei wrote: >> On Tue, Apr 16, 2013 at 3:57 PM, Oliver Neukum wrote: > >> > Generally, saying that somebody else has a problem is not an argument >> > as long as the fix is very simple. Of course it is

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-16 Thread Oliver Neukum
On Wednesday 17 April 2013 09:56:33 Ming Lei wrote: > On Tue, Apr 16, 2013 at 3:57 PM, Oliver Neukum wrote: > > Generally, saying that somebody else has a problem is not an argument > > as long as the fix is very simple. Of course it is a corner case, but why > > fail to solve it as long as the c

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-16 Thread Ming Lei
On Tue, Apr 16, 2013 at 3:57 PM, Oliver Neukum wrote: > On Tuesday 16 April 2013 09:15:45 Ming Lei wrote: >> On Mon, Apr 15, 2013 at 11:59 PM, Dan Williams wrote: >> > >> > So, what was the general consensus on this one? I know Oliver signed >> > off on it, but the discussion about memflags seem

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-16 Thread Oliver Neukum
On Tuesday 16 April 2013 09:15:45 Ming Lei wrote: > On Mon, Apr 15, 2013 at 11:59 PM, Dan Williams wrote: > > > > So, what was the general consensus on this one? I know Oliver signed > > off on it, but the discussion about memflags seemed to die out without a > > specific conclusion. davem might

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-15 Thread Ming Lei
On Mon, Apr 15, 2013 at 11:59 PM, Dan Williams wrote: > > So, what was the general consensus on this one? I know Oliver signed > off on it, but the discussion about memflags seemed to die out without a > specific conclusion. davem might be looking for that conclusion before > moving forward with

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-15 Thread Dan Williams
On Wed, 2013-04-10 at 15:30 -0500, Dan Williams wrote: > Some drivers (sierra_net) need the status interrupt URB > active even when the device is closed, because they receive > custom indications from firmware. Add functions to refcount > the status interrupt URB submit/kill operation so that > su

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-12 Thread Bjørn Mork
Oliver Neukum writes: > On Friday 12 April 2013 17:42:46 Ming Lei wrote: > >> Wrt. the usbnet_status_start() API, looks no good reason to call >> it in another queue context, it should be enough to call it in probe() or >> bind() if init_status() can be put before info->bind() in usbnet_probe(). >

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-12 Thread Ming Lei
On Fri, Apr 12, 2013 at 6:02 PM, Oliver Neukum wrote: > On Friday 12 April 2013 17:42:46 Ming Lei wrote: >> On Fri, Apr 12, 2013 at 2:37 PM, Oliver Neukum wrote: >> >> The work will complete when memory is reclaimed, and the rx/tx path is >> >> still working, so memory reclaim can continue and th

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-12 Thread Oliver Neukum
On Friday 12 April 2013 17:42:46 Ming Lei wrote: > On Fri, Apr 12, 2013 at 2:37 PM, Oliver Neukum wrote: > >> The work will complete when memory is reclaimed, and the rx/tx path is > >> still working, so memory reclaim can continue and the deadlock may not > >> be caused, may it? > > > > Only if t

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-12 Thread Ming Lei
On Fri, Apr 12, 2013 at 2:37 PM, Oliver Neukum wrote: >> The work will complete when memory is reclaimed, and the rx/tx path is >> still working, so memory reclaim can continue and the deadlock may not >> be caused, may it? > > Only if the memory allocation goes to the same interface. If the block

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-12 Thread Oliver Neukum
On Thursday 11 April 2013 10:28:33 Dan Williams wrote: > > Looks it isn't a good practice to duplicate code in above four functions, > > but > > it should be OK to merge first. > > Oliver requested this approach. I'd originally taken your suggestion to > merge them, but this proved somewhat more

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Oliver Neukum
On Thursday 11 April 2013 20:59:05 Ming Lei wrote: > On Thu, Apr 11, 2013 at 8:28 PM, Oliver Neukum wrote: > > On Thursday 11 April 2013 20:11:13 Ming Lei wrote: > >> On Thu, Apr 11, 2013 at 7:14 PM, Oliver Neukum wrote: > >> > > >> > Sorry, I misunderstood. > >> > >> No problem, :-) > >> > >> >

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Dan Williams
On Thu, 2013-04-11 at 10:31 +0800, Ming Lei wrote: > On Thu, Apr 11, 2013 at 4:30 AM, Dan Williams wrote: > > Some drivers (sierra_net) need the status interrupt URB > > active even when the device is closed, because they receive > > custom indications from firmware. Add functions to refcount > >

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Ming Lei
On Thu, Apr 11, 2013 at 8:28 PM, Oliver Neukum wrote: > On Thursday 11 April 2013 20:11:13 Ming Lei wrote: >> On Thu, Apr 11, 2013 at 7:14 PM, Oliver Neukum wrote: >> > >> > Sorry, I misunderstood. >> >> No problem, :-) >> >> > >> > Task A Task B

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Oliver Neukum
On Thursday 11 April 2013 20:11:13 Ming Lei wrote: > On Thu, Apr 11, 2013 at 7:14 PM, Oliver Neukum wrote: > > > > Sorry, I misunderstood. > > No problem, :-) > > > > > Task A Task B > > queue > > > > queue work > >

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Ming Lei
On Thu, Apr 11, 2013 at 7:54 PM, Oliver Neukum wrote: >> > Is this example >> > a) wrong, or >> > b) not applicable, or >> > c) to be excluded from the new API? >> >> IMO, it may be a) or b), and we can find many GFP_KERNEL usage >> inside usbnet(kevent(), ...). > > Only in the rx path. So it

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Ming Lei
On Thu, Apr 11, 2013 at 7:14 PM, Oliver Neukum wrote: > > Sorry, I misunderstood. No problem, :-) > > Task A Task B queue > > queue work > request a reset >

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Oliver Neukum
On Thursday 11 April 2013 19:42:53 Ming Lei wrote: > On Thu, Apr 11, 2013 at 7:08 PM, Bjørn Mork wrote: > > > > The docs for usb_submit_urb() in drivers/usb/core/urb.c lists some > > possible mem_flags use cases. Among these are (where (b) and (c) are > > cases needing GFP_ATOMIC and not applicabl

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Ming Lei
On Thu, Apr 11, 2013 at 7:08 PM, Bjørn Mork wrote: > > The docs for usb_submit_urb() in drivers/usb/core/urb.c lists some > possible mem_flags use cases. Among these are (where (b) and (c) are > cases needing GFP_ATOMIC and not applicable here): > > > * (3) If you use a kernel thread with a net

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Oliver Neukum
On Thursday 11 April 2013 18:03:09 Ming Lei wrote: > On Thu, Apr 11, 2013 at 5:53 PM, Oliver Neukum wrote: > > On Thursday 11 April 2013 16:09:16 Ming Lei wrote: > >> > >> Could you explain why work queue need GFP_NOIO? > > > > Your fix for the memory allocation depends on it happening in the same

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Bjørn Mork
Ming Lei writes: > OK, I say it again, GFP_KERNEL is enough to cover all cases, and the > mem_flags parameter is redundant. The docs for usb_submit_urb() in drivers/usb/core/urb.c lists some possible mem_flags use cases. Among these are (where (b) and (c) are cases needing GFP_ATOMIC and not app

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Ming Lei
On Thu, Apr 11, 2013 at 6:26 PM, Bjørn Mork wrote: > Ming Lei writes: > >> On Thu, Apr 11, 2013 at 6:04 PM, Bjørn Mork wrote: >>> Ming Lei writes: >>> >>> Again: What problem are you attempting to solve by removing the >>> mem_flags from the API? >> >> It is not about removing anything, we are

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Bjørn Mork
Ming Lei writes: > On Thu, Apr 11, 2013 at 6:04 PM, Bjørn Mork wrote: >> Ming Lei writes: >> >> Again: What problem are you attempting to solve by removing the >> mem_flags from the API? > > It is not about removing anything, we are discussing one new API > (include the parameters) to be introd

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Ming Lei
On Thu, Apr 11, 2013 at 6:04 PM, Bjørn Mork wrote: > Ming Lei writes: > > Again: What problem are you attempting to solve by removing the > mem_flags from the API? It is not about removing anything, we are discussing one new API (include the parameters) to be introduced. Thanks, -- Ming Lei --

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Ming Lei
On Thu, Apr 11, 2013 at 6:04 PM, Bjørn Mork wrote: > Ming Lei writes: > > I think you are turning this the wrong way around. Please explain why > there are no use cases where different flags are needed. You seem to be > only concerned about the resume case. This API is not limited to > resuming.

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Bjørn Mork
Ming Lei writes: > On Thu, Apr 11, 2013 at 4:06 PM, Bjørn Mork wrote: >> Oliver Neukum writes: >> >> My immediate thought was that someone also might want to use this new >> API from atomic context, e.g. calling it directly from an URB callback. > > I am wondering it is a valid use case, and if

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Ming Lei
On Thu, Apr 11, 2013 at 5:53 PM, Oliver Neukum wrote: > On Thursday 11 April 2013 16:09:16 Ming Lei wrote: >> >> Could you explain why work queue need GFP_NOIO? > > Your fix for the memory allocation depends on it happening in the same > context. If you execute code on a work queue this happens in

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Oliver Neukum
On Thursday 11 April 2013 16:37:53 Ming Lei wrote: > On Thu, Apr 11, 2013 at 4:06 PM, Bjørn Mork wrote: > > Oliver Neukum writes: > > > > My immediate thought was that someone also might want to use this new > > API from atomic context, e.g. calling it directly from an URB callback. > > I am won

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Oliver Neukum
On Thursday 11 April 2013 16:09:16 Ming Lei wrote: > On Thu, Apr 11, 2013 at 2:50 PM, Oliver Neukum wrote: > > On Thursday 11 April 2013 10:31:31 Ming Lei wrote: > > > >> 'mem_flags' isn't needed any more since we can apply allocation > >> of GFP_NOIO automatically in resume path now, and you can

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Ming Lei
On Thu, Apr 11, 2013 at 4:06 PM, Bjørn Mork wrote: > Oliver Neukum writes: > > My immediate thought was that someone also might want to use this new > API from atomic context, e.g. calling it directly from an URB callback. I am wondering it is a valid use case, and if there is one URB submitted,

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Ming Lei
On Thu, Apr 11, 2013 at 2:50 PM, Oliver Neukum wrote: > On Thursday 11 April 2013 10:31:31 Ming Lei wrote: > >> 'mem_flags' isn't needed any more since we can apply allocation >> of GFP_NOIO automatically in resume path now, and you can always >> use GFP_KERNEL safely. Considered that it is a API,

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-11 Thread Bjørn Mork
Oliver Neukum writes: > On Thursday 11 April 2013 10:31:31 Ming Lei wrote: > >> 'mem_flags' isn't needed any more since we can apply allocation >> of GFP_NOIO automatically in resume path now, and you can always >> use GFP_KERNEL safely. Considered that it is a API, please don't >> introduce it.

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-10 Thread Oliver Neukum
On Thursday 11 April 2013 10:31:31 Ming Lei wrote: > 'mem_flags' isn't needed any more since we can apply allocation > of GFP_NOIO automatically in resume path now, and you can always > use GFP_KERNEL safely. Considered that it is a API, please don't > introduce it. The automatic system goes a l

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-10 Thread Ming Lei
On Thu, Apr 11, 2013 at 4:30 AM, Dan Williams wrote: > Some drivers (sierra_net) need the status interrupt URB > active even when the device is closed, because they receive > custom indications from firmware. Add functions to refcount > the status interrupt URB submit/kill operation so that > sub

Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-10 Thread Oliver Neukum
On Wednesday 10 April 2013 15:30:50 Dan Williams wrote: > Some drivers (sierra_net) need the status interrupt URB > active even when the device is closed, because they receive > custom indications from firmware. Add functions to refcount > the status interrupt URB submit/kill operation so that > s

[PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

2013-04-10 Thread Dan Williams
Some drivers (sierra_net) need the status interrupt URB active even when the device is closed, because they receive custom indications from firmware. Add functions to refcount the status interrupt URB submit/kill operation so that sub-drivers and the generic driver don't fight over whether the sta