diff mbox

[API-NEXT,PATCHv9,01/13] api: schedule: additional scheduler group functions

Message ID 1438948036-31868-2-git-send-email-bill.fischofer@linaro.org
State New
Headers show

Commit Message

Bill Fischofer Aug. 7, 2015, 11:47 a.m. UTC
Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
---
 include/odp/api/schedule.h | 63 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 59 insertions(+), 4 deletions(-)

Comments

Mike Holmes Aug. 7, 2015, 12:36 p.m. UTC | #1
Thinking about how this will look in the git log history all a reader will
know is that some thing was added but not what, for example this patch
looks like this

commit d91078b931352fcbffdb8c69064982b64cdc1208
Author: Bill Fischofer <bill.fischofer@linaro.org>
Date:   Fri Aug 7 06:47:04 2015 -0500

    api: schedule: additional scheduler group functions

    Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>



If the body of the commit listed the names of what is added the git history
would be much more valuable and would read thus:


commit 50bdcb7cc71223ab3372d5d35108b661fea00410
Author: Bill Fischofer <bill.fischofer@linaro.org>
Date:   Fri Aug 7 06:47:04 2015 -0500

    api: schedule: additional scheduler group functions

    Added four new APIs

    odp_schedule_group_t odp_schedule_group_lookup(const char *name);
    int odp_schedule_group_join(odp_schedule_group_t group, const
odp_thrmask_t *mask);
    int odp_schedule_group_leave(odp_schedule_group_t group, const
odp_thrmask_t *mask);
    int odp_schedule_group_count(odp_schedule_group_t group);

    Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>


When multiplied by 13 commits the git log becomes a valuable summary.
Scrolling back in our log I think we are often not descriptive enough.




On 7 August 2015 at 07:47, Bill Fischofer <bill.fischofer@linaro.org> wrote:

> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
> ---
>  include/odp/api/schedule.h | 63
> +++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/include/odp/api/schedule.h b/include/odp/api/schedule.h
> index 36e52cd..bd858b3 100644
> --- a/include/odp/api/schedule.h
> +++ b/include/odp/api/schedule.h
> @@ -187,12 +187,12 @@ int odp_schedule_num_prio(void);
>  /**
>   * Schedule group create
>   *
> - * Creates a schedule group with the thread mask. Only threads in the
> + * Create a schedule group with the specified thread mask. Only threads
> in the
>   * mask will receive events from a queue that belongs to the schedule
> group.
>   * Thread masks of various schedule groups may overlap. There are
> predefined
>   * groups such as ODP_SCHED_GROUP_ALL and ODP_SCHED_GROUP_WORKER, which
> are
> - * always present and automatically updated. Group name is optional
> - * (may be NULL) and can have ODP_SCHED_GROUP_NAME_LEN characters in
> maximum.
> + * always present and automatically updated. Group name is optional (may
> be
> + * NULL) and can contain a maximum of ODP_SCHED_GROUP_NAME_LEN characters.
>   *
>   * @param name    Schedule group name
>   * @param mask    Thread mask
> @@ -208,7 +208,7 @@ odp_schedule_group_t odp_schedule_group_create(const
> char *name,
>  /**
>   * Schedule group destroy
>   *
> - * Destroys a schedule group. All queues belonging to the schedule group
> must
> + * Destroy a schedule group. All queues belonging to the schedule group
> must
>   * be destroyed before destroying the group. Other operations on this
> group
>   * must not be invoked in parallel.
>   *
> @@ -220,6 +220,61 @@ odp_schedule_group_t odp_schedule_group_create(const
> char *name,
>  int odp_schedule_group_destroy(odp_schedule_group_t group);
>
>  /**
> + * Look up a schedule group by name
> + *
> + * Return the handle of a schedule group from its name
> + *
> + * @param name   Name of schedule group
> + *
> + * @return Handle of schedule group for specified name
> + * @retval ODP_SCHEDULE_GROUP_INVALID No matching schedule group found
> + */
> +odp_schedule_group_t odp_schedule_group_lookup(const char *name);
> +
> +/**
> + * Join a schedule group
> + *
> + * Join a threadmask to an existing schedule group
> + *
> + * @param group  Schdule group handle
> + * @param mask   Thread mask
> + *
> + * @retval 0 on success
> + * @retval <0 on failure
>
+ */
> +int odp_schedule_group_join(odp_schedule_group_t group,
> +                           const odp_thrmask_t *mask);
> +
> +/**
> + * Leave a schedule group
> + *
> + * Remove a threadmask from an existing schedule group
> + *
> + * @param group  Schedule group handle
> + * @param mask   Thread mask
> + *
> + * @retval 0 on success
> + * @retval <0 on failure
> + *
> + * @note Leaving a schedule group means threads in the specified mask
> will no
> + * longer receive events from queues belonging to the specified schedule
> + * group. This effect is not instantaneous, however, and events that have
> been
> + * prestaged may still be presented to the masked threads.
> + */
> +int odp_schedule_group_leave(odp_schedule_group_t group,
> +                            const odp_thrmask_t *mask);
> +
> +/**
> + * Count members in a schedule group
> + *
> + * @param group  Schedule group handle
> + *
> + * @return Count of threads currently members of this schedule group.
> + * @retval <0 Invalid group specified
> + */
> +int odp_schedule_group_count(odp_schedule_group_t group);
> +
> +/**
>   * Initialize ordered context lock
>   *
>   * Initialize an ordered queue context lock. The lock can be associated
> only
> --
> 2.1.4
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
Ivan Khoronzhuk Aug. 7, 2015, 6:39 p.m. UTC | #2
+

On 07.08.15 15:36, Mike Holmes wrote:
> Thinking about how this will look in the git log history all a reader will know is that some thing was added but not what, for example this patch looks like this
>
> commit d91078b931352fcbffdb8c69064982b64cdc1208
> Author: Bill Fischofer <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>>
> Date:   Fri Aug 7 06:47:04 2015 -0500
>
>      api: schedule: additional scheduler group functions
>
>      Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>>
>
>
>
> If the body of the commit listed the names of what is added the git history would be much more valuable and would read thus:
>
>
> commit 50bdcb7cc71223ab3372d5d35108b661fea00410
> Author: Bill Fischofer <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>>
> Date:   Fri Aug 7 06:47:04 2015 -0500
>
>      api: schedule: additional scheduler group functions
>
>      Added four new APIs
>
>      odp_schedule_group_t odp_schedule_group_lookup(const char *name);
>      int odp_schedule_group_join(odp_schedule_group_t group, const odp_thrmask_t *mask);
>      int odp_schedule_group_leave(odp_schedule_group_t group, const odp_thrmask_t *mask);
>      int odp_schedule_group_count(odp_schedule_group_t group);
>
>      Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>>
>
>
> When multiplied by 13 commits the git log becomes a valuable summary. Scrolling back in our log I think we are often not descriptive enough.

I don't want to be too picky, just a little word...
Maybe exactly this patch is not a place for such information, but
often short information about why the change was added is absent...
:-/

>
>
>
>
> On 7 August 2015 at 07:47, Bill Fischofer <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>> wrote:
>
>     Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>>
>     ---
>       include/odp/api/schedule.h | 63 +++++++++++++++++++++++++++++++++++++++++++---
>       1 file changed, 59 insertions(+), 4 deletions(-)
>
>     diff --git a/include/odp/api/schedule.h b/include/odp/api/schedule.h
>     index 36e52cd..bd858b3 100644
>     --- a/include/odp/api/schedule.h
>     +++ b/include/odp/api/schedule.h
>     @@ -187,12 +187,12 @@ int odp_schedule_num_prio(void);
>       /**
>        * Schedule group create
>        *
>     - * Creates a schedule group with the thread mask. Only threads in the
>     + * Create a schedule group with the specified thread mask. Only threads in the
>        * mask will receive events from a queue that belongs to the schedule group.
>        * Thread masks of various schedule groups may overlap. There are predefined
>        * groups such as ODP_SCHED_GROUP_ALL and ODP_SCHED_GROUP_WORKER, which are
>     - * always present and automatically updated. Group name is optional
>     - * (may be NULL) and can have ODP_SCHED_GROUP_NAME_LEN characters in maximum.
>     + * always present and automatically updated. Group name is optional (may be
>     + * NULL) and can contain a maximum of ODP_SCHED_GROUP_NAME_LEN characters.
>        *
>        * @param name    Schedule group name
>        * @param mask    Thread mask
>     @@ -208,7 +208,7 @@ odp_schedule_group_t odp_schedule_group_create(const char *name,
>       /**
>        * Schedule group destroy
>        *
>     - * Destroys a schedule group. All queues belonging to the schedule group must
>     + * Destroy a schedule group. All queues belonging to the schedule group must
>        * be destroyed before destroying the group. Other operations on this group
>        * must not be invoked in parallel.
>        *
>     @@ -220,6 +220,61 @@ odp_schedule_group_t odp_schedule_group_create(const char *name,
>       int odp_schedule_group_destroy(odp_schedule_group_t group);
>
>       /**
>     + * Look up a schedule group by name
>     + *
>     + * Return the handle of a schedule group from its name
>     + *
>     + * @param name   Name of schedule group
>     + *
>     + * @return Handle of schedule group for specified name
>     + * @retval ODP_SCHEDULE_GROUP_INVALID No matching schedule group found
>     + */
>     +odp_schedule_group_t odp_schedule_group_lookup(const char *name);
>     +
>     +/**
>     + * Join a schedule group
>     + *
>     + * Join a threadmask to an existing schedule group
>     + *
>     + * @param group  Schdule group handle
>     + * @param mask   Thread mask
>     + *
>     + * @retval 0 on success
>     + * @retval <0 on failure
>
>     + */
>     +int odp_schedule_group_join(odp_schedule_group_t group,
>     +                           const odp_thrmask_t *mask);
>     +
>     +/**
>     + * Leave a schedule group
>     + *
>     + * Remove a threadmask from an existing schedule group
>     + *
>     + * @param group  Schedule group handle
>     + * @param mask   Thread mask
>     + *
>     + * @retval 0 on success
>     + * @retval <0 on failure
>     + *
>     + * @note Leaving a schedule group means threads in the specified mask will no
>     + * longer receive events from queues belonging to the specified schedule
>     + * group. This effect is not instantaneous, however, and events that have been
>     + * prestaged may still be presented to the masked threads.
>     + */
>     +int odp_schedule_group_leave(odp_schedule_group_t group,
>     +                            const odp_thrmask_t *mask);
>     +
>     +/**
>     + * Count members in a schedule group
>     + *
>     + * @param group  Schedule group handle
>     + *
>     + * @return Count of threads currently members of this schedule group.
>     + * @retval <0 Invalid group specified
>     + */
>     +int odp_schedule_group_count(odp_schedule_group_t group);
>     +
>     +/**
>        * Initialize ordered context lock
>        *
>        * Initialize an ordered queue context lock. The lock can be associated only
>     --
>     2.1.4
>
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
> --
> Mike Holmes
> Technical Manager - Linaro Networking Group
> Linaro.org <http://www.linaro.org/>***│ *Open source software for ARM SoCs
>
> __
>
>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
diff mbox

Patch

diff --git a/include/odp/api/schedule.h b/include/odp/api/schedule.h
index 36e52cd..bd858b3 100644
--- a/include/odp/api/schedule.h
+++ b/include/odp/api/schedule.h
@@ -187,12 +187,12 @@  int odp_schedule_num_prio(void);
 /**
  * Schedule group create
  *
- * Creates a schedule group with the thread mask. Only threads in the
+ * Create a schedule group with the specified thread mask. Only threads in the
  * mask will receive events from a queue that belongs to the schedule group.
  * Thread masks of various schedule groups may overlap. There are predefined
  * groups such as ODP_SCHED_GROUP_ALL and ODP_SCHED_GROUP_WORKER, which are
- * always present and automatically updated. Group name is optional
- * (may be NULL) and can have ODP_SCHED_GROUP_NAME_LEN characters in maximum.
+ * always present and automatically updated. Group name is optional (may be
+ * NULL) and can contain a maximum of ODP_SCHED_GROUP_NAME_LEN characters.
  *
  * @param name    Schedule group name
  * @param mask    Thread mask
@@ -208,7 +208,7 @@  odp_schedule_group_t odp_schedule_group_create(const char *name,
 /**
  * Schedule group destroy
  *
- * Destroys a schedule group. All queues belonging to the schedule group must
+ * Destroy a schedule group. All queues belonging to the schedule group must
  * be destroyed before destroying the group. Other operations on this group
  * must not be invoked in parallel.
  *
@@ -220,6 +220,61 @@  odp_schedule_group_t odp_schedule_group_create(const char *name,
 int odp_schedule_group_destroy(odp_schedule_group_t group);
 
 /**
+ * Look up a schedule group by name
+ *
+ * Return the handle of a schedule group from its name
+ *
+ * @param name   Name of schedule group
+ *
+ * @return Handle of schedule group for specified name
+ * @retval ODP_SCHEDULE_GROUP_INVALID No matching schedule group found
+ */
+odp_schedule_group_t odp_schedule_group_lookup(const char *name);
+
+/**
+ * Join a schedule group
+ *
+ * Join a threadmask to an existing schedule group
+ *
+ * @param group  Schdule group handle
+ * @param mask   Thread mask
+ *
+ * @retval 0 on success
+ * @retval <0 on failure
+ */
+int odp_schedule_group_join(odp_schedule_group_t group,
+			    const odp_thrmask_t *mask);
+
+/**
+ * Leave a schedule group
+ *
+ * Remove a threadmask from an existing schedule group
+ *
+ * @param group  Schedule group handle
+ * @param mask   Thread mask
+ *
+ * @retval 0 on success
+ * @retval <0 on failure
+ *
+ * @note Leaving a schedule group means threads in the specified mask will no
+ * longer receive events from queues belonging to the specified schedule
+ * group. This effect is not instantaneous, however, and events that have been
+ * prestaged may still be presented to the masked threads.
+ */
+int odp_schedule_group_leave(odp_schedule_group_t group,
+			     const odp_thrmask_t *mask);
+
+/**
+ * Count members in a schedule group
+ *
+ * @param group  Schedule group handle
+ *
+ * @return Count of threads currently members of this schedule group.
+ * @retval <0 Invalid group specified
+ */
+int odp_schedule_group_count(odp_schedule_group_t group);
+
+/**
  * Initialize ordered context lock
  *
  * Initialize an ordered queue context lock. The lock can be associated only