[PATCH 05/10] IB/core: Remove checks for negative values in create_flow uverbs

2013-10-09 Thread Yann Droneaud
Fields in flow_attr structure are unsigned, so it's not necessary to check for negative values. Signed-off-by: Yann Droneaud Link: http://marc.info/?i=cover.1381351016.git.ydrone...@opteya.com Link: http://mid.gmane.org/cover.1381351016.git.ydrone...@opteya.com --- drivers/infiniband/core/uverbs

[PATCH 07/10] IB/core: Uses a common header for uverbs flow_specs

2013-10-09 Thread Yann Droneaud
A common header allows better checking of flow specs size, while ensuring strict alignment to 64bits. Signed-off-by: Yann Droneaud Link: http://marc.info/?i=cover.1381351016.git.ydrone...@opteya.com Link: http://mid.gmane.org/cover.1381351016.git.ydrone...@opteya.com --- drivers/infiniband/core/

[PATCH 10/10] IB/core: Add proper bound checking in flow_spec uverbs conversion to verbs

2013-10-09 Thread Yann Droneaud
Relying on the size sent by userspace as the size of kernel internal data structure is not safe. This patch add some level of protections to - not read past uverbs command in case of truncated uverbs flow spec - not write past flow_attr in case of changes in kernel flow spec data structure forma

[PATCH 03/10] IB/core: Don't include command size in uverbs create_flow

2013-10-09 Thread Yann Droneaud
This patch changes the semantic of the size field of create_flow command to be the size of the flow_spec's list instead of the total size of the command, counting command header, command and flow_spec's. Flow attributes must be independent of command header: they are part of different layers. Sig

[PATCH 04/10] IB/core: Allocate memory only for flow_spec in create_flow uverbs

2013-10-09 Thread Yann Droneaud
Instead of allocating memory for the flow attribute structure and all the flow specs, this patch makes use of the flow attribute structure already read as part of the command, allocating memory only for the flow_specs. Signed-off-by: Yann Droneaud Link: http://marc.info/?i=cover.1381351016.git.yd

[PATCH 02/10] IB/core: Makes uverbs flow structure using names more similar to verbs ones

2013-10-09 Thread Yann Droneaud
This patch add "flow" prefix to most of data structure added as part of commit 436f2ad to keep those names in sync with the data structures added in commit 319a441. It's just a matter of translating 'ib_flow' to 'ib_uverbs_flow'. Signed-off-by: Yann Droneaud Link: http://marc.info/?i=cover.13813

[PATCH 09/10] IB/core: Remove ib_uverbs_flow_spec structure from userspace

2013-10-09 Thread Yann Droneaud
The structure holding any types of flow_spec is of no use to userspace. It would be wrong for userspace to do: struct ib_uverbs_flow_spec flow_spec; flow_spec.type = IB_FLOW_SPEC_TCP; flow_spec.size = sizeof(flow_spec); Instead, userspace should use the dedicated flow_spec structure for

[PATCH 08/10] IB/core: Use flow_spec common header in uverbs flow_spec conversion function

2013-10-09 Thread Yann Droneaud
While the validity (and type) of the flow_spec is not known before calling the conversion function, it's make more sense to use a pointer to the header structure. Once the header parameters are validated, the flow_spec can be processed. Signed-off-by: Yann Droneaud Link: http://marc.info/?i=cove

[PATCH 00/10] IB/core: more fixes for create_flow uverbs (for v3.12)

2013-10-09 Thread Yann Droneaud
Hi, I've suggested reverting temporarily commit 22878db, 436f2ad, 400dbc9 for v3.12 in a previous message [1]. This proposition doesn't sound acceptable for Or. So I continue to improve them to be in a better shape for v3.12. Please find some important changes against create_flow uverbs implemen

[PATCH 01/10] IB/core: Rename 'flow' structs to match other uverbs structs

2013-10-09 Thread Yann Droneaud
Commit 436f2ad added public data structures to support receive flow steering. The new structs are not following the 'uverbs' pattern: they're lacking the common prefix 'ib_uverbs'. This patch replaces ib_kern prefix by ib_uverbs. Signed-off-by: Yann Droneaud Link: http://marc.info/?i=cover.13813

[PATCH 06/10] IB/core: Check against uverbs structure size while parsing flow_spec

2013-10-09 Thread Yann Droneaud
Instead of checking input size against the kernel verbs data structure, it's safer to check input size against uverbs data structure. Signed-off-by: Yann Droneaud Link: http://marc.info/?i=cover.1381351016.git.ydrone...@opteya.com Link: http://mid.gmane.org/cover.1381351016.git.ydrone...@opteya.c

