Re: [PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Huang, Ying]

Byungchul Park  writes:

> On Mon, Feb 13, 2017 at 04:58:05PM +0900, Byungchul Park wrote:
>> On Mon, Feb 13, 2017 at 03:52:44PM +0800, Huang, Ying wrote:
>> > Byungchul Park  writes:
>> > 
>> > > On Mon, Feb 13, 2017 at 03:36:33PM +0800, Huang, Ying wrote:
>> > >> Byungchul Park  writes:
>> > >> 
>> > >> > Sometimes we have to dereference next field of llist node before 
>> > >> > entering
>> > >> > loop becasue the node might be deleted or the next field might be
>> > >> > modified within the loop. So this adds the safe version of 
>> > >> > llist_for_each,
>> > >> > that is, llist_for_each_safe.
>> > >> >
>> > >> > Signed-off-by: Byungchul Park 
>> > >> > ---
>> > >> >  include/linux/llist.h | 19 +++
>> > >> >  1 file changed, 19 insertions(+)
>> > >> >
>> > >> > diff --git a/include/linux/llist.h b/include/linux/llist.h
>> > >> > index fd4ca0b..4c508a5 100644
>> > >> > --- a/include/linux/llist.h
>> > >> > +++ b/include/linux/llist.h
>> > >> > @@ -105,6 +105,25 @@ static inline void init_llist_head(struct 
>> > >> > llist_head *list)
>> > >> >   for ((pos) = (node); pos; (pos) = (pos)->next)
>> > >> >  
>> > >> >  /**
>> > >> > + * llist_for_each_safe - iterate over some deleted entries of a 
>> > >> > lock-less list
>> > >> > + *safe against removal of list entry
>> > >> > + * @pos: the  llist_node to use as a loop cursor
>> > >> > + * @n:   another type * to use as temporary storage
>> > >> 
>> > >> s/type */ llist_node/
>> > >
>> > > Yes.
>> > >
>> > >> 
>> > >> > + * @node:the first entry of deleted list entries
>> > >> > + *
>> > >> > + * In general, some entries of the lock-less list can be traversed
>> > >> > + * safely only after being deleted from list, so start with an entry
>> > >> > + * instead of list head.
>> > >> > + *
>> > >> > + * If being used on entries deleted from lock-less list directly, the
>> > >> > + * traverse order is from the newest to the oldest added entry.  If
>> > >> > + * you want to traverse from the oldest to the newest, you must
>> > >> > + * reverse the order by yourself before traversing.
>> > >> > + */
>> > >> > +#define llist_for_each_safe(pos, n, node)\
>> > >> > + for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) 
>> > >> > = (n))
>> > >> > +
>> > >> 
>> > >> Following the style of other xxx_for_each_safe,
>> > >> 
>> > >> #define llist_for_each_safe(pos, n, node)   \
>> > >> for (pos = (node), (pos && (n = pos->next)); pos; pos = n, n = 
>> > >> pos->next)
>> > >
>> > > Do you think it should be modified? I think mine is simpler. No?
>> > 
>> > Personally I prefer the style of other xxx_for_each_safe().
>> 
>> Yes, I will modify it as you recommand.
>> 
>> Thank you very much.
>
> I wanted to modify it as you recommanded but it has a bug. It should be
> (to fix the bug):
>
>for (pos = (node), (pos && (n = pos->next)); pos; pos = n, (pos && \
>(n = pos->next)))
>
> Don't you think this is too messy? Or do I miss something? I still think
> the following is neater and simpler.
>
>for (pos = node; pos && (n = pos->next, true); pos = n)

OK.  This looks better.

Best Regards,
Huang, Ying

> Or could you recommand another preference?

Re: [PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Huang, Ying]

Byungchul Park  writes:

