RE: [f2fs-dev] [PATCH 2/2] f2fs: use list_for_each_entry{_safe} for simplyfying code

From: Chao Yu
Date: Mon Mar 31 2014 - 21:46:47 EST


Hi Gu,

> -----Original Message-----
> From: Gu Zheng [mailto:guz.fnst@xxxxxxxxxxxxxx]
> Sent: Monday, March 31, 2014 6:07 PM
> To: Chao Yu
> Cc: ???; linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx; linux-fsdevel@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [f2fs-dev] [PATCH 2/2] f2fs: use list_for_each_entry{_safe} for simplyfying code
>
> Hi Yu,
> On 03/29/2014 11:33 AM, Chao Yu wrote:
>
> > This patch use list_for_each_entry{_safe} instead of list_for_each{_safe} for
> > simplfying code.
> >
> > Signed-off-by: Chao Yu <chao2.yu@xxxxxxxxxxx>
> > ---
> > fs/f2fs/checkpoint.c | 37 ++++++++++++++-----------------------
> > fs/f2fs/node.c | 16 ++++++----------
> > fs/f2fs/recovery.c | 6 ++----
> > fs/f2fs/segment.c | 6 ++----
> > 4 files changed, 24 insertions(+), 41 deletions(-)
> >
> > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > index d877f46..4aa521a 100644
> > --- a/fs/f2fs/checkpoint.c
> > +++ b/fs/f2fs/checkpoint.c
> > @@ -308,16 +308,15 @@ void release_orphan_inode(struct f2fs_sb_info *sbi)
> >
> > void add_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
> > {
> > - struct list_head *head, *this;
> > - struct orphan_inode_entry *new = NULL, *orphan = NULL;
> > + struct list_head *head;
> > + struct orphan_inode_entry *new, *orphan;
> >
> > new = f2fs_kmem_cache_alloc(orphan_entry_slab, GFP_ATOMIC);
> > new->ino = ino;
> >
> > spin_lock(&sbi->orphan_inode_lock);
> > head = &sbi->orphan_inode_list;
> > - list_for_each(this, head) {
> > - orphan = list_entry(this, struct orphan_inode_entry, list);
> > + list_for_each_entry(orphan, head, list) {
> > if (orphan->ino == ino) {
> > spin_unlock(&sbi->orphan_inode_lock);
> > kmem_cache_free(orphan_entry_slab, new);
> > @@ -326,14 +325,10 @@ void add_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
> >
> > if (orphan->ino > ino)
> > break;
> > - orphan = NULL;
> > }
> >
> > - /* add new_oentry into list which is sorted by inode number */
> > - if (orphan)
> > - list_add(&new->list, this->prev);
> > - else
> > - list_add_tail(&new->list, head);
> > + /* add new orphan entry into list which is sorted by inode number */
> > + list_add_tail(&new->list, &orphan->list);
>
> It seems that the logic can not be changed here, otherwise the orphan list will not be in order
> if
> the new ino is bigger than all the in-list ones.
> E.g.
> ino:5
> 1-->2-->3-->4
> ==>
> 1-->2-->3-->5-->4

As I checked, if new ino is bigger than all, it will break from list_for_each_entry because
&orphan->list is pointing to head. So list_add_tail can add the new entry before head to make
this list in order.

Thanks.

>
> Regards,
> Gu
>

[snip]

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/