Re: OpenSM 3.3.16 at 100% CPU load, "console off"

2013-10-09 Thread Hal Rosenstock
On 10/9/2013 11:52 AM, Sebastian Riemer wrote: > On 09.10.2013 17:15, Hal Rosenstock wrote: >> What does service restart do in terms of OpenSM ? >> >> Note that the console parameter is _not_ changeable "on the fly" right >> now so if OpenSM is being SIGHUP'd by service restart then this is a >> cu

Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern

2013-10-09 Thread Tejun Heo
Hello, On Tue, Oct 08, 2013 at 11:07:16AM +0200, Alexander Gordeev wrote: > Multipe MSIs is just a handful of drivers, really. MSI-X impact still Yes, so it's pretty nice to try out things there before going full-on. > will be huge. But if we opt a different name for the new pci_enable_msix() >

Re: [PATCH RFC 07/77] PCI/MSI: Re-design MSI/MSI-X interrupts enablement pattern

2013-10-09 Thread Tejun Heo
Hello, Alexander. On Tue, Oct 08, 2013 at 09:48:26AM +0200, Alexander Gordeev wrote: > > If there are many which duplicate the above pattern, it'd probably be > > worthwhile to provide a helper? It's usually a good idea to reduce > > the amount of boilerplate code in drivers. > > I wanted to lim

Re: OpenSM 3.3.16 at 100% CPU load, "console off"

2013-10-09 Thread Sebastian Riemer
On 09.10.2013 17:15, Hal Rosenstock wrote: > What does service restart do in terms of OpenSM ? > > Note that the console parameter is _not_ changeable "on the fly" right > now so if OpenSM is being SIGHUP'd by service restart then this is a > current limitation (and is clearly not detected/protect

Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern

