deadlock on write in tmpfs

Hi Linus and Stephen,

tmpfs deadlocks when writing into a file from a mapping of the same file.

The problem is the following:

- shmem_file_write may call shmem_no_page and calls shmem_getpage_locked later,
- shmem_no_page calls shmem_getpage_locked
- shmem_getpage_locked may call shmem_writepage on page allocation

- shmem_file_write holds the inode semaphore
- shmem_getpage_locked prevent races against shmem_writepage with the shmem spinlock
- shmem_getpage_locked needs serialization against itself and shmem_truncate

The last was done with the inode semaphore, which deadlocks with shmem_write

So I see two choices:

1) Do not serialise the whole of shmem_getpage_locked but protect critical pathes with the spinlock and do retries after sleeps
2) Add another semaphore to serialize shmem_getpage_locked and shmem_truncate

I tried some time to get 1) done but the retry logic became way too complicated. So the attached patch implements 2)

I still think it's ugly to add another semaphore, but it works.

Greetings
Christoph

diff -uNr 2.4.4/include/linux/shmem_fs.h c/include/linux/shmem_fs.h
@@ -19,6 +19,7 @@
 
 struct shmem_inode_info {
 	spinlock_t	lock;
+	struct semaphore sem;
 	unsigned long	max_index;
 	swp_entry_t	i_direct[SHMEM_NR_DIRECT]; /* for the first blocks */
 	swp_entry_t   **i_indirect; /* doubly indirect blocks */

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

hi,

If the locking is for a completely different reason, then a different semaphore is quite appropriate. In this case you're trying to lock the shm internal info structures, which is quite different from the sort of inode locking which the VFS tries to do itself, so the new semaphore appears quite clean --- and definitely needed.

--Stephen
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Hi Stephen,

It's not the addition to the inode semaphore I do care about, but the addition to the spin lock which protects also the shmem internals. But you are probably right: It only protects the onthefly pages between page cache and swap cache.

Greetings
Christoph

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/