mirror of
https://github.com/torvalds/linux.git
synced 2025-12-01 07:26:02 +07:00
Commit20d72b00ca("netfs: Fix the request's work item to not require a ref") modified netfs_alloc_request() to initialize the reference counter to 2 instead of 1. The rationale was that the requet's "work" would release the second reference after completion (via netfs_{read,write}_collection_worker()). That works most of the time if all goes well. However, it leaks this additional reference if the request is released before the I/O operation has been submitted: the error code path only decrements the reference counter once and the work item will never be queued because there will never be a completion. This has caused outages of our whole server cluster today because tasks were blocked in netfs_wait_for_outstanding_io(), leading to deadlocks in Ceph (another bug that I will address soon in another patch). This was caused by a netfs_pgpriv2_begin_copy_to_cache() call which failed in fscache_begin_write_operation(). The leaked netfs_io_request was never completed, leaving `netfs_inode.io_count` with a positive value forever. All of this is super-fragile code. Finding out which code paths will lead to an eventual completion and which do not is hard to see: - Some functions like netfs_create_write_req() allocate a request, but will never submit any I/O. - netfs_unbuffered_read_iter_locked() calls netfs_unbuffered_read() and then netfs_put_request(); however, netfs_unbuffered_read() can also fail early before submitting the I/O request, therefore another netfs_put_request() call must be added there. A rule of thumb is that functions that return a `netfs_io_request` do not submit I/O, and all of their callers must be checked. For my taste, the whole netfs code needs an overhaul to make reference counting easier to understand and less fragile & obscure. But to fix this bug here and now and produce a patch that is adequate for a stable backport, I tried a minimal approach that quickly frees the request object upon early failure. I decided against adding a second netfs_put_request() each time because that would cause code duplication which obscures the code further. Instead, I added the function netfs_put_failed_request() which frees such a failed request synchronously under the assumption that the reference count is exactly 2 (as initially set by netfs_alloc_request() and never touched), verified by a WARN_ON_ONCE(). It then deinitializes the request object (without going through the "cleanup_work" indirection) and frees the allocation (with RCU protection to protect against concurrent access by netfs_requests_seq_start()). All code paths that fail early have been changed to call netfs_put_failed_request() instead of netfs_put_request(). Additionally, I have added a netfs_put_request() call to netfs_unbuffered_read() as explained above because the netfs_put_failed_request() approach does not work there. Fixes:20d72b00ca("netfs: Fix the request's work item to not require a ref") Signed-off-by: Max Kellermann <max.kellermann@ionos.com> Signed-off-by: David Howells <dhowells@redhat.com> cc: Paulo Alcantara <pc@manguebit.org> cc: netfs@lists.linux.dev cc: linux-fsdevel@vger.kernel.org cc: stable@vger.kernel.org Signed-off-by: Christian Brauner <brauner@kernel.org>
187 lines
5.3 KiB
C
187 lines
5.3 KiB
C
// SPDX-License-Identifier: GPL-2.0-or-later
|
|
/* Unbuffered and direct write support.
|
|
*
|
|
* Copyright (C) 2023 Red Hat, Inc. All Rights Reserved.
|
|
* Written by David Howells (dhowells@redhat.com)
|
|
*/
|
|
|
|
#include <linux/export.h>
|
|
#include <linux/uio.h>
|
|
#include "internal.h"
|
|
|
|
/*
|
|
* Perform an unbuffered write where we may have to do an RMW operation on an
|
|
* encrypted file. This can also be used for direct I/O writes.
|
|
*/
|
|
ssize_t netfs_unbuffered_write_iter_locked(struct kiocb *iocb, struct iov_iter *iter,
|
|
struct netfs_group *netfs_group)
|
|
{
|
|
struct netfs_io_request *wreq;
|
|
unsigned long long start = iocb->ki_pos;
|
|
unsigned long long end = start + iov_iter_count(iter);
|
|
ssize_t ret, n;
|
|
size_t len = iov_iter_count(iter);
|
|
bool async = !is_sync_kiocb(iocb);
|
|
|
|
_enter("");
|
|
|
|
/* We're going to need a bounce buffer if what we transmit is going to
|
|
* be different in some way to the source buffer, e.g. because it gets
|
|
* encrypted/compressed or because it needs expanding to a block size.
|
|
*/
|
|
// TODO
|
|
|
|
_debug("uw %llx-%llx", start, end);
|
|
|
|
wreq = netfs_create_write_req(iocb->ki_filp->f_mapping, iocb->ki_filp, start,
|
|
iocb->ki_flags & IOCB_DIRECT ?
|
|
NETFS_DIO_WRITE : NETFS_UNBUFFERED_WRITE);
|
|
if (IS_ERR(wreq))
|
|
return PTR_ERR(wreq);
|
|
|
|
wreq->io_streams[0].avail = true;
|
|
trace_netfs_write(wreq, (iocb->ki_flags & IOCB_DIRECT ?
|
|
netfs_write_trace_dio_write :
|
|
netfs_write_trace_unbuffered_write));
|
|
|
|
{
|
|
/* If this is an async op and we're not using a bounce buffer,
|
|
* we have to save the source buffer as the iterator is only
|
|
* good until we return. In such a case, extract an iterator
|
|
* to represent as much of the the output buffer as we can
|
|
* manage. Note that the extraction might not be able to
|
|
* allocate a sufficiently large bvec array and may shorten the
|
|
* request.
|
|
*/
|
|
if (user_backed_iter(iter)) {
|
|
n = netfs_extract_user_iter(iter, len, &wreq->buffer.iter, 0);
|
|
if (n < 0) {
|
|
ret = n;
|
|
goto error_put;
|
|
}
|
|
wreq->direct_bv = (struct bio_vec *)wreq->buffer.iter.bvec;
|
|
wreq->direct_bv_count = n;
|
|
wreq->direct_bv_unpin = iov_iter_extract_will_pin(iter);
|
|
} else {
|
|
/* If this is a kernel-generated async DIO request,
|
|
* assume that any resources the iterator points to
|
|
* (eg. a bio_vec array) will persist till the end of
|
|
* the op.
|
|
*/
|
|
wreq->buffer.iter = *iter;
|
|
}
|
|
}
|
|
|
|
__set_bit(NETFS_RREQ_USE_IO_ITER, &wreq->flags);
|
|
if (async)
|
|
__set_bit(NETFS_RREQ_OFFLOAD_COLLECTION, &wreq->flags);
|
|
|
|
/* Copy the data into the bounce buffer and encrypt it. */
|
|
// TODO
|
|
|
|
/* Dispatch the write. */
|
|
__set_bit(NETFS_RREQ_UPLOAD_TO_SERVER, &wreq->flags);
|
|
if (async)
|
|
wreq->iocb = iocb;
|
|
wreq->len = iov_iter_count(&wreq->buffer.iter);
|
|
ret = netfs_unbuffered_write(wreq, is_sync_kiocb(iocb), wreq->len);
|
|
if (ret < 0) {
|
|
_debug("begin = %zd", ret);
|
|
goto out;
|
|
}
|
|
|
|
if (!async) {
|
|
ret = netfs_wait_for_write(wreq);
|
|
if (ret > 0)
|
|
iocb->ki_pos += ret;
|
|
} else {
|
|
ret = -EIOCBQUEUED;
|
|
}
|
|
|
|
out:
|
|
netfs_put_request(wreq, netfs_rreq_trace_put_return);
|
|
return ret;
|
|
|
|
error_put:
|
|
netfs_put_failed_request(wreq);
|
|
return ret;
|
|
}
|
|
EXPORT_SYMBOL(netfs_unbuffered_write_iter_locked);
|
|
|
|
/**
|
|
* netfs_unbuffered_write_iter - Unbuffered write to a file
|
|
* @iocb: IO state structure
|
|
* @from: iov_iter with data to write
|
|
*
|
|
* Do an unbuffered write to a file, writing the data directly to the server
|
|
* and not lodging the data in the pagecache.
|
|
*
|
|
* Return:
|
|
* * Negative error code if no data has been written at all of
|
|
* vfs_fsync_range() failed for a synchronous write
|
|
* * Number of bytes written, even for truncated writes
|
|
*/
|
|
ssize_t netfs_unbuffered_write_iter(struct kiocb *iocb, struct iov_iter *from)
|
|
{
|
|
struct file *file = iocb->ki_filp;
|
|
struct address_space *mapping = file->f_mapping;
|
|
struct inode *inode = mapping->host;
|
|
struct netfs_inode *ictx = netfs_inode(inode);
|
|
ssize_t ret;
|
|
loff_t pos = iocb->ki_pos;
|
|
unsigned long long end = pos + iov_iter_count(from) - 1;
|
|
|
|
_enter("%llx,%zx,%llx", pos, iov_iter_count(from), i_size_read(inode));
|
|
|
|
if (!iov_iter_count(from))
|
|
return 0;
|
|
|
|
trace_netfs_write_iter(iocb, from);
|
|
netfs_stat(&netfs_n_wh_dio_write);
|
|
|
|
ret = netfs_start_io_direct(inode);
|
|
if (ret < 0)
|
|
return ret;
|
|
ret = generic_write_checks(iocb, from);
|
|
if (ret <= 0)
|
|
goto out;
|
|
ret = file_remove_privs(file);
|
|
if (ret < 0)
|
|
goto out;
|
|
ret = file_update_time(file);
|
|
if (ret < 0)
|
|
goto out;
|
|
if (iocb->ki_flags & IOCB_NOWAIT) {
|
|
/* We could block if there are any pages in the range. */
|
|
ret = -EAGAIN;
|
|
if (filemap_range_has_page(mapping, pos, end))
|
|
if (filemap_invalidate_inode(inode, true, pos, end))
|
|
goto out;
|
|
} else {
|
|
ret = filemap_write_and_wait_range(mapping, pos, end);
|
|
if (ret < 0)
|
|
goto out;
|
|
}
|
|
|
|
/*
|
|
* After a write we want buffered reads to be sure to go to disk to get
|
|
* the new data. We invalidate clean cached page from the region we're
|
|
* about to write. We do this *before* the write so that we can return
|
|
* without clobbering -EIOCBQUEUED from ->direct_IO().
|
|
*/
|
|
ret = filemap_invalidate_inode(inode, true, pos, end);
|
|
if (ret < 0)
|
|
goto out;
|
|
end = iocb->ki_pos + iov_iter_count(from);
|
|
if (end > ictx->zero_point)
|
|
ictx->zero_point = end;
|
|
|
|
fscache_invalidate(netfs_i_cookie(ictx), NULL, i_size_read(inode),
|
|
FSCACHE_INVAL_DIO_WRITE);
|
|
ret = netfs_unbuffered_write_iter_locked(iocb, from, NULL);
|
|
out:
|
|
netfs_end_io_direct(inode);
|
|
return ret;
|
|
}
|
|
EXPORT_SYMBOL(netfs_unbuffered_write_iter);
|