> On Mon, Feb 13, 2017 at 04:58:05PM +0900, Byungchul Park wrote:
>> On Mon, Feb 13, 2017 at 03:52:44PM +0800, Huang, Ying wrote:
>> > Byungchul Park  writes:
>> > 
>> > > On Mon, Feb 13, 2017 at 03:36:33PM +0800, Huang, Ying wrote:
>> > >> Byungchul Park  writes:
>> > >> 
>> > >> > Sometimes we have to dereference next field of llist node before 
>> > >> > entering
>> > >> > loop becasue the node might be deleted or the next field might be
>> > >> > modified within the loop. So this adds the safe version of 
>> > >> > llist_for_each,
>> > >> > that is, llist_for_each_safe.
>> > >> >
>> > >> > Signed-off-by: Byungchul Park 
>> > >> > ---
>> > >> >  include/linux/llist.h | 19 +++
>> > >> >  1 file changed, 19 insertions(+)
>> > >> >
>> > >> > diff --git a/include/linux/llist.h b/include/linux/llist.h
>> > >> > index fd4ca0b..4c508a5 100644
>> > >> > --- a/include/linux/llist.h
>> > >> > +++ b/include/linux/llist.h
>> > >> > @@ -105,6 +105,25 @@ static inline void init_llist_head(struct 
>> > >> > llist_head *list)
>> > >> >   for ((pos) = (node); pos; (pos) = (pos)->next)
>> > >> >  
>> > >> >  /**
>> > >> > + * llist_for_each_safe - iterate over some deleted entries of a 
>> > >> > lock-less list
>> > >> > + *safe against removal of list entry
>> > >> > + * @pos: the  llist_node to use as a loop cursor
>> > >> > + * @n:   another type * to use as temporary storage
>> > >> 
>> > >> s/type */ llist_node/
>> > >
>> > > Yes.
>> > >
>> > >> 
>> > >> > + * @node:the first entry of deleted list entries
>> > >> > + *
>> > >> > + * In general, some entries of the lock-less list can be traversed
>> > >> > + * safely only after being deleted from list, so start with an entry
>> > >> > + * instead of list head.
>> > >> > + *
>> > >> > + * If being used on entries deleted from lock-less list directly, the
>> > >> > + * traverse order is from the newest to the oldest added entry.  If
>> > >> > + * you want to traverse from the oldest to the newest, you must
>> > >> > + * reverse the order by yourself before traversing.
>> > >> > + */
>> > >> > +#define llist_for_each_safe(pos, n, node)\
>> > >> > + for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) 
>> > >> > = (n))
>> > >> > +
>> > >> 
>> > >> Following the style of other xxx_for_each_safe,
>> > >> 
>> > >> #define llist_for_each_safe(pos, n, node)   \
>> > >> for (pos = (node), (pos && (n = pos->next)); pos; pos = n, n = 
>> > >> pos->next)
>> > >
>> > > Do you think it should be modified? I think mine is simpler. No?
>> > 
>> > Personally I prefer the style of other xxx_for_each_safe().
>> 
>> Yes, I will modify it as you recommand.
>> 
>> Thank you very much.
>
> I wanted to modify it as you recommanded but it has a bug. It should be
> (to fix the bug):
>
>for (pos = (node), (pos && (n = pos->next)); pos; pos = n, (pos && \
>(n = pos->next)))
>
> Don't you think this is too messy? Or do I miss something? I still think
> the following is neater and simpler.
>
>for (pos = node; pos && (n = pos->next, true); pos = n)

OK.  This looks better.

Best Regards,
Huang, Ying

> Or could you recommand another preference?

Re: [PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Byungchul Park]

On Mon, Feb 13, 2017 at 04:58:05PM +0900, Byungchul Park wrote:
> On Mon, Feb 13, 2017 at 03:52:44PM +0800, Huang, Ying wrote:
> > Byungchul Park  writes:
> > 
> > > On Mon, Feb 13, 2017 at 03:36:33PM +0800, Huang, Ying wrote:
> > >> Byungchul Park  writes:
> > >> 
> > >> > Sometimes we have to dereference next field of llist node before 
> > >> > entering
> > >> > loop becasue the node might be deleted or the next field might be
> > >> > modified within the loop. So this adds the safe version of 
> > >> > llist_for_each,
> > >> > that is, llist_for_each_safe.
> > >> >
> > >> > Signed-off-by: Byungchul Park 
> > >> > ---
> > >> >  include/linux/llist.h | 19 +++
> > >> >  1 file changed, 19 insertions(+)
> > >> >
> > >> > diff --git a/include/linux/llist.h b/include/linux/llist.h
> > >> > index fd4ca0b..4c508a5 100644
> > >> > --- a/include/linux/llist.h
> > >> > +++ b/include/linux/llist.h
> > >> > @@ -105,6 +105,25 @@ static inline void init_llist_head(struct 
> > >> > llist_head *list)
> > >> >for ((pos) = (node); pos; (pos) = (pos)->next)
> > >> >  
> > >> >  /**
> > >> > + * llist_for_each_safe - iterate over some deleted entries of a 
> > >> > lock-less list
> > >> > + * safe against removal of list entry
> > >> > + * @pos:  the  llist_node to use as a loop cursor
> > >> > + * @n:another type * to use as temporary storage
> > >> 
> > >> s/type */ llist_node/
> > >
> > > Yes.
> > >
> > >> 
> > >> > + * @node: the first entry of deleted list entries
> > >> > + *
> > >> > + * In general, some entries of the lock-less list can be traversed
> > >> > + * safely only after being deleted from list, so start with an entry
> > >> > + * instead of list head.
> > >> > + *
> > >> > + * If being used on entries deleted from lock-less list directly, the
> > >> > + * traverse order is from the newest to the oldest added entry.  If
> > >> > + * you want to traverse from the oldest to the newest, you must
> > >> > + * reverse the order by yourself before traversing.
> > >> > + */
> > >> > +#define llist_for_each_safe(pos, n, node) \
> > >> > +  for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) 
> > >> > = (n))
> > >> > +
> > >> 
> > >> Following the style of other xxx_for_each_safe,
> > >> 
> > >> #define llist_for_each_safe(pos, n, node)\
> > >>  for (pos = (node), (pos && (n = pos->next)); pos; pos = n, n = 
> > >> pos->next)
> > >
> > > Do you think it should be modified? I think mine is simpler. No?
> > 
> > Personally I prefer the style of other xxx_for_each_safe().
> 
> Yes, I will modify it as you recommand.
> 
> Thank you very much.