2013-10-09 Thread Tejun Heo
On Mon, Oct 07, 2013 at 09:48:01PM +0100, Ben Hutchings wrote: > > There is one major flaw in min-max approach - the generic MSI layer > > will have to take decisions on exact number of MSIs to request, not > > device drivers. > [... > > No, the min-max functions should be implemented using the sa

Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern

2013-10-09 Thread Tejun Heo
Hello, On Wed, Oct 09, 2013 at 02:57:16PM +0200, Alexander Gordeev wrote: > On Mon, Oct 07, 2013 at 02:01:11PM -0400, Tejun Heo wrote: > > Hmmm... yean, the race condition could be an issue as multiple msi > > allocation might fail even if the driver can and explicitly handle > > multiple allocati

Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern

2013-10-09 Thread Tejun Heo
Hello, On Tue, Oct 08, 2013 at 02:22:16PM +0200, Alexander Gordeev wrote: > If we talk about pSeries quota, then the current pSeries pci_enable_msix() > implementation is racy internally and could fail if the quota went down > *while* pci_enable_msix() is executing. In this case the loop will have

Re: OpenSM 3.3.16 at 100% CPU load, "console off"

2013-10-09 Thread Hal Rosenstock
On 10/9/2013 10:45 AM, Sebastian Riemer wrote: > On 09.10.2013 16:00, Hal Rosenstock wrote: > >> Do you recall the sequence to get to this ? >> >> Was console option changed to off and then OpenSM SIGHUP'd ? Something >> else ? >> >> Is this reproducible ? > > Yes, now I can reproduce it. The ope

Re: OpenSM 3.3.16 at 100% CPU load, "console off"

2013-10-09 Thread Sebastian Riemer
On 09.10.2013 16:00, Hal Rosenstock wrote: > Do you recall the sequence to get to this ? > > Was console option changed to off and then OpenSM SIGHUP'd ? Something > else ? > > Is this reproducible ? Yes, now I can reproduce it. The opensm has been initially started with "console off" and I act

Re: OpenSM 3.3.16 at 100% CPU load, "console off"

2013-10-09 Thread Hal Rosenstock
Hi Sebastian, On 10/9/2013 9:28 AM, Hal Rosenstock wrote: > Hi Sebastian, > > On 10/9/2013 7:10 AM, Sebastian Riemer wrote: >> Hi Hal, >> >> we've encountered an issue with OpenSM 3.3.16 and the config option >> "console off". >> OpenSM processes are at 100% CPU load. >> >> >From strace: >> poll(

Re: OpenSM 3.3.16 at 100% CPU load, "console off"

2013-10-09 Thread Sebastian Riemer
On 09.10.2013 15:30, David Dillow wrote: > On Wed, 2013-10-09 at 09:28 -0400, Hal Rosenstock wrote: >>> >From strace: >>> poll([{fd=0, events=POLLIN}], 1, 1000) = 1 ([{fd=0, revents=POLLIN}]) >>> read(0, "", 4096) = 0 >>> poll([{fd=0, events=POLLIN}], 1, 1000) = 1 ([{fd=0, r

Re: OpenSM 3.3.16 at 100% CPU load, "console off"

2013-10-09 Thread David Dillow
On Wed, 2013-10-09 at 09:28 -0400, Hal Rosenstock wrote: > >>From strace: > > poll([{fd=0, events=POLLIN}], 1, 1000) = 1 ([{fd=0, revents=POLLIN}]) > > read(0, "", 4096) = 0 > > poll([{fd=0, events=POLLIN}], 1, 1000) = 1 ([{fd=0, revents=POLLIN}]) > > read(0, "", 4096)

Re: OpenSM 3.3.16 at 100% CPU load, "console off"

2013-10-09 Thread Hal Rosenstock
Hi Sebastian, On 10/9/2013 7:10 AM, Sebastian Riemer wrote: > Hi Hal, > > we've encountered an issue with OpenSM 3.3.16 and the config option > "console off". > OpenSM processes are at 100% CPU load. > >>From strace: > poll([{fd=0, events=POLLIN}], 1, 1000) = 1 ([{fd=0, revents=POLLIN}]) > read

Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern

2013-10-09 Thread Alexander Gordeev
On Mon, Oct 07, 2013 at 02:01:11PM -0400, Tejun Heo wrote: > Hmmm... yean, the race condition could be an issue as multiple msi > allocation might fail even if the driver can and explicitly handle > multiple allocation if the quota gets reduced inbetween. BTW, should we care about the quota gettin

Re: [PATCH v2 1/4] IB/core: extended command: an improved infrastructure for uverbs commands

2013-10-09 Thread Yann Droneaud
Le 09.10.2013 13:38, Or Gerlitz a écrit : On 07/10/2013 23:49, Yann Droneaud wrote: Commit 400dbc9 added an infrastructure for extensible uverbs commands while later commit 436f2ad exported ib_create_flow()/ib_destroy_flow() functions using this new infrastructure. According to the commit 400

Re: [PATCH v2 1/4] IB/core: extended command: an improved infrastructure for uverbs commands

2013-10-09 Thread Or Gerlitz
On 07/10/2013 23:49, Yann Droneaud wrote: Commit 400dbc9 added an infrastructure for extensible uverbs commands while later commit 436f2ad exported ib_create_flow()/ib_destroy_flow() functions using this new infrastructure. According to the commit 400dbc9, the purpose of this infrastructure is t

Re: [PATCH v2 4/4] IB/core: extended command: add a common extended response header

2013-10-09 Thread Or Gerlitz
On 09/10/2013 14:29, Yann Droneaud wrote: The other solution is reverting the current patches (22878db, 436f2ad, 400dbc9) and continue to work on this subject for 3.13: I've proposed a slightly different scheme, but it share the same spirit. In the same time I feel like we haven't find agreemen

Re: [PATCH v2 4/4] IB/core: extended command: add a common extended response header

2013-10-09 Thread Yann Droneaud
Hi, Le 09.10.2013 12:30, Or Gerlitz a écrit : On 08/10/2013 14:58, Matan Barak wrote: Regarding the last patch, you are right that it simplifies things for creating new uverbs where command parts are in-lined one after another, but the infrastructure got a bit more complex. If we're going to

OpenSM 3.3.16 at 100% CPU load, "console off"

2013-10-09 Thread Sebastian Riemer
Hi Hal, we've encountered an issue with OpenSM 3.3.16 and the config option "console off". OpenSM processes are at 100% CPU load. >From strace: poll([{fd=0, events=POLLIN}], 1, 1000) = 1 ([{fd=0, revents=POLLIN}]) read(0, "", 4096) = 0 poll([{fd=0, events=POLLIN}], 1, 1000)

Re: [PATCH v2 4/4] IB/core: extended command: add a common extended response header

2013-10-09 Thread Or Gerlitz
On 08/10/2013 14:58, Matan Barak wrote: Regarding the last patch, you are right that it simplifies things for creating new uverbs where command parts are in-lined one after another, but the infrastructure got a bit more complex. If we're going to this direction, I think the we should also deal