Azat Khuzhin
2013-Jul-27 05:15 UTC
[PATCH] btrfs: use list_for_each_entry_safe() when delete items
Replace list_for_each_entry() by list_for_each_entry_safe() in next
functions:
- lock_stripe_add()
- __btrfs_close_devices()
Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com>
---
fs/btrfs/raid56.c | 4 ++--
fs/btrfs/volumes.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index 0525e13..37e6dc9 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -636,7 +636,7 @@ static noinline int lock_stripe_add(struct btrfs_raid_bio
*rbio)
{
int bucket = rbio_bucket(rbio);
struct btrfs_stripe_hash *h = rbio->fs_info->stripe_hash_table->table
+ bucket;
- struct btrfs_raid_bio *cur;
+ struct btrfs_raid_bio *cur, *next;
struct btrfs_raid_bio *pending;
unsigned long flags;
DEFINE_WAIT(wait);
@@ -646,7 +646,7 @@ static noinline int lock_stripe_add(struct btrfs_raid_bio
*rbio)
int walk = 0;
spin_lock_irqsave(&h->lock, flags);
- list_for_each_entry(cur, &h->hash_list, hash_list) {
+ list_for_each_entry_safe(cur, next, &h->hash_list, hash_list) {
walk++;
if (cur->raid_map[0] == rbio->raid_map[0]) {
spin_lock(&cur->bio_list_lock);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 78b8717..1d1b595 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -616,13 +616,13 @@ static void free_device(struct rcu_head *head)
static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
{
- struct btrfs_device *device;
+ struct btrfs_device *device, *next;
if (--fs_devices->opened > 0)
return 0;
mutex_lock(&fs_devices->device_list_mutex);
- list_for_each_entry(device, &fs_devices->devices, dev_list) {
+ list_for_each_entry_safe(device, next, &fs_devices->devices, dev_list)
{
struct btrfs_device *new_device;
struct rcu_string *name;
--
1.7.10.4
Azat Khuzhin
2013-Jul-27 10:12 UTC
[PATCH] btrfs: use list_for_each_entry_safe() when delete items
Replace list_for_each_entry() by list_for_each_entry_safe() in
__btrfs_close_devices()
There is another place that delete items lock_stripe_add(), but there we
don''t need safe version, because after deleting we exit from loop.
Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com>
---
fs/btrfs/volumes.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 78b8717..1d1b595 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -616,13 +616,13 @@ static void free_device(struct rcu_head *head)
static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
{
- struct btrfs_device *device;
+ struct btrfs_device *device, *next;
if (--fs_devices->opened > 0)
return 0;
mutex_lock(&fs_devices->device_list_mutex);
- list_for_each_entry(device, &fs_devices->devices, dev_list) {
+ list_for_each_entry_safe(device, next, &fs_devices->devices, dev_list)
{
struct btrfs_device *new_device;
struct rcu_string *name;
--
1.7.10.4
Azat Khuzhin
2013-Jul-29 07:48 UTC
Re: [PATCH] btrfs: use list_for_each_entry_safe() when delete items
On Sat, Jul 27, 2013 at 2:12 PM, Azat Khuzhin <a3at.mail@gmail.com> wrote:> Replace list_for_each_entry() by list_for_each_entry_safe() in > __btrfs_close_devices() > > There is another place that delete items lock_stripe_add(), but there we > don''t need safe version, because after deleting we exit from loop. > > Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> > --- > fs/btrfs/volumes.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 78b8717..1d1b595 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -616,13 +616,13 @@ static void free_device(struct rcu_head *head) > > static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices) > { > - struct btrfs_device *device; > + struct btrfs_device *device, *next; > > if (--fs_devices->opened > 0) > return 0; > > mutex_lock(&fs_devices->device_list_mutex); > - list_for_each_entry(device, &fs_devices->devices, dev_list) { > + list_for_each_entry_safe(device, next, &fs_devices->devices, dev_list) { > struct btrfs_device *new_device; > struct rcu_string *name;There is "kfree(device);" at the end of loop, maybe there must "goto again;" after it? (instead of this patch)> > -- > 1.7.10.4 >-- Respectfully Azat Khuzhin
Miao Xie
2013-Jul-30 03:40 UTC
Re: [PATCH] btrfs: use list_for_each_entry_safe() when delete items
On mon, 29 Jul 2013 11:48:32 +0400, Azat Khuzhin wrote:> On Sat, Jul 27, 2013 at 2:12 PM, Azat Khuzhin <a3at.mail@gmail.com> wrote: >> Replace list_for_each_entry() by list_for_each_entry_safe() in >> __btrfs_close_devices() >> >> There is another place that delete items lock_stripe_add(), but there we >> don''t need safe version, because after deleting we exit from loop. >> >> Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> >> --- >> fs/btrfs/volumes.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index 78b8717..1d1b595 100644 >> --- a/fs/btrfs/volumes.c >> +++ b/fs/btrfs/volumes.c >> @@ -616,13 +616,13 @@ static void free_device(struct rcu_head *head) >> >> static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices) >> { >> - struct btrfs_device *device; >> + struct btrfs_device *device, *next; >> >> if (--fs_devices->opened > 0) >> return 0; >> >> mutex_lock(&fs_devices->device_list_mutex); >> - list_for_each_entry(device, &fs_devices->devices, dev_list) { >> + list_for_each_entry_safe(device, next, &fs_devices->devices, dev_list) { >> struct btrfs_device *new_device; >> struct rcu_string *name; > > There is "kfree(device);" at the end of loop, maybe there must "goto > again;" after it? > (instead of this patch)Your fix is right, we needn''t search from the head once again. The other fix way is: call_rcu(&device->rcu, free_device); + device = new_device; } but from the viewpoint of the readability, this way is not so good. Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>> >> >> -- >> 1.7.10.4 >> > > >
Azat Khuzhin
2013-Aug-31 06:11 UTC
Re: [PATCH] btrfs: use list_for_each_entry_safe() when delete items
On Tue, Jul 30, 2013 at 7:40 AM, Miao Xie <miaox@cn.fujitsu.com> wrote:> On mon, 29 Jul 2013 11:48:32 +0400, Azat Khuzhin wrote: >> On Sat, Jul 27, 2013 at 2:12 PM, Azat Khuzhin <a3at.mail@gmail.com> wrote: >>> Replace list_for_each_entry() by list_for_each_entry_safe() in >>> __btrfs_close_devices() >>> >>> There is another place that delete items lock_stripe_add(), but there we >>> don''t need safe version, because after deleting we exit from loop. >>> >>> Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> >>> --- >>> fs/btrfs/volumes.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >>> index 78b8717..1d1b595 100644 >>> --- a/fs/btrfs/volumes.c >>> +++ b/fs/btrfs/volumes.c >>> @@ -616,13 +616,13 @@ static void free_device(struct rcu_head *head) >>> >>> static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices) >>> { >>> - struct btrfs_device *device; >>> + struct btrfs_device *device, *next; >>> >>> if (--fs_devices->opened > 0) >>> return 0; >>> >>> mutex_lock(&fs_devices->device_list_mutex); >>> - list_for_each_entry(device, &fs_devices->devices, dev_list) { >>> + list_for_each_entry_safe(device, next, &fs_devices->devices, dev_list) { >>> struct btrfs_device *new_device; >>> struct rcu_string *name; >> >> There is "kfree(device);" at the end of loop, maybe there must "goto >> again;" after it? >> (instead of this patch)Ugh. I was looking into another function!> > Your fix is right, we needn''t search from the head once again. > > The other fix way is: > call_rcu(&device->rcu, free_device); > + device = new_device; > } > but from the viewpoint of the readability, this way is not so good. > > Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>Thanks! Miao, should I resend patch with you reviewed-by?> >> >>> >>> -- >>> 1.7.10.4 >>> >> >> >> >-- Respectfully Azat Khuzhin