I wanted to modify it as you recommanded but it has a bug. It should be
(to fix the bug):

   for (pos = (node), (pos && (n = pos->next)); pos; pos = n, (pos && \
   (n = pos->next)))

Don't you think this is too messy? Or do I miss something? I still think
the following is neater and simpler.

   for (pos = node; pos && (n = pos->next, true); pos = n)

Or could you recommand another preference?

Re: [PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Byungchul Park]

On Mon, Feb 13, 2017 at 04:58:05PM +0900, Byungchul Park wrote:
> On Mon, Feb 13, 2017 at 03:52:44PM +0800, Huang, Ying wrote:
> > Byungchul Park  writes:
> > 
> > > On Mon, Feb 13, 2017 at 03:36:33PM +0800, Huang, Ying wrote:
> > >> Byungchul Park  writes:
> > >> 
> > >> > Sometimes we have to dereference next field of llist node before 
> > >> > entering
> > >> > loop becasue the node might be deleted or the next field might be
> > >> > modified within the loop. So this adds the safe version of 
> > >> > llist_for_each,
> > >> > that is, llist_for_each_safe.
> > >> >
> > >> > Signed-off-by: Byungchul Park 
> > >> > ---
> > >> >  include/linux/llist.h | 19 +++
> > >> >  1 file changed, 19 insertions(+)
> > >> >
> > >> > diff --git a/include/linux/llist.h b/include/linux/llist.h
> > >> > index fd4ca0b..4c508a5 100644
> > >> > --- a/include/linux/llist.h
> > >> > +++ b/include/linux/llist.h
> > >> > @@ -105,6 +105,25 @@ static inline void init_llist_head(struct 
> > >> > llist_head *list)
> > >> >for ((pos) = (node); pos; (pos) = (pos)->next)
> > >> >  
> > >> >  /**
> > >> > + * llist_for_each_safe - iterate over some deleted entries of a 
> > >> > lock-less list
> > >> > + * safe against removal of list entry
> > >> > + * @pos:  the  llist_node to use as a loop cursor
> > >> > + * @n:another type * to use as temporary storage
> > >> 
> > >> s/type */ llist_node/
> > >
> > > Yes.
> > >
> > >> 
> > >> > + * @node: the first entry of deleted list entries
> > >> > + *
> > >> > + * In general, some entries of the lock-less list can be traversed
> > >> > + * safely only after being deleted from list, so start with an entry
> > >> > + * instead of list head.
> > >> > + *
> > >> > + * If being used on entries deleted from lock-less list directly, the
> > >> > + * traverse order is from the newest to the oldest added entry.  If
> > >> > + * you want to traverse from the oldest to the newest, you must
> > >> > + * reverse the order by yourself before traversing.
> > >> > + */
> > >> > +#define llist_for_each_safe(pos, n, node) \
> > >> > +  for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) 
> > >> > = (n))
> > >> > +
> > >> 
> > >> Following the style of other xxx_for_each_safe,
> > >> 
> > >> #define llist_for_each_safe(pos, n, node)\
> > >>  for (pos = (node), (pos && (n = pos->next)); pos; pos = n, n = 
> > >> pos->next)
> > >
> > > Do you think it should be modified? I think mine is simpler. No?
> > 
> > Personally I prefer the style of other xxx_for_each_safe().
> 
> Yes, I will modify it as you recommand.
> 
> Thank you very much.

I wanted to modify it as you recommanded but it has a bug. It should be
(to fix the bug):

   for (pos = (node), (pos && (n = pos->next)); pos; pos = n, (pos && \
   (n = pos->next)))

Don't you think this is too messy? Or do I miss something? I still think
the following is neater and simpler.

   for (pos = node; pos && (n = pos->next, true); pos = n)

Or could you recommand another preference?

Re: [PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Byungchul Park]

