linux-stable.git/fs/nfs/write.c, branch linux-3.9.y

NFS: Ensure that we free the rpc_task after read and write cleanups are done

2013-01-04T17:59:10+00:00

This patch ensures that we free the rpc_task after the cleanup callbacks
are done in order to avoid a deadlock problem that can be triggered if
the callback needs to wait for another workqueue item to complete.

Signed-off-by: Trond Myklebust 
Cc: Weston Andros Adamson 
Cc: Tejun Heo 
Cc: Bruce Fields 
Cc: stable@vger.kernel.org [>= 3.5]

NFS: nfs_migrate_page() does not wait for FS-Cache to finish with a page

2012-12-20T22:12:03+00:00

nfs_migrate_page() does not wait for FS-Cache to finish with a page, probably
leading to the following bad-page-state:

 BUG: Bad page state in process python-bin  pfn:17d39b
 page:ffffea00053649e8 flags:004000000000100c count:0 mapcount:0 mapping:(null)
index:38686 (Tainted: G    B      ---------------- )
 Pid: 31053, comm: python-bin Tainted: G    B      ----------------
2.6.32-71.24.1.el6.x86_64 #1
 Call Trace:
 [] bad_page+0x107/0x160
 [] free_hot_cold_page+0x1c9/0x220
 [] __pagevec_free+0x59/0xb0
 [] ? flush_tlb_others_ipi+0x128/0x130
 [] release_pages+0x21c/0x250
 [] ? remove_migration_pte+0x28a/0x2b0
 [] ? mem_cgroup_get_reclaim_stat_from_page+0x18/0x70
 [] ____pagevec_lru_add+0x167/0x180
 [] __lru_cache_add+0x58/0x70
 [] lru_cache_add_lru+0x21/0x40
 [] putback_lru_page+0x69/0x100
 [] migrate_pages+0x13d/0x5d0
 [] ? ____pagevec_lru_add+0x167/0x180
 [] ? compaction_alloc+0x0/0x370
 [] compact_zone+0x4cc/0x600
 [] ? get_page_from_freelist+0x15c/0x820
 [] ? check_preempt_wakeup+0x1c4/0x3c0
 [] compact_zone_order+0x7e/0xb0
 [] try_to_compact_pages+0x109/0x170
 [] __alloc_pages_nodemask+0x5ed/0x850
 [] ? thread_return+0x4e/0x778
 [] alloc_pages_vma+0x93/0x150
 [] do_huge_pmd_anonymous_page+0x135/0x340
 [] ? rwsem_down_read_failed+0x26/0x30
 [] handle_mm_fault+0x245/0x2b0
 [] do_page_fault+0x123/0x3a0
 [] page_fault+0x25/0x30

nfs_migrate_page() calls nfs_fscache_release_page() which doesn't actually wait
- even if __GFP_WAIT is set.  The reason that doesn't wait is that
fscache_maybe_release_page() might deadlock the allocator as the work threads
writing to the cache may all end up sleeping on memory allocation.

However, I wonder if that is actually a problem.  There are a number of things
I can do to deal with this:

 (1) Make nfs_migrate_page() wait.

 (2) Make fscache_maybe_release_page() honour the __GFP_WAIT flag.

 (3) Set a timeout around the wait.

 (4) Make nfs_migrate_page() return an error if the page is still busy.

For the moment, I'll select (2) and (4).

Signed-off-by: David Howells 
Acked-by: Jeff Layton

NFS: Don't use SetPageError in the NFS writeback code

2012-12-15T22:12:14+00:00

The writeback code is already capable of passing errors back to user space
by means of the open_context->error. In the case of ENOSPC, Neil Brown
is reporting seeing 2 errors being returned.

Neil writes:

"e.g. if /mnt2/ if an nfs mounted filesystem that has no space then

strace dd if=/dev/zero conv=fsync >> /mnt2/afile count=1

reported Input/output error and the relevant parts of the strace output are:

write(1, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 512) = 512
fsync(1)                                = -1 EIO (Input/output error)
close(1)                                = -1 ENOSPC (No space left on device)"

Neil then shows that the duplication of error messages appears to be due to
the use of the PageError() mechanism, which causes filemap_fdatawait_range
to return the extra EIO. The regression was introduced by
commit 7b281ee026552f10862b617a2a51acf49c829554 (NFS: fsync() must exit
with an error if page writeback failed).

