Re: [PATCH 5/5] Direct Migration V9: Avoid writeback / page_migrate()method

[Christoph Lameter]

On Tue, 10 Jan 2006, Andrew Morton wrote:

> Christoph Lameter <clameter@xxxxxxx> wrote:
> >
> > +	spin_lock(&mapping->private_lock);
> >  +
> >  +	bh = head;
> >  +	do {
> >  +		get_bh(bh);
> >  +		lock_buffer(bh);
> >  +		bh = bh->b_this_page;
> >  +
> >  +	} while (bh != head);
> >  +
> 
> Guys, lock_buffer() sleeps and cannot be called inside spinlock.

I took it the way it was in the hotplug patches.We are taking the 
spinlock here to protect the scan over the list of bh's of this page 
right?

Is it not sufficient to have the page locked to guarantee that the list of 
buffers is not changed? Seems that ext3 does that (see 
ext3_ordered_writepage() etc).

like this?

Index: linux-2.6.15/fs/buffer.c
===================================================================
--- linux-2.6.15.orig/fs/buffer.c	2006-01-10 22:13:37.000000000 -0800
+++ linux-2.6.15/fs/buffer.c	2006-01-10 22:37:28.000000000 -0800
@@ -3070,8 +3070,6 @@ int buffer_migrate_page(struct page *new
 	if (migrate_page_remove_references(newpage, page, 3))
 		return -EAGAIN;
 
-	spin_lock(&mapping->private_lock);
-
 	bh = head;
 	do {
 		get_bh(bh);
@@ -3094,11 +3092,9 @@ int buffer_migrate_page(struct page *new
 	} while (bh != head);
 
 	SetPagePrivate(newpage);
-	spin_unlock(&mapping->private_lock);
 
 	migrate_page_copy(newpage, page);
 
-	spin_lock(&mapping->private_lock);
 	bh = head;
 	do {
 		unlock_buffer(bh);
@@ -3106,7 +3102,6 @@ int buffer_migrate_page(struct page *new
 		bh = bh->b_this_page;
 
 	} while (bh != head);
-	spin_unlock(&mapping->private_lock);
 
 	return 0;
 }
-
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/