On Mon, Feb 13, 2017 at 03:52:44PM +0800, Huang, Ying wrote:
> Byungchul Park  writes:
> 
> > On Mon, Feb 13, 2017 at 03:36:33PM +0800, Huang, Ying wrote:
> >> Byungchul Park  writes:
> >> 
> >> > Sometimes we have to dereference next field of llist node before entering
> >> > loop becasue the node might be deleted or the next field might be
> >> > modified within the loop. So this adds the safe version of 
> >> > llist_for_each,
> >> > that is, llist_for_each_safe.
> >> >
> >> > Signed-off-by: Byungchul Park 
> >> > ---
> >> >  include/linux/llist.h | 19 +++
> >> >  1 file changed, 19 insertions(+)
> >> >
> >> > diff --git a/include/linux/llist.h b/include/linux/llist.h
> >> > index fd4ca0b..4c508a5 100644
> >> > --- a/include/linux/llist.h
> >> > +++ b/include/linux/llist.h
> >> > @@ -105,6 +105,25 @@ static inline void init_llist_head(struct 
> >> > llist_head *list)
> >> >  for ((pos) = (node); pos; (pos) = (pos)->next)
> >> >  
> >> >  /**
> >> > + * llist_for_each_safe - iterate over some deleted entries of a 
> >> > lock-less list
> >> > + *   safe against removal of list entry
> >> > + * @pos:the  llist_node to use as a loop cursor
> >> > + * @n:  another type * to use as temporary storage
> >> 
> >> s/type */ llist_node/
> >
> > Yes.
> >
> >> 
> >> > + * @node:   the first entry of deleted list entries
> >> > + *
> >> > + * In general, some entries of the lock-less list can be traversed
> >> > + * safely only after being deleted from list, so start with an entry
> >> > + * instead of list head.
> >> > + *
> >> > + * If being used on entries deleted from lock-less list directly, the
> >> > + * traverse order is from the newest to the oldest added entry.  If
> >> > + * you want to traverse from the oldest to the newest, you must
> >> > + * reverse the order by yourself before traversing.
> >> > + */
> >> > +#define llist_for_each_safe(pos, n, node)   \
> >> > +for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) 
> >> > = (n))
> >> > +
> >> 
> >> Following the style of other xxx_for_each_safe,
> >> 
> >> #define llist_for_each_safe(pos, n, node)  \
> >>for (pos = (node), (pos && (n = pos->next)); pos; pos = n, n = 
> >> pos->next)
> >
> > Do you think it should be modified? I think mine is simpler. No?
> 
> Personally I prefer the style of other xxx_for_each_safe().

Yes, I will modify it as you recommand.

Thank you very much.

> 
> Best Regards,
> Huang, Ying
> 
> >> 
> >> Best Regards,
> >> Huang, Ying
> >> 
> >> > +/**
> >> >   * llist_for_each_entry - iterate over some deleted entries of 
> >> > lock-less list of given type
> >> >   * @pos:the type * to use as a loop cursor.
> >> >   * @node:   the fist entry of deleted list entries.

Re: [PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Byungchul Park]

On Mon, Feb 13, 2017 at 03:52:44PM +0800, Huang, Ying wrote:
> Byungchul Park  writes:
> 
> > On Mon, Feb 13, 2017 at 03:36:33PM +0800, Huang, Ying wrote:
> >> Byungchul Park  writes:
> >> 
> >> > Sometimes we have to dereference next field of llist node before entering
> >> > loop becasue the node might be deleted or the next field might be
> >> > modified within the loop. So this adds the safe version of 
> >> > llist_for_each,
> >> > that is, llist_for_each_safe.
> >> >
> >> > Signed-off-by: Byungchul Park 
> >> > ---
> >> >  include/linux/llist.h | 19 +++
> >> >  1 file changed, 19 insertions(+)
> >> >
> >> > diff --git a/include/linux/llist.h b/include/linux/llist.h
> >> > index fd4ca0b..4c508a5 100644
> >> > --- a/include/linux/llist.h
> >> > +++ b/include/linux/llist.h
> >> > @@ -105,6 +105,25 @@ static inline void init_llist_head(struct 
> >> > llist_head *list)
> >> >  for ((pos) = (node); pos; (pos) = (pos)->next)
> >> >  
> >> >  /**
> >> > + * llist_for_each_safe - iterate over some deleted entries of a 
> >> > lock-less list
> >> > + *   safe against removal of list entry
> >> > + * @pos:the  llist_node to use as a loop cursor
> >> > + * @n:  another type * to use as temporary storage
> >> 
> >> s/type */ llist_node/
> >
> > Yes.
> >
> >> 
> >> > + * @node:   the first entry of deleted list entries
> >> > + *
> >> > + * In general, some entries of the lock-less list can be traversed
> >> > + * safely only after being deleted from list, so start with an entry
> >> > + * instead of list head.
> >> > + *
> >> > + * If being used on entries deleted from lock-less list directly, the
> >> > + * traverse order is from the newest to the oldest added entry.  If
> >> > + * you want to traverse from the oldest to the newest, you must
> >> > + * reverse the order by yourself before traversing.
> >> > + */
> >> > +#define llist_for_each_safe(pos, n, node)   \
> >> > +for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) 
> >> > = (n))
> >> > +
> >> 
> >> Following the style of other xxx_for_each_safe,
> >> 
> >> #define llist_for_each_safe(pos, n, node)  \
> >>for (pos = (node), (pos && (n = pos->next)); pos; pos = n, n = 
> >> pos->next)
> >
> > Do you think it should be modified? I think mine is simpler. No?
> 
> Personally I prefer the style of other xxx_for_each_safe().