Fix this by removing the call to SetPageError(), and just relying on
open_context->error reporting the ENOSPC back to fsync().

Reported-by: Neil Brown 
Tested-by: Neil Brown 
Signed-off-by: Trond Myklebust 
Cc: stable@vger.kernel.org [3.6+]

Merge branch 'bugfixes' into nfs-for-next

2012-12-11T14:16:26+00:00

nfs: don't extend writes to cover entire page if pagecache is invalid

2012-12-11T14:14:51+00:00

Jian reported that the following sequence would leave "testfile" with
corrupt data:

    # mount localhost:/export /mnt/nfs/ -o vers=3
    # echo abc > /mnt/nfs/testfile; echo def >> /export/testfile; echo ghi >> /mnt/nfs/testfile
    # cat -v /export/testfile
    abc
    ^@^@^@^@ghi

While there's no locking involved here, the operations are serialized,
so CTO should prevent corruption.

The first write to the file is fine and writes 4 bytes. The file is then
extended on the server. When it's reopened a GETATTR is issued and the
size change is noticed. This causes NFS_INO_INVALID_DATA to be set on
the file. Because the file is opened for write only,
nfs_want_read_modify_write() returns 0 to nfs_write_begin().
nfs_updatepage then calls nfs_write_pageuptodate() to see if it should
extend the nfs_page to cover the whole page. NFS_INO_INVALID_DATA is
still set on the file at that point, but that flag is ignored and
nfs_pageuptodate erroneously extends the write to cover the whole page,
with the write done on the server side filled in with zeroes.

This patch just has that function check for NFS_INO_INVALID_DATA in
addition to NFS_INO_REVAL_PAGECACHE. This fixes the bug, but looking
over the code, I wonder if we might have a similar bug in
nfs_revalidate_size(). The difference between those two flags is very
subtle, so it seems like we ought to be checking for
NFS_INO_INVALID_DATA in most of the places that we look for
NFS_INO_REVAL_PAGECACHE.

I believe this is regression introduced by commit 8d197a568. The code
did check for NFS_INO_INVALID_DATA prior to that patch.

Original bug report is here:

    https://bugzilla.redhat.com/show_bug.cgi?id=885743

Cc:  # 3.5+
Reported-by: Jian Li 
Signed-off-by: Jeff Layton 
Signed-off-by: Trond Myklebust

nfs: Fix wrong slab cache in nfs_commit_mempool

2012-11-25T16:59:33+00:00

The slab cache in nfs_commit_mempool is wrong, and I think it is just a slip.
I tested it on a x86-32 machine, the size of nfs_write_header is 544, and
the size of nfs_commit_data is 408, so it works fine. It is also true that
sizeof(struct nfs_write_header) > sizeof(struct nfs_commit_data) on other
platforms in my opinoin. Just fix it.

Signed-off-by: Yanchuan Nian 
Signed-off-by: Trond Myklebust

NFS: Remove BUG_ON() calls from the generic writeback code

2012-11-04T19:43:39+00:00

...and ensure that we set the return value for nfs_page_async_flush()
to zero! (Reported-by: Dros Adamson)

Signed-off-by: Trond Myklebust

NFS: Fix fdatasync/fsync() when confronted with a server reboot

2012-09-28T20:03:05+00:00

If the server reboots before it can commit the unstable writes to disk,
then nfs_commit_release_pages() will detect this when it compares the
verifier returned by COMMIT to the one returned by WRITE. When this
happens, the client needs to resend those writes in order to guarantee
that they make it to stable storage.

This patch adds a signalling mechanism to notify fsync() that it
needs to retry all writes before it can exit.

Signed-off-by: Trond Myklebust

NFS: Clean up helper function nfs4_select_rw_stateid()

2012-09-28T20:03:04+00:00

We want to be able to pass on the information that the page was not
dirtied under a lock. Instead of adding a flag parameter, do this
by passing a pointer to a 'struct nfs_lock_owner' that may be NULL.

Also reuse this structure in struct nfs_lock_context to carry the
fl_owner_t and pid_t.

Signed-off-by: Trond Myklebust

nfs: tear down caches in nfs_init_writepagecache when allocation fails

2012-08-02T21:36:07+00:00

...and ensure that we tear down the nfs_commit_data cache too when
unloading the module.

Cc: Bryan Schumaker 
Cc: stable@vger.kernel.org
Signed-off-by: Jeff Layton 
Signed-off-by: Trond Myklebust