| Message ID | 20191010134849.9078-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Honour O_NONBLOCK before throttling execbuf submissions | expand |
Quoting Chris Wilson (2019-10-10 14:48:49) > Check the user's flags on the struct file before deciding whether or not > to stall before submitting a request. This allows us to reasonably > cheaply honour O_NONBLOCK without checking at more critical phases > during request submission. One might reasonably expect poll(POLLOUT) to be supported as well in this case :| Bring on ugpu. -Chris
On 10/10/2019 14:58, Chris Wilson wrote: > Quoting Chris Wilson (2019-10-10 14:48:49) >> Check the user's flags on the struct file before deciding whether or not >> to stall before submitting a request. This allows us to reasonably >> cheaply honour O_NONBLOCK without checking at more critical phases >> during request submission. > > One might reasonably expect poll(POLLOUT) to be supported as well in > this case :| That doesn't kind of fit - mismatch between one fd and multiple contexts, no? Or you could signal POLLOUT on any, or on all have space. But that's taking it too far. :) Regards, Tvrtko > Bring on ugpu. > -Chris > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On 10/10/2019 14:48, Chris Wilson wrote: > Check the user's flags on the struct file before deciding whether or not > to stall before submitting a request. This allows us to reasonably > cheaply honour O_NONBLOCK without checking at more critical phases > during request submission. > > Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 21 ++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > index 98816c35ffc3..bc6bcb8f6d79 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -2189,15 +2189,22 @@ static int __eb_pin_engine(struct i915_execbuffer *eb, struct intel_context *ce) > intel_context_timeline_unlock(tl); > > if (rq) { > - if (i915_request_wait(rq, > - I915_WAIT_INTERRUPTIBLE, > - MAX_SCHEDULE_TIMEOUT) < 0) { > - i915_request_put(rq); > - err = -EINTR; > - goto err_exit; > - } > + bool nonblock = eb->file->filp->f_flags & O_NONBLOCK; > + long timeout; > + > + timeout = MAX_SCHEDULE_TIMEOUT; > + if (nonblock) > + timeout = 0; > > + timeout = i915_request_wait(rq, > + I915_WAIT_INTERRUPTIBLE, > + timeout); > i915_request_put(rq); > + > + if (timeout < 0) { > + err = nonblock ? -EWOULDBLOCK : timeout; > + goto err_exit; > + } > } > > eb->engine = ce->engine; > Why not, if there is userspace! :) Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
Quoting Tvrtko Ursulin (2019-10-11 09:20:12) > > On 10/10/2019 14:58, Chris Wilson wrote: > > Quoting Chris Wilson (2019-10-10 14:48:49) > >> Check the user's flags on the struct file before deciding whether or not > >> to stall before submitting a request. This allows us to reasonably > >> cheaply honour O_NONBLOCK without checking at more critical phases > >> during request submission. > > > > One might reasonably expect poll(POLLOUT) to be supported as well in > > this case :| > > That doesn't kind of fit - mismatch between one fd and multiple > contexts, no? Or you could signal POLLOUT on any, or on all have space. > But that's taking it too far. :) Aye, that's what I was thinking of with the ugpu comment, where one fd is one user submit queue. -Chris
Quoting Chris Wilson (2019-10-10 14:48:49) > Check the user's flags on the struct file before deciding whether or not > to stall before submitting a request. This allows us to reasonably > cheaply honour O_NONBLOCK without checking at more critical phases > during request submission. > > Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 21 ++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > index 98816c35ffc3..bc6bcb8f6d79 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -2189,15 +2189,22 @@ static int __eb_pin_engine(struct i915_execbuffer *eb, struct intel_context *ce) > intel_context_timeline_unlock(tl); > > if (rq) { > - if (i915_request_wait(rq, > - I915_WAIT_INTERRUPTIBLE, > - MAX_SCHEDULE_TIMEOUT) < 0) { > - i915_request_put(rq); > - err = -EINTR; > - goto err_exit; > - } > + bool nonblock = eb->file->filp->f_flags & O_NONBLOCK; > + long timeout; The alternative or addendum would be to use an execbuf.flag to opt out of throttling. O_NONBLOCK seems fitting though. -Chris
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 98816c35ffc3..bc6bcb8f6d79 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -2189,15 +2189,22 @@ static int __eb_pin_engine(struct i915_execbuffer *eb, struct intel_context *ce) intel_context_timeline_unlock(tl); if (rq) { - if (i915_request_wait(rq, - I915_WAIT_INTERRUPTIBLE, - MAX_SCHEDULE_TIMEOUT) < 0) { - i915_request_put(rq); - err = -EINTR; - goto err_exit; - } + bool nonblock = eb->file->filp->f_flags & O_NONBLOCK; + long timeout; + + timeout = MAX_SCHEDULE_TIMEOUT; + if (nonblock) + timeout = 0; + timeout = i915_request_wait(rq, + I915_WAIT_INTERRUPTIBLE, + timeout); i915_request_put(rq); + + if (timeout < 0) { + err = nonblock ? -EWOULDBLOCK : timeout; + goto err_exit; + } } eb->engine = ce->engine;
Check the user's flags on the struct file before deciding whether or not to stall before submitting a request. This allows us to reasonably cheaply honour O_NONBLOCK without checking at more critical phases during request submission. Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)