Yes, I will modify it as you recommand.

Thank you very much.

> 
> Best Regards,
> Huang, Ying
> 
> >> 
> >> Best Regards,
> >> Huang, Ying
> >> 
> >> > +/**
> >> >   * llist_for_each_entry - iterate over some deleted entries of 
> >> > lock-less list of given type
> >> >   * @pos:the type * to use as a loop cursor.
> >> >   * @node:   the fist entry of deleted list entries.

Re: [PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Huang, Ying]

Byungchul Park  writes:

> On Mon, Feb 13, 2017 at 03:36:33PM +0800, Huang, Ying wrote:
>> Byungchul Park  writes:
>> 
>> > Sometimes we have to dereference next field of llist node before entering
>> > loop becasue the node might be deleted or the next field might be
>> > modified within the loop. So this adds the safe version of llist_for_each,
>> > that is, llist_for_each_safe.
>> >
>> > Signed-off-by: Byungchul Park 
>> > ---
>> >  include/linux/llist.h | 19 +++
>> >  1 file changed, 19 insertions(+)
>> >
>> > diff --git a/include/linux/llist.h b/include/linux/llist.h
>> > index fd4ca0b..4c508a5 100644
>> > --- a/include/linux/llist.h
>> > +++ b/include/linux/llist.h
>> > @@ -105,6 +105,25 @@ static inline void init_llist_head(struct llist_head 
>> > *list)
>> >for ((pos) = (node); pos; (pos) = (pos)->next)
>> >  
>> >  /**
>> > + * llist_for_each_safe - iterate over some deleted entries of a lock-less 
>> > list
>> > + * safe against removal of list entry
>> > + * @pos:  the  llist_node to use as a loop cursor
>> > + * @n:another type * to use as temporary storage
>> 
>> s/type */ llist_node/
>
> Yes.
>
>> 
>> > + * @node: the first entry of deleted list entries
>> > + *
>> > + * In general, some entries of the lock-less list can be traversed
>> > + * safely only after being deleted from list, so start with an entry
>> > + * instead of list head.
>> > + *
>> > + * If being used on entries deleted from lock-less list directly, the
>> > + * traverse order is from the newest to the oldest added entry.  If
>> > + * you want to traverse from the oldest to the newest, you must
>> > + * reverse the order by yourself before traversing.
>> > + */
>> > +#define llist_for_each_safe(pos, n, node) \
>> > +  for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) = (n))
>> > +
>> 
>> Following the style of other xxx_for_each_safe,
>> 
>> #define llist_for_each_safe(pos, n, node)\
>>  for (pos = (node), (pos && (n = pos->next)); pos; pos = n, n = 
>> pos->next)
>
> Do you think it should be modified? I think mine is simpler. No?

Personally I prefer the style of other xxx_for_each_safe().

Best Regards,
Huang, Ying

