set_page_dirty/page_launder deadlock

set_page_dirty/page_launder deadlock

[Marcelo Tosatti]

Hi,

While taking a look at page_launder()...

                 /* And re-start the thing.. */
                 spin_lock(&pagemap_lru_lock); 	<----------
                 if (result != 1)
                 	continue;
                 /* writepage refused to do anything */
                 set_page_dirty(page);
	      	 ^^^^^^^^^^^^^^^^^^^^
       		 goto page_active;
            }


set_page_dirty() may lock the pagecache_lock which means potential
deadlock since we have the pagemap_lru_lock locked.


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

Re: set_page_dirty/page_launder deadlock

[David S. Miller]

Marcelo Tosatti writes:
 > 
 > While taking a look at page_launder()...

 ...

 > set_page_dirty() may lock the pagecache_lock which means potential
 > deadlock since we have the pagemap_lru_lock locked.

Indeed, the following should work as a fix:

--- mm/vmscan.c.~1~	Thu Jan 11 02:22:19 2001
+++ mm/vmscan.c	Sun Jan 14 05:26:17 2001
@@ -493,12 +493,15 @@
 			page_cache_release(page);
 
 			/* And re-start the thing.. */
-			spin_lock(&pagemap_lru_lock);
-			if (result != 1)
+			if (result != 1) {
+				spin_lock(&pagemap_lru_lock);
 				continue;
-			/* writepage refused to do anything */
-			set_page_dirty(page);
-			goto page_active;
+			} else {
+				/* writepage refused to do anything */
+				set_page_dirty(page);
+				spin_lock(&pagemap_lru_lock);
+				goto page_active;
+			}
 		}
 
 		/*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: set_page_dirty/page_launder deadlock

[Linus Torvalds]

On Sun, 14 Jan 2001, David S. Miller wrote:
> 
> Marcelo Tosatti writes:
>  > 
>  > While taking a look at page_launder()...
> 
>  ...
> 
>  > set_page_dirty() may lock the pagecache_lock which means potential
>  > deadlock since we have the pagemap_lru_lock locked.
> 
> Indeed, the following should work as a fix:

Well, as the new shm code doesn't return 1 any more, the whole locked page
handling should just be deleted. ramfs always just re-marked the page
dirty in its own "writepage()" function, so it was only shmfs that ever
returned this special case, and because of other issues it already got
excised by Christoph..

		Linus

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

Re: set_page_dirty/page_launder deadlock

[Chris Mason]

On Sunday, January 14, 2001 10:56:10 AM -0800 Linus Torvalds
<torvalds@transmeta.com> wrote:

>> Marcelo Tosatti writes:
>>  > 
>>  > While taking a look at page_launder()...
>> 
>>  ...
>> 
>>  > set_page_dirty() may lock the pagecache_lock which means potential
>>  > deadlock since we have the pagemap_lru_lock locked.
>> 
> 
> Well, as the new shm code doesn't return 1 any more, the whole locked page
> handling should just be deleted. ramfs always just re-marked the page
> dirty in its own "writepage()" function, so it was only shmfs that ever
> returned this special case, and because of other issues it already got
> excised by Christoph..
> 

Then I'm confused by the code in 2.4.1pre8:

-chris

/*
 * Move the page from the page cache to the swap cache
 */
static int shmem_writepage(struct page * page)
{
	int error;
	struct shmem_inode_info *info;
	swp_entry_t *entry, swap;

	info = &page->mapping->host->u.shmem_i;
	if (info->locked)
		return 1;
	swap = __get_swap_page(2);
	if (!swap.val)
		return 1;


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

Re: set_page_dirty/page_launder deadlock

[Christoph Rohland]

Hi Linus,

On Sun, 14 Jan 2001, Linus Torvalds wrote:
> Well, as the new shm code doesn't return 1 any more, the whole
> locked page handling should just be deleted. ramfs always just
> re-marked the page dirty in its own "writepage()" function, so it
> was only shmfs that ever returned this special case, and because of
> other issues it already got excised by Christoph..

No, that's not completely right. There may be rare cases like out of
swap that shmem_write does return 1. But couldn't it simply set the
page dirty like ramfs_writepage?

Greetings
		Christoph

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

Re: set_page_dirty/page_launder deadlock

[Hugh Dickins]

On 19 Jan 2001, Christoph Rohland wrote:
> On Sun, 14 Jan 2001, Linus Torvalds wrote:
> > Well, as the new shm code doesn't return 1 any more, the whole
> > locked page handling should just be deleted. ramfs always just
> > re-marked the page dirty in its own "writepage()" function, so it
> > was only shmfs that ever returned this special case, and because of
> > other issues it already got excised by Christoph..
> 
> No, that's not completely right. There may be rare cases like out of
> swap that shmem_write does return 1. But couldn't it simply set the
> page dirty like ramfs_writepage?

I notice that shmem_writepage() in 2.4.1-pre10 is still doing an
early "return 1" without UnlockPage(page): surely that's wrong?

Hugh

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