| Message ID | 1449839521-21958-12-git-send-email-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/12/15 13:11, John.C.Harrison@Intel.com wrote: > From: Peter Lawthers <peter.lawthers@intel.com> > > In the 3.14 kernel, a signaled fence was indicated by the status field > == 1. In 4.x, a status == 0 indicates signaled, status < 0 indicates error, > and status > 0 indicates active. > > This patch wraps the check for a signaled fence in a function so that > callers no longer needs to know the underlying implementation. > > v3: New patch for series. > > Change-Id: I8e565e49683e3efeb9474656cd84cf4add6ad6a2 > Tracked-On: https://jira01.devtools.intel.com/browse/ACD-308 > Signed-off-by: Peter Lawthers <peter.lawthers@intel.com> > --- > drivers/android/sync.h | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/drivers/android/sync.h b/drivers/android/sync.h > index d57fa0a..75532d8 100644 > --- a/drivers/android/sync.h > +++ b/drivers/android/sync.h > @@ -345,6 +345,27 @@ int sync_fence_cancel_async(struct sync_fence *fence, > */ > int sync_fence_wait(struct sync_fence *fence, long timeout); > > +/** > + * sync_fence_is_signaled() - Return an indication if the fence is signaled > + * @fence: fence to check > + * > + * returns 1 if fence is signaled > + * returns 0 if fence is not signaled > + * returns < 0 if fence is in error state > + */ > +static inline int > +sync_fence_is_signaled(struct sync_fence *fence) > +{ > + int status; > + > + status = atomic_read(&fence->status); > + if (status == 0) > + return 1; > + if (status > 0) > + return 0; > + return status; > +} Not so important but could simply return bool, like "return status <= 0"? Since it is called "is_signaled" and it is only used in boolean mode in future patches. Regards, Tvrtko
On 11/12/2015 15:57, Tvrtko Ursulin wrote: > > On 11/12/15 13:11, John.C.Harrison@Intel.com wrote: >> From: Peter Lawthers <peter.lawthers@intel.com> >> >> In the 3.14 kernel, a signaled fence was indicated by the status field >> == 1. In 4.x, a status == 0 indicates signaled, status < 0 indicates >> error, >> and status > 0 indicates active. >> >> This patch wraps the check for a signaled fence in a function so that >> callers no longer needs to know the underlying implementation. >> >> v3: New patch for series. >> >> Change-Id: I8e565e49683e3efeb9474656cd84cf4add6ad6a2 >> Tracked-On: https://jira01.devtools.intel.com/browse/ACD-308 >> Signed-off-by: Peter Lawthers <peter.lawthers@intel.com> >> --- >> drivers/android/sync.h | 21 +++++++++++++++++++++ >> 1 file changed, 21 insertions(+) >> >> diff --git a/drivers/android/sync.h b/drivers/android/sync.h >> index d57fa0a..75532d8 100644 >> --- a/drivers/android/sync.h >> +++ b/drivers/android/sync.h >> @@ -345,6 +345,27 @@ int sync_fence_cancel_async(struct sync_fence >> *fence, >> */ >> int sync_fence_wait(struct sync_fence *fence, long timeout); >> >> +/** >> + * sync_fence_is_signaled() - Return an indication if the fence is >> signaled >> + * @fence: fence to check >> + * >> + * returns 1 if fence is signaled >> + * returns 0 if fence is not signaled >> + * returns < 0 if fence is in error state >> + */ >> +static inline int >> +sync_fence_is_signaled(struct sync_fence *fence) >> +{ >> + int status; >> + >> + status = atomic_read(&fence->status); >> + if (status == 0) >> + return 1; >> + if (status > 0) >> + return 0; >> + return status; >> +} > > Not so important but could simply return bool, like "return status <= > 0"? Since it is called "is_signaled" and it is only used in boolean > mode in future patches. There is no point in throwing away the error code unnecessarily. It can be useful in debug output and indeed will show up the scheduler status dump via debugfs. > > Regards, > > Tvrtko
On 14/12/15 11:22, John Harrison wrote: > On 11/12/2015 15:57, Tvrtko Ursulin wrote: >> >> On 11/12/15 13:11, John.C.Harrison@Intel.com wrote: >>> From: Peter Lawthers <peter.lawthers@intel.com> >>> >>> In the 3.14 kernel, a signaled fence was indicated by the status field >>> == 1. In 4.x, a status == 0 indicates signaled, status < 0 indicates >>> error, >>> and status > 0 indicates active. >>> >>> This patch wraps the check for a signaled fence in a function so that >>> callers no longer needs to know the underlying implementation. >>> >>> v3: New patch for series. >>> >>> Change-Id: I8e565e49683e3efeb9474656cd84cf4add6ad6a2 >>> Tracked-On: https://jira01.devtools.intel.com/browse/ACD-308 >>> Signed-off-by: Peter Lawthers <peter.lawthers@intel.com> >>> --- >>> drivers/android/sync.h | 21 +++++++++++++++++++++ >>> 1 file changed, 21 insertions(+) >>> >>> diff --git a/drivers/android/sync.h b/drivers/android/sync.h >>> index d57fa0a..75532d8 100644 >>> --- a/drivers/android/sync.h >>> +++ b/drivers/android/sync.h >>> @@ -345,6 +345,27 @@ int sync_fence_cancel_async(struct sync_fence >>> *fence, >>> */ >>> int sync_fence_wait(struct sync_fence *fence, long timeout); >>> >>> +/** >>> + * sync_fence_is_signaled() - Return an indication if the fence is >>> signaled >>> + * @fence: fence to check >>> + * >>> + * returns 1 if fence is signaled >>> + * returns 0 if fence is not signaled >>> + * returns < 0 if fence is in error state >>> + */ >>> +static inline int >>> +sync_fence_is_signaled(struct sync_fence *fence) >>> +{ >>> + int status; >>> + >>> + status = atomic_read(&fence->status); >>> + if (status == 0) >>> + return 1; >>> + if (status > 0) >>> + return 0; >>> + return status; >>> +} >> >> Not so important but could simply return bool, like "return status <= >> 0"? Since it is called "is_signaled" and it is only used in boolean >> mode in future patches. > > There is no point in throwing away the error code unnecessarily. It can > be useful in debug output and indeed will show up the scheduler status > dump via debugfs. You could still grab it directly from that call site. Or by adding another accessor like sync_fence_get_error(). Just saying that it may be good to decouple more from sync_fence implementation, since the internals have changed once already. Regards, Tvrtko
diff --git a/drivers/android/sync.h b/drivers/android/sync.h index d57fa0a..75532d8 100644 --- a/drivers/android/sync.h +++ b/drivers/android/sync.h @@ -345,6 +345,27 @@ int sync_fence_cancel_async(struct sync_fence *fence, */ int sync_fence_wait(struct sync_fence *fence, long timeout); +/** + * sync_fence_is_signaled() - Return an indication if the fence is signaled + * @fence: fence to check + * + * returns 1 if fence is signaled + * returns 0 if fence is not signaled + * returns < 0 if fence is in error state + */ +static inline int +sync_fence_is_signaled(struct sync_fence *fence) +{ + int status; + + status = atomic_read(&fence->status); + if (status == 0) + return 1; + if (status > 0) + return 0; + return status; +} + #ifdef CONFIG_DEBUG_FS void sync_timeline_debug_add(struct sync_timeline *obj);