>> 
>> Best Regards,
>> Huang, Ying
>> 
>> > +/**
>> >   * llist_for_each_entry - iterate over some deleted entries of lock-less 
>> > list of given type
>> >   * @pos:  the type * to use as a loop cursor.
>> >   * @node: the fist entry of deleted list entries.

Re: [PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Huang, Ying]

Byungchul Park  writes:

> On Mon, Feb 13, 2017 at 03:36:33PM +0800, Huang, Ying wrote:
>> Byungchul Park  writes:
>> 
>> > Sometimes we have to dereference next field of llist node before entering
>> > loop becasue the node might be deleted or the next field might be
>> > modified within the loop. So this adds the safe version of llist_for_each,
>> > that is, llist_for_each_safe.
>> >
>> > Signed-off-by: Byungchul Park 
>> > ---
>> >  include/linux/llist.h | 19 +++
>> >  1 file changed, 19 insertions(+)
>> >
>> > diff --git a/include/linux/llist.h b/include/linux/llist.h
>> > index fd4ca0b..4c508a5 100644
>> > --- a/include/linux/llist.h
>> > +++ b/include/linux/llist.h
>> > @@ -105,6 +105,25 @@ static inline void init_llist_head(struct llist_head 
>> > *list)
>> >for ((pos) = (node); pos; (pos) = (pos)->next)
>> >  
>> >  /**
>> > + * llist_for_each_safe - iterate over some deleted entries of a lock-less 
>> > list
>> > + * safe against removal of list entry
>> > + * @pos:  the  llist_node to use as a loop cursor
>> > + * @n:another type * to use as temporary storage
>> 
>> s/type */ llist_node/
>
> Yes.
>
>> 
>> > + * @node: the first entry of deleted list entries
>> > + *
>> > + * In general, some entries of the lock-less list can be traversed
>> > + * safely only after being deleted from list, so start with an entry
>> > + * instead of list head.
>> > + *
>> > + * If being used on entries deleted from lock-less list directly, the
>> > + * traverse order is from the newest to the oldest added entry.  If
>> > + * you want to traverse from the oldest to the newest, you must
>> > + * reverse the order by yourself before traversing.
>> > + */
>> > +#define llist_for_each_safe(pos, n, node) \
>> > +  for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) = (n))
>> > +
>> 
>> Following the style of other xxx_for_each_safe,
>> 
>> #define llist_for_each_safe(pos, n, node)\
>>  for (pos = (node), (pos && (n = pos->next)); pos; pos = n, n = 
>> pos->next)
>
> Do you think it should be modified? I think mine is simpler. No?

Personally I prefer the style of other xxx_for_each_safe().

Best Regards,
Huang, Ying

>> 
>> Best Regards,
>> Huang, Ying
>> 
>> > +/**
>> >   * llist_for_each_entry - iterate over some deleted entries of lock-less 
>> > list of given type
>> >   * @pos:  the type * to use as a loop cursor.
>> >   * @node: the fist entry of deleted list entries.

Re: [PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Byungchul Park]

On Mon, Feb 13, 2017 at 03:36:33PM +0800, Huang, Ying wrote:
> Byungchul Park  writes:
> 
> > Sometimes we have to dereference next field of llist node before entering
> > loop becasue the node might be deleted or the next field might be
> > modified within the loop. So this adds the safe version of llist_for_each,
> > that is, llist_for_each_safe.
> >
> > Signed-off-by: Byungchul Park 
> > ---
> >  include/linux/llist.h | 19 +++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/include/linux/llist.h b/include/linux/llist.h
> > index fd4ca0b..4c508a5 100644
> > --- a/include/linux/llist.h
> > +++ b/include/linux/llist.h
> > @@ -105,6 +105,25 @@ static inline void init_llist_head(struct llist_head 
> > *list)
> > for ((pos) = (node); pos; (pos) = (pos)->next)
> >  
> >  /**
> > + * llist_for_each_safe - iterate over some deleted entries of a lock-less 
> > list
> > + *  safe against removal of list entry
> > + * @pos:   the  llist_node to use as a loop cursor
> > + * @n: another type * to use as temporary storage
> 
> s/type */ llist_node/

Yes.

> 
> > + * @node:  the first entry of deleted list entries
> > + *
> > + * In general, some entries of the lock-less list can be traversed
> > + * safely only after being deleted from list, so start with an entry
> > + * instead of list head.
> > + *
> > + * If being used on entries deleted from lock-less list directly, the
> > + * traverse order is from the newest to the oldest added entry.  If
> > + * you want to traverse from the oldest to the newest, you must
> > + * reverse the order by yourself before traversing.
> > + */
> > +#define llist_for_each_safe(pos, n, node)  \
> > +   for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) = (n))
> > +
> 
> Following the style of other xxx_for_each_safe,
> 
> #define llist_for_each_safe(pos, n, node) \
>   for (pos = (node), (pos && (n = pos->next)); pos; pos = n, n = 
> pos->next)

Do you think it should be modified? I think mine is simpler. No?

