Revision ad576e63e0c8b274a8558b8e05a6d0526e804dc0 authored by Nick Piggin on 05 May 2005, 23:15:46 UTC, committed by Linus Torvalds on 05 May 2005, 23:36:40 UTC
When running
	fsstress -v -d $DIR/tmp -n 1000 -p 1000 -l 2
on an ext2 filesystem with 1024 byte block size, on SMP i386 with 4096 byte
page size over loopback to an image file on a tmpfs filesystem, I would
very quickly hit
	BUG_ON(!buffer_async_write(bh));
in fs/buffer.c:end_buffer_async_write

It seems that more than one request would be submitted for a given bh
at a time.

What would happen is the following:
2 threads doing __mpage_writepages on the same page.
Thread 1 - lock the page first, and enter __block_write_full_page.
Thread 1 - (eg.) mark_buffer_async_write on the first 2 buffers.
Thread 1 - set page writeback, unlock page.
Thread 2 - lock page, wait on page writeback
Thread 1 - submit_bh on the first 2 buffers.
=> both requests complete, none of the page buffers are async_write,
   end_page_writeback is called.
Thread 2 - wakes up. enters __block_write_full_page.
Thread 2 - mark_buffer_async_write on (eg.) the last buffer
Thread 1 - finds the last buffer has async_write set, submit_bh on that.
Thread 2 - submit_bh on the last buffer.
=> oops.

So change __block_write_full_page to explicitly keep track of the last bh
we need to issue, so we don't touch anything after issuing the last
request.

Signed-off-by: Nick Piggin <nickpiggin@yahoo.com.au>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
1 parent f3ddbdc
Raw File
fifo.c
/*
 *  linux/fs/fifo.c
 *
 *  written by Paul H. Hargrove
 *
 *  Fixes:
 *	10-06-1999, AV: fixed OOM handling in fifo_open(), moved
 *			initialization there, switched to external
 *			allocation of pipe_inode_info.
 */

#include <linux/mm.h>
#include <linux/slab.h>
#include <linux/smp_lock.h>
#include <linux/fs.h>
#include <linux/pipe_fs_i.h>

static void wait_for_partner(struct inode* inode, unsigned int* cnt)
{
	int cur = *cnt;	
	while(cur == *cnt) {
		pipe_wait(inode);
		if(signal_pending(current))
			break;
	}
}

static void wake_up_partner(struct inode* inode)
{
	wake_up_interruptible(PIPE_WAIT(*inode));
}

static int fifo_open(struct inode *inode, struct file *filp)
{
	int ret;

	ret = -ERESTARTSYS;
	if (down_interruptible(PIPE_SEM(*inode)))
		goto err_nolock_nocleanup;

	if (!inode->i_pipe) {
		ret = -ENOMEM;
		if(!pipe_new(inode))
			goto err_nocleanup;
	}
	filp->f_version = 0;

	/* We can only do regular read/write on fifos */
	filp->f_mode &= (FMODE_READ | FMODE_WRITE);

	switch (filp->f_mode) {
	case 1:
	/*
	 *  O_RDONLY
	 *  POSIX.1 says that O_NONBLOCK means return with the FIFO
	 *  opened, even when there is no process writing the FIFO.
	 */
		filp->f_op = &read_fifo_fops;
		PIPE_RCOUNTER(*inode)++;
		if (PIPE_READERS(*inode)++ == 0)
			wake_up_partner(inode);

		if (!PIPE_WRITERS(*inode)) {
			if ((filp->f_flags & O_NONBLOCK)) {
				/* suppress POLLHUP until we have
				 * seen a writer */
				filp->f_version = PIPE_WCOUNTER(*inode);
			} else 
			{
				wait_for_partner(inode, &PIPE_WCOUNTER(*inode));
				if(signal_pending(current))
					goto err_rd;
			}
		}
		break;
	
	case 2:
	/*
	 *  O_WRONLY
	 *  POSIX.1 says that O_NONBLOCK means return -1 with
	 *  errno=ENXIO when there is no process reading the FIFO.
	 */
		ret = -ENXIO;
		if ((filp->f_flags & O_NONBLOCK) && !PIPE_READERS(*inode))
			goto err;

		filp->f_op = &write_fifo_fops;
		PIPE_WCOUNTER(*inode)++;
		if (!PIPE_WRITERS(*inode)++)
			wake_up_partner(inode);

		if (!PIPE_READERS(*inode)) {
			wait_for_partner(inode, &PIPE_RCOUNTER(*inode));
			if (signal_pending(current))
				goto err_wr;
		}
		break;
	
	case 3:
	/*
	 *  O_RDWR
	 *  POSIX.1 leaves this case "undefined" when O_NONBLOCK is set.
	 *  This implementation will NEVER block on a O_RDWR open, since
	 *  the process can at least talk to itself.
	 */
		filp->f_op = &rdwr_fifo_fops;

		PIPE_READERS(*inode)++;
		PIPE_WRITERS(*inode)++;
		PIPE_RCOUNTER(*inode)++;
		PIPE_WCOUNTER(*inode)++;
		if (PIPE_READERS(*inode) == 1 || PIPE_WRITERS(*inode) == 1)
			wake_up_partner(inode);
		break;

	default:
		ret = -EINVAL;
		goto err;
	}

	/* Ok! */
	up(PIPE_SEM(*inode));
	return 0;

err_rd:
	if (!--PIPE_READERS(*inode))
		wake_up_interruptible(PIPE_WAIT(*inode));
	ret = -ERESTARTSYS;
	goto err;

err_wr:
	if (!--PIPE_WRITERS(*inode))
		wake_up_interruptible(PIPE_WAIT(*inode));
	ret = -ERESTARTSYS;
	goto err;

err:
	if (!PIPE_READERS(*inode) && !PIPE_WRITERS(*inode))
		free_pipe_info(inode);

err_nocleanup:
	up(PIPE_SEM(*inode));

err_nolock_nocleanup:
	return ret;
}

/*
 * Dummy default file-operations: the only thing this does
 * is contain the open that then fills in the correct operations
 * depending on the access mode of the file...
 */
struct file_operations def_fifo_fops = {
	.open		= fifo_open,	/* will set read or write pipe_fops */
};
back to top