2018-09-26 08:50:29

by zhong jiang

Subject: [PATCH] btrfs: list usage cleanup

Trival cleanup, list_move_tail will implement the same function that
list_del() + list_add_tail() will do. hence just replace them.

Signed-off-by: zhong jiang <[email protected]>
---
fs/btrfs/send.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 094cc144..d87f416 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -2075,8 +2075,7 @@ static struct name_cache_entry *name_cache_search(struct send_ctx *sctx,
*/
static void name_cache_used(struct send_ctx *sctx, struct name_cache_entry *nce)
{
- list_del(&nce->list);
- list_add_tail(&nce->list, &sctx->name_cache_list);
+ list_move_tail(&nce->list, &sctx->name_cache_list);
}

/*
--
1.7.12.4

2018-09-26 08:54:52

by Nikolay Borisov

Subject: Re: [PATCH] btrfs: list usage cleanup

On 26.09.2018 11:35, zhong jiang wrote:
> Trival cleanup, list_move_tail will implement the same function that
> list_del() + list_add_tail() will do. hence just replace them.
>
> Signed-off-by: zhong jiang <[email protected]>

Reviewed-by: Nikolay Borisov <[email protected]>

> ---
> fs/btrfs/send.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 094cc144..d87f416 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -2075,8 +2075,7 @@ static struct name_cache_entry *name_cache_search(struct send_ctx *sctx,
> */
> static void name_cache_used(struct send_ctx *sctx, struct name_cache_entry *nce)
> {
> - list_del(&nce->list);
> - list_add_tail(&nce->list, &sctx->name_cache_list);
> + list_move_tail(&nce->list, &sctx->name_cache_list);
> }
>
> /*
>

2018-09-27 09:09:22

by David Sterba

Subject: Re: [PATCH] btrfs: list usage cleanup

On Wed, Sep 26, 2018 at 04:35:45PM +0800, zhong jiang wrote:
> Trival cleanup, list_move_tail will implement the same function that
> list_del() + list_add_tail() will do. hence just replace them.

Thanks, I've checked that this is the only instance where this cleanup
is applicable.

Reviewed-by: David Sterba <[email protected]>

2018-09-27 18:48:02

by Omar Sandoval

Subject: Re: [PATCH] btrfs: list usage cleanup

On Wed, Sep 26, 2018 at 04:35:45PM +0800, zhong jiang wrote:
> Trival cleanup, list_move_tail will implement the same function that
> list_del() + list_add_tail() will do. hence just replace them.
>
> Signed-off-by: zhong jiang <[email protected]>
> ---
> fs/btrfs/send.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 094cc144..d87f416 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -2075,8 +2075,7 @@ static struct name_cache_entry *name_cache_search(struct send_ctx *sctx,
> */
> static void name_cache_used(struct send_ctx *sctx, struct name_cache_entry *nce)
> {
> - list_del(&nce->list);
> - list_add_tail(&nce->list, &sctx->name_cache_list);
> + list_move_tail(&nce->list, &sctx->name_cache_list);
> }

At that point do we even need such a trivial helper, considering that
this is only called in one place?

2018-10-01 16:48:55

by David Sterba

Subject: Re: [PATCH] btrfs: list usage cleanup

On Thu, Sep 27, 2018 at 11:47:04AM -0700, Omar Sandoval wrote:
> On Wed, Sep 26, 2018 at 04:35:45PM +0800, zhong jiang wrote:
> > Trival cleanup, list_move_tail will implement the same function that
> > list_del() + list_add_tail() will do. hence just replace them.
> >
> > Signed-off-by: zhong jiang <[email protected]>
> > ---
> > fs/btrfs/send.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> > index 094cc144..d87f416 100644
> > --- a/fs/btrfs/send.c
> > +++ b/fs/btrfs/send.c
> > @@ -2075,8 +2075,7 @@ static struct name_cache_entry *name_cache_search(struct send_ctx *sctx,
> > */
> > static void name_cache_used(struct send_ctx *sctx, struct name_cache_entry *nce)
> > {
> > - list_del(&nce->list);
> > - list_add_tail(&nce->list, &sctx->name_cache_list);
> > + list_move_tail(&nce->list, &sctx->name_cache_list);
> > }
>
> At that point do we even need such a trivial helper, considering that
> this is only called in one place?

Fair point and trivial one-line helpers are on the cleanup todo list.
The exception is when the actual helper implementation is obscuring the
semantics and the helper is used in many places so it's not practical to
add a comment everywhere. But it's not the case here.

Zhong Jiang, please update the patch and resend, thanks.

2018-10-04 03:26:14

by zhong jiang

Subject: Re: [PATCH] btrfs: list usage cleanup

On 2018/10/2 0:48, David Sterba wrote:
> On Thu, Sep 27, 2018 at 11:47:04AM -0700, Omar Sandoval wrote:
>> On Wed, Sep 26, 2018 at 04:35:45PM +0800, zhong jiang wrote:
>>> Trival cleanup, list_move_tail will implement the same function that
>>> list_del() + list_add_tail() will do. hence just replace them.
>>>
>>> Signed-off-by: zhong jiang <[email protected]>
>>> ---
>>> fs/btrfs/send.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
>>> index 094cc144..d87f416 100644
>>> --- a/fs/btrfs/send.c
>>> +++ b/fs/btrfs/send.c
>>> @@ -2075,8 +2075,7 @@ static struct name_cache_entry *name_cache_search(struct send_ctx *sctx,
>>> */
>>> static void name_cache_used(struct send_ctx *sctx, struct name_cache_entry *nce)
>>> {
>>> - list_del(&nce->list);
>>> - list_add_tail(&nce->list, &sctx->name_cache_list);
>>> + list_move_tail(&nce->list, &sctx->name_cache_list);
>>> }
>> At that point do we even need such a trivial helper, considering that
>> this is only called in one place?
> Fair point and trivial one-line helpers are on the cleanup todo list.
> The exception is when the actual helper implementation is obscuring the
> semantics and the helper is used in many places so it's not practical to
> add a comment everywhere. But it's not the case here.
>
> Zhong Jiang, please update the patch and resend, thanks.
Will add a comment here and resend in v2.

Thanks,
zhong jiang
> .
>