> 
> Best Regards,
> Huang, Ying
> 
> > +/**
> >   * llist_for_each_entry - iterate over some deleted entries of lock-less 
> > list of given type
> >   * @pos:   the type * to use as a loop cursor.
> >   * @node:  the fist entry of deleted list entries.

Re: [PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Byungchul Park]

On Mon, Feb 13, 2017 at 03:36:33PM +0800, Huang, Ying wrote:
> Byungchul Park  writes:
> 
> > Sometimes we have to dereference next field of llist node before entering
> > loop becasue the node might be deleted or the next field might be
> > modified within the loop. So this adds the safe version of llist_for_each,
> > that is, llist_for_each_safe.
> >
> > Signed-off-by: Byungchul Park 
> > ---
> >  include/linux/llist.h | 19 +++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/include/linux/llist.h b/include/linux/llist.h
> > index fd4ca0b..4c508a5 100644
> > --- a/include/linux/llist.h
> > +++ b/include/linux/llist.h
> > @@ -105,6 +105,25 @@ static inline void init_llist_head(struct llist_head 
> > *list)
> > for ((pos) = (node); pos; (pos) = (pos)->next)
> >  
> >  /**
> > + * llist_for_each_safe - iterate over some deleted entries of a lock-less 
> > list
> > + *  safe against removal of list entry
> > + * @pos:   the  llist_node to use as a loop cursor
> > + * @n: another type * to use as temporary storage
> 
> s/type */ llist_node/

Yes.

> 
> > + * @node:  the first entry of deleted list entries
> > + *
> > + * In general, some entries of the lock-less list can be traversed
> > + * safely only after being deleted from list, so start with an entry
> > + * instead of list head.
> > + *
> > + * If being used on entries deleted from lock-less list directly, the
> > + * traverse order is from the newest to the oldest added entry.  If
> > + * you want to traverse from the oldest to the newest, you must
> > + * reverse the order by yourself before traversing.
> > + */
> > +#define llist_for_each_safe(pos, n, node)  \
> > +   for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) = (n))
> > +
> 
> Following the style of other xxx_for_each_safe,
> 
> #define llist_for_each_safe(pos, n, node) \
>   for (pos = (node), (pos && (n = pos->next)); pos; pos = n, n = 
> pos->next)

Do you think it should be modified? I think mine is simpler. No?

> 
> Best Regards,
> Huang, Ying
> 
> > +/**
> >   * llist_for_each_entry - iterate over some deleted entries of lock-less 
> > list of given type
> >   * @pos:   the type * to use as a loop cursor.
> >   * @node:  the fist entry of deleted list entries.

Re: [PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Huang, Ying]

Byungchul Park  writes:

> Sometimes we have to dereference next field of llist node before entering
> loop becasue the node might be deleted or the next field might be
> modified within the loop. So this adds the safe version of llist_for_each,
> that is, llist_for_each_safe.
>
> Signed-off-by: Byungchul Park 
> ---
>  include/linux/llist.h | 19 +++
>  1 file changed, 19 insertions(+)
>
> diff --git a/include/linux/llist.h b/include/linux/llist.h
> index fd4ca0b..4c508a5 100644
> --- a/include/linux/llist.h
> +++ b/include/linux/llist.h
> @@ -105,6 +105,25 @@ static inline void init_llist_head(struct llist_head 
> *list)
>   for ((pos) = (node); pos; (pos) = (pos)->next)
>  
>  /**
> + * llist_for_each_safe - iterate over some deleted entries of a lock-less 
> list
> + *safe against removal of list entry
> + * @pos: the  llist_node to use as a loop cursor
> + * @n:   another type * to use as temporary storage

s/type */ llist_node/

> + * @node:the first entry of deleted list entries
> + *
> + * In general, some entries of the lock-less list can be traversed
> + * safely only after being deleted from list, so start with an entry
> + * instead of list head.
> + *
> + * If being used on entries deleted from lock-less list directly, the
> + * traverse order is from the newest to the oldest added entry.  If
> + * you want to traverse from the oldest to the newest, you must
> + * reverse the order by yourself before traversing.
> + */
> +#define llist_for_each_safe(pos, n, node)\
> + for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) = (n))
> +

Following the style of other xxx_for_each_safe,

#define llist_for_each_safe(pos, n, node)   \
for (pos = (node), (pos && (n = pos->next)); pos; pos = n, n = 
pos->next)

Best Regards,
Huang, Ying

> +/**
>   * llist_for_each_entry - iterate over some deleted entries of lock-less 
> list of given type
>   * @pos: the type * to use as a loop cursor.
>   * @node:the fist entry of deleted list entries.

Re: [PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Huang, Ying]

Byungchul Park  writes:

> Sometimes we have to dereference next field of llist node before entering
> loop becasue the node might be deleted or the next field might be
> modified within the loop. So this adds the safe version of llist_for_each,
> that is, llist_for_each_safe.
>
> Signed-off-by: Byungchul Park 
> ---
>  include/linux/llist.h | 19 +++
>  1 file changed, 19 insertions(+)
>
> diff --git a/include/linux/llist.h b/include/linux/llist.h
> index fd4ca0b..4c508a5 100644
> --- a/include/linux/llist.h
> +++ b/include/linux/llist.h
> @@ -105,6 +105,25 @@ static inline void init_llist_head(struct llist_head 
> *list)
>   for ((pos) = (node); pos; (pos) = (pos)->next)
>  
>  /**
> + * llist_for_each_safe - iterate over some deleted entries of a lock-less 
> list
> + *safe against removal of list entry
> + * @pos: the  llist_node to use as a loop cursor
> + * @n:   another type * to use as temporary storage

s/type */ llist_node/

> + * @node:the first entry of deleted list entries
> + *
> + * In general, some entries of the lock-less list can be traversed
> + * safely only after being deleted from list, so start with an entry
> + * instead of list head.
> + *
> + * If being used on entries deleted from lock-less list directly, the
> + * traverse order is from the newest to the oldest added entry.  If
> + * you want to traverse from the oldest to the newest, you must
> + * reverse the order by yourself before traversing.
> + */
> +#define llist_for_each_safe(pos, n, node)\
> + for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) = (n))
> +

Following the style of other xxx_for_each_safe,

#define llist_for_each_safe(pos, n, node)   \
for (pos = (node), (pos && (n = pos->next)); pos; pos = n, n = 
pos->next)

Best Regards,
Huang, Ying

> +/**
>   * llist_for_each_entry - iterate over some deleted entries of lock-less 
> list of given type
>   * @pos: the type * to use as a loop cursor.
>   * @node:the fist entry of deleted list entries.

[PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Byungchul Park]

Sometimes we have to dereference next field of llist node before entering
loop becasue the node might be deleted or the next field might be
modified within the loop. So this adds the safe version of llist_for_each,
that is, llist_for_each_safe.

Signed-off-by: Byungchul Park 
---
 include/linux/llist.h | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/llist.h b/include/linux/llist.h
index fd4ca0b..4c508a5 100644
--- a/include/linux/llist.h
+++ b/include/linux/llist.h
@@ -105,6 +105,25 @@ static inline void init_llist_head(struct llist_head *list)
for ((pos) = (node); pos; (pos) = (pos)->next)
 
 /**
+ * llist_for_each_safe - iterate over some deleted entries of a lock-less list
+ *  safe against removal of list entry
+ * @pos:   the  llist_node to use as a loop cursor
+ * @n: another type * to use as temporary storage
+ * @node:  the first entry of deleted list entries
+ *
+ * In general, some entries of the lock-less list can be traversed
+ * safely only after being deleted from list, so start with an entry
+ * instead of list head.
+ *
+ * If being used on entries deleted from lock-less list directly, the
+ * traverse order is from the newest to the oldest added entry.  If
+ * you want to traverse from the oldest to the newest, you must
+ * reverse the order by yourself before traversing.
+ */
+#define llist_for_each_safe(pos, n, node)  \
+   for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) = (n))
+
+/**
  * llist_for_each_entry - iterate over some deleted entries of lock-less list 
of given type
  * @pos:   the type * to use as a loop cursor.
  * @node:  the fist entry of deleted list entries.
-- 
1.9.1

[PATCH v2 1/9] llist: Provide a safe version for llist_for_each

[Byungchul Park]

Sometimes we have to dereference next field of llist node before entering
loop becasue the node might be deleted or the next field might be
modified within the loop. So this adds the safe version of llist_for_each,
that is, llist_for_each_safe.

Signed-off-by: Byungchul Park 
---
 include/linux/llist.h | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/llist.h b/include/linux/llist.h
index fd4ca0b..4c508a5 100644
--- a/include/linux/llist.h
+++ b/include/linux/llist.h
@@ -105,6 +105,25 @@ static inline void init_llist_head(struct llist_head *list)
for ((pos) = (node); pos; (pos) = (pos)->next)
 
 /**
+ * llist_for_each_safe - iterate over some deleted entries of a lock-less list
+ *  safe against removal of list entry
+ * @pos:   the  llist_node to use as a loop cursor
+ * @n: another type * to use as temporary storage
+ * @node:  the first entry of deleted list entries
+ *
+ * In general, some entries of the lock-less list can be traversed
+ * safely only after being deleted from list, so start with an entry
+ * instead of list head.
+ *
+ * If being used on entries deleted from lock-less list directly, the
+ * traverse order is from the newest to the oldest added entry.  If
+ * you want to traverse from the oldest to the newest, you must
+ * reverse the order by yourself before traversing.
+ */
+#define llist_for_each_safe(pos, n, node)  \
+   for ((pos) = (node); (pos) && ((n) = (pos)->next, true); (pos) = (n))
+
+/**
  * llist_for_each_entry - iterate over some deleted entries of lock-less list 
of given type
  * @pos:   the type * to use as a loop cursor.
  * @node:  the fist entry of deleted list entries.
-- 
1.9.1