lkml.org 
[lkml]   [2016]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 6/9] net: ethernet: ti: cpts: clean up event list if event pool is empty
From
Date
On 09/14/2016 05:14 PM, Richard Cochran wrote:
> On Wed, Sep 14, 2016 at 04:02:28PM +0300, Grygorii Strashko wrote:
>> From: WingMan Kwok <w-kwok2@ti.com>
>>
>> When a CPTS user does not exit gracefully by disabling cpts
>> timestamping and leaving a joined multicast group, the system
>> continues to receive and timestamps the ptp packets which eventually
>> occupy all the event list entries. When this happns, the added code
>> tries to remove some list entries which are expired.
>>
>> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>> drivers/net/ethernet/ti/cpts.c | 30 ++++++++++++++++++++++++++++--
>> 1 file changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
>> index 970d4e2..ff8bb85 100644
>> --- a/drivers/net/ethernet/ti/cpts.c
>> +++ b/drivers/net/ethernet/ti/cpts.c
>> @@ -57,22 +57,48 @@ static int cpts_fifo_pop(struct cpts *cpts, u32 *high, u32 *low)
>> return -1;
>> }
>>
>> +static int cpts_event_list_clean_up(struct cpts *cpts)
>
> 5 words, that is quite a mouth full. How about this instead?
>
> static int cpts_purge_events(struct cpts *cpts);
>
>> +{
>> + struct list_head *this, *next;
>> + struct cpts_event *event;
>> + int removed = 0;
>> +
>> + list_for_each_safe(this, next, &cpts->events) {
>> + event = list_entry(this, struct cpts_event, list);
>> + if (event_expired(event)) {
>> + list_del_init(&event->list);
>> + list_add(&event->list, &cpts->pool);
>> + ++removed;
>> + }
>> + }
>> + return removed;
>> +}
>> +
>> /*
>> * Returns zero if matching event type was found.
>> */
>> static int cpts_fifo_read(struct cpts *cpts, int match)
>> {
>> int i, type = -1;
>> + int removed;
>
> No need for another variable, just change the return code above to
>
> return removed ? 0 : -1;
>
> and then you have ...
>
>> u32 hi, lo;
>> struct cpts_event *event;
>>
>> for (i = 0; i < CPTS_FIFO_DEPTH; i++) {
>> if (cpts_fifo_pop(cpts, &hi, &lo))
>> break;
>> +
>> if (list_empty(&cpts->pool)) {
>> - pr_err("cpts: event pool is empty\n");
>> - return -1;
>> + removed = cpts_event_list_clean_up(cpts);
>> + if (!removed) {
>> + dev_err(cpts->dev,
>> + "cpts: event pool is empty\n");
>> + return -1;
>> + }
>
> if (cpts_purge_events(cpts)) {
> dev_err(cpts->dev, "cpts: event pool empty\n");
> return -1;
> }
>
> Notice how I avoided the ugly line break?
>
>> + dev_dbg(cpts->dev,
>> + "cpts: event pool cleaned up %d\n", removed);
>> }
>> +
>> event = list_first_entry(&cpts->pool, struct cpts_event, list);
>> event->tmo = jiffies + 2;
>> event->high = hi;
>> --
>> 2.9.3
>>
>

NP here - will update. Thanks for you review.


--
regards,
-grygorii

\
 
 \ /
  Last update: 2016-09-17 09:59    [W:0.050 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site