Skip to content

Commit

Permalink
NTFS: Fix a mount time deadlock.
Browse files Browse the repository at this point in the history
Big thanks go to Mathias Kolehmainen for reporting the bug, providing
debug output and testing the patches I sent him to get it working.

The fix was to stop calling ntfs_attr_set() at mount time as that causes
balance_dirty_pages_ratelimited() to be called which on systems with
little memory actually tries to go and balance the dirty pages which tries
to take the s_umount semaphore but because we are still in fill_super()
across which the VFS holds s_umount for writing this results in a
deadlock.

We now do the dirty work by hand by submitting individual buffers.  This
has the annoying "feature" that mounting can take a few seconds if the
journal is large as we have clear it all.  One day someone should improve
on this by deferring the journal clearing to a helper kernel thread so it
can be done in the background but I don't have time for this at the moment
and the current solution works fine so I am leaving it like this for now.

Signed-off-by: Anton Altaparmakov <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Anton Altaparmakov authored and Linus Torvalds committed Oct 12, 2007
1 parent f26e51f commit bfab36e
Show file tree
Hide file tree
Showing 9 changed files with 181 additions and 53 deletions.
4 changes: 3 additions & 1 deletion Documentation/filesystems/ntfs.txt
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ raiddev /dev/md0
device /dev/hda5
raid-disk 0
device /dev/hdb1
raid-disl 1
raid-disk 1

For linear raid, just change the raid-level above to "raid-level linear", for
mirrors, change it to "raid-level 1", and for stripe sets with parity, change
Expand Down Expand Up @@ -457,6 +457,8 @@ ChangeLog

Note, a technical ChangeLog aimed at kernel hackers is in fs/ntfs/ChangeLog.

2.1.29:
- Fix a deadlock when mounting read-write.
2.1.28:
- Fix a deadlock.
2.1.27:
Expand Down
12 changes: 12 additions & 0 deletions fs/ntfs/ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@ ToDo/Notes:
happen is unclear however so it is worth waiting until someone hits
the problem.

2.1.29 - Fix a deadlock at mount time.

- During mount the VFS holds s_umount lock on the superblock. So when
we try to empty the journal $LogFile contents by calling
ntfs_attr_set() when the machine does not have much memory and the
journal is large ntfs_attr_set() results in the VM trying to balance
dirty pages which in turn tries to that the s_umount lock and thus we
get a deadlock. The solution is to not use ntfs_attr_set() and
instead do the zeroing by hand at the block level rather than page
cache level.
- Fix sparse warnings.

2.1.28 - Fix a deadlock.

- Fix deadlock in fs/ntfs/inode.c::ntfs_put_inode(). Thanks to Sergey
Expand Down
2 changes: 1 addition & 1 deletion fs/ntfs/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ ntfs-objs := aops.o attrib.o collate.o compress.o debug.o dir.o file.o \
index.o inode.o mft.o mst.o namei.o runlist.o super.o sysctl.o \
unistr.o upcase.o

EXTRA_CFLAGS = -DNTFS_VERSION=\"2.1.28\"
EXTRA_CFLAGS = -DNTFS_VERSION=\"2.1.29\"

ifeq ($(CONFIG_NTFS_DEBUG),y)
EXTRA_CFLAGS += -DDEBUG
Expand Down
22 changes: 11 additions & 11 deletions fs/ntfs/aops.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* aops.c - NTFS kernel address space operations and page cache handling.
* Part of the Linux-NTFS project.
*
* Copyright (c) 2001-2006 Anton Altaparmakov
* Copyright (c) 2001-2007 Anton Altaparmakov
* Copyright (c) 2002 Richard Russon
*
* This program/include file is free software; you can redistribute it and/or
Expand Down Expand Up @@ -396,7 +396,7 @@ static int ntfs_readpage(struct file *file, struct page *page)
loff_t i_size;
struct inode *vi;
ntfs_inode *ni, *base_ni;
u8 *kaddr;
u8 *addr;
ntfs_attr_search_ctx *ctx;
MFT_RECORD *mrec;
unsigned long flags;
Expand Down Expand Up @@ -491,15 +491,15 @@ static int ntfs_readpage(struct file *file, struct page *page)
/* Race with shrinking truncate. */
attr_len = i_size;
}
kaddr = kmap_atomic(page, KM_USER0);
addr = kmap_atomic(page, KM_USER0);
/* Copy the data to the page. */
memcpy(kaddr, (u8*)ctx->attr +
memcpy(addr, (u8*)ctx->attr +
le16_to_cpu(ctx->attr->data.resident.value_offset),
attr_len);
/* Zero the remainder of the page. */
memset(kaddr + attr_len, 0, PAGE_CACHE_SIZE - attr_len);
memset(addr + attr_len, 0, PAGE_CACHE_SIZE - attr_len);
flush_dcache_page(page);
kunmap_atomic(kaddr, KM_USER0);
kunmap_atomic(addr, KM_USER0);
put_unm_err_out:
ntfs_attr_put_search_ctx(ctx);
unm_err_out:
Expand Down Expand Up @@ -1344,7 +1344,7 @@ static int ntfs_writepage(struct page *page, struct writeback_control *wbc)
loff_t i_size;
struct inode *vi = page->mapping->host;
ntfs_inode *base_ni = NULL, *ni = NTFS_I(vi);
char *kaddr;
char *addr;
ntfs_attr_search_ctx *ctx = NULL;
MFT_RECORD *m = NULL;
u32 attr_len;
Expand Down Expand Up @@ -1484,14 +1484,14 @@ static int ntfs_writepage(struct page *page, struct writeback_control *wbc)
/* Shrinking cannot fail. */
BUG_ON(err);
}
kaddr = kmap_atomic(page, KM_USER0);
addr = kmap_atomic(page, KM_USER0);
/* Copy the data from the page to the mft record. */
memcpy((u8*)ctx->attr +
le16_to_cpu(ctx->attr->data.resident.value_offset),
kaddr, attr_len);
addr, attr_len);
/* Zero out of bounds area in the page cache page. */
memset(kaddr + attr_len, 0, PAGE_CACHE_SIZE - attr_len);
kunmap_atomic(kaddr, KM_USER0);
memset(addr + attr_len, 0, PAGE_CACHE_SIZE - attr_len);
kunmap_atomic(addr, KM_USER0);
flush_dcache_page(page);
flush_dcache_mft_record_page(ctx->ntfs_ino);
/* We are done with the page. */
Expand Down
8 changes: 6 additions & 2 deletions fs/ntfs/attrib.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* attrib.c - NTFS attribute operations. Part of the Linux-NTFS project.
*
* Copyright (c) 2001-2006 Anton Altaparmakov
* Copyright (c) 2001-2007 Anton Altaparmakov
* Copyright (c) 2002 Richard Russon
*
* This program/include file is free software; you can redistribute it and/or
Expand Down Expand Up @@ -2500,7 +2500,7 @@ int ntfs_attr_set(ntfs_inode *ni, const s64 ofs, const s64 cnt, const u8 val)
struct page *page;
u8 *kaddr;
pgoff_t idx, end;
unsigned int start_ofs, end_ofs, size;
unsigned start_ofs, end_ofs, size;

ntfs_debug("Entering for ofs 0x%llx, cnt 0x%llx, val 0x%hx.",
(long long)ofs, (long long)cnt, val);
Expand Down Expand Up @@ -2548,6 +2548,8 @@ int ntfs_attr_set(ntfs_inode *ni, const s64 ofs, const s64 cnt, const u8 val)
kunmap_atomic(kaddr, KM_USER0);
set_page_dirty(page);
page_cache_release(page);
balance_dirty_pages_ratelimited(mapping);
cond_resched();
if (idx == end)
goto done;
idx++;
Expand Down Expand Up @@ -2604,6 +2606,8 @@ int ntfs_attr_set(ntfs_inode *ni, const s64 ofs, const s64 cnt, const u8 val)
kunmap_atomic(kaddr, KM_USER0);
set_page_dirty(page);
page_cache_release(page);
balance_dirty_pages_ratelimited(mapping);
cond_resched();
}
done:
ntfs_debug("Done.");
Expand Down
36 changes: 17 additions & 19 deletions fs/ntfs/file.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* file.c - NTFS kernel file operations. Part of the Linux-NTFS project.
*
* Copyright (c) 2001-2006 Anton Altaparmakov
* Copyright (c) 2001-2007 Anton Altaparmakov
*
* This program/include file is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License as published
Expand All @@ -26,7 +26,6 @@
#include <linux/swap.h>
#include <linux/uio.h>
#include <linux/writeback.h>
#include <linux/sched.h>

#include <asm/page.h>
#include <asm/uaccess.h>
Expand Down Expand Up @@ -362,7 +361,7 @@ static inline void ntfs_fault_in_pages_readable(const char __user *uaddr,
volatile char c;

/* Set @end to the first byte outside the last page we care about. */
end = (const char __user*)PAGE_ALIGN((ptrdiff_t __user)uaddr + bytes);
end = (const char __user*)PAGE_ALIGN((unsigned long)uaddr + bytes);

while (!__get_user(c, uaddr) && (uaddr += PAGE_SIZE, uaddr < end))
;
Expand Down Expand Up @@ -532,7 +531,8 @@ static int ntfs_prepare_pages_for_non_resident_write(struct page **pages,
blocksize_bits = vol->sb->s_blocksize_bits;
u = 0;
do {
struct page *page = pages[u];
page = pages[u];
BUG_ON(!page);
/*
* create_empty_buffers() will create uptodate/dirty buffers if
* the page is uptodate/dirty.
Expand Down Expand Up @@ -1291,7 +1291,7 @@ static inline size_t ntfs_copy_from_user(struct page **pages,
size_t bytes)
{
struct page **last_page = pages + nr_pages;
char *kaddr;
char *addr;
size_t total = 0;
unsigned len;
int left;
Expand All @@ -1300,13 +1300,13 @@ static inline size_t ntfs_copy_from_user(struct page **pages,
len = PAGE_CACHE_SIZE - ofs;
if (len > bytes)
len = bytes;
kaddr = kmap_atomic(*pages, KM_USER0);
left = __copy_from_user_inatomic(kaddr + ofs, buf, len);
kunmap_atomic(kaddr, KM_USER0);
addr = kmap_atomic(*pages, KM_USER0);
left = __copy_from_user_inatomic(addr + ofs, buf, len);
kunmap_atomic(addr, KM_USER0);
if (unlikely(left)) {
/* Do it the slow way. */
kaddr = kmap(*pages);
left = __copy_from_user(kaddr + ofs, buf, len);
addr = kmap(*pages);
left = __copy_from_user(addr + ofs, buf, len);
kunmap(*pages);
if (unlikely(left))
goto err_out;
Expand Down Expand Up @@ -1408,26 +1408,26 @@ static inline size_t ntfs_copy_from_user_iovec(struct page **pages,
size_t *iov_ofs, size_t bytes)
{
struct page **last_page = pages + nr_pages;
char *kaddr;
char *addr;
size_t copied, len, total = 0;

do {
len = PAGE_CACHE_SIZE - ofs;
if (len > bytes)
len = bytes;
kaddr = kmap_atomic(*pages, KM_USER0);
copied = __ntfs_copy_from_user_iovec_inatomic(kaddr + ofs,
addr = kmap_atomic(*pages, KM_USER0);
copied = __ntfs_copy_from_user_iovec_inatomic(addr + ofs,
*iov, *iov_ofs, len);
kunmap_atomic(kaddr, KM_USER0);
kunmap_atomic(addr, KM_USER0);
if (unlikely(copied != len)) {
/* Do it the slow way. */
kaddr = kmap(*pages);
copied = __ntfs_copy_from_user_iovec_inatomic(kaddr + ofs,
addr = kmap(*pages);
copied = __ntfs_copy_from_user_iovec_inatomic(addr + ofs,
*iov, *iov_ofs, len);
/*
* Zero the rest of the target like __copy_from_user().
*/
memset(kaddr + ofs + copied, 0, len - copied);
memset(addr + ofs + copied, 0, len - copied);
kunmap(*pages);
if (unlikely(copied != len))
goto err_out;
Expand Down Expand Up @@ -1735,8 +1735,6 @@ static int ntfs_commit_pages_after_write(struct page **pages,
read_unlock_irqrestore(&ni->size_lock, flags);
BUG_ON(initialized_size != i_size);
if (end > initialized_size) {
unsigned long flags;

write_lock_irqsave(&ni->size_lock, flags);
ni->initialized_size = end;
i_size_write(vi, end);
Expand Down
3 changes: 0 additions & 3 deletions fs/ntfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#include "dir.h"
#include "debug.h"
#include "inode.h"
#include "attrib.h"
#include "lcnalloc.h"
#include "malloc.h"
#include "mft.h"
Expand Down Expand Up @@ -2500,8 +2499,6 @@ int ntfs_truncate(struct inode *vi)
/* Resize the attribute record to best fit the new attribute size. */
if (new_size < vol->mft_record_size &&
!ntfs_resident_attr_value_resize(m, a, new_size)) {
unsigned long flags;

/* The resize succeeded! */
flush_dcache_mft_record_page(ctx->ntfs_ino);
mark_mft_record_dirty(ctx->ntfs_ino);
Expand Down
Loading

0 comments on commit bfab36e

Please sign in to comment.