netfs: Miscellaneous fixes
Subsystems (8)
David Howells
maintainerv3 cover netfs: Miscellaneous fixes
Hi Christian,
Here are some miscellaneous fixes for netfslib. I separated them from my netfs-next branch. Various Sashiko review comments[1][2][3] are addressed:
(1) Fix the decision whether to disallow write-streaming due to fscache use.
(2) Fix netfs_create_write_req() to better handle async cache object creation.
(3) Fix a double fput in cachefiles_create_tmpfile().
(4) Fix alteration of S_KERNEL_FILE inode flag without holding inode lock.
(5) Fix a potential mathematical underflow in iov_iter_extract_xarray_pages() and make it return 0 and free the array if no pages could be extracted.
(6) Fix a missing alloc failure check in iov_iter_extract_bvec_pages().
(7) Fix iov_iter_extract_user_pages() so that it doesn't leak the pages array if it returns an error or 0 (inasmuch as the leak is really in the callers).
(8) Remove an unused variable in kunit_iov_iter.c.
(9) Fix extract_xarray_to_sg() to calculate folio offset correctly.
(10) Fix a kdoc comment.
(11) Replace the netfs_inode::wb_lock mutex with a bit lock so that the lock can be passed to the collector so that multiple asynchronous writebacks won't interfere with each other.
(12) Fix writeback error handling to go through writeback_iter() so that it can clean up its state.
(13) Fix ENOMEM handling in writeback to clean up the current folio if we can't allocate a rolling buffer segment.
(14) Fix unbuffered/DIO write retry for filesystems that don't have a
->prepare_write() method.
The patches can also be found here:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=netfs-fixes
Thanks,
David
[1] https://sashiko.dev/#/patchset/20260608145432.681865-1-dhowells%40redhat.com
[2] https://sashiko.dev/#/patchset/20260616100821.2062304-1-dhowells%40redhat.com
[3] https://sashiko.dev/#/patchset/20260619140646.2633762-1-dhowells%40redhat.com
[4] https://sashiko.dev/#/patchset/20260624115737.2964520-1-dhowells%40redhat.com
Changes
=======
ver #3)
- Addressed more Sashiko comments[4]:
- Applied the disallow write-streaming fix to netfs_write_begin() also.
- Improve the synchronisation with async cache object creation in netfs_create_write_req().
- Fix netfs_writepages() so it doesn't both redirty and kill folios (and double unlock) if it can't create the request.
- Move the cleanup of the folio on ENOMEM from netfs_write_folio() to netfs_writepages() as writethrough shouldn't do this.
- Alter the unbuffered/DIO write retry fix so that it sets the IN_PROGRESS flag. Also made it clear the MADE_PROGRESS flag and update wreq->transferred.
ver #2)
- Addressed various Sashiko comments[3]:
- Changed some kfree() to kvfree().
- Added some writeback error handling patches.
- Fixed DIO write retry.
ver #1)
- Added a fix for S_KERNEL_FILE in cachefiles_bury_object().
- Fixed how iov_iter_extract_*_pages() deals with *pages when returning 0 or error.
- Fixed the replacement of wb_lock with bit lock:
- Only release the bit lock for writeback, writethrough and write-single, not for PG_private_2-based I/O, which doesn't take the lock.
- In netfs_writeback_single(), need to redirty the inode if can't get the lock.
David Howells (15):
netfs: Fix decision whether to disallow write-streaming due to fscache
use
netfs: Fix netfs_create_write_req() to handle async cache object
creation
cachefiles: Fix double fput
cachefiles: Fix file burial to take lock when unsetting S_KERNEL_FILE
iov_iter: Fix potential underflow in iov_iter_extract_xarray_pages()
iov_iter: Fix missing alloc fail check in
iov_iter_extract_bvec_pages()
iov_iter: Fix a memory leak in iov_iter_extract_user_pages()
iov_iter: Remove unused variable in kunit_iov_iter.c
scatterlist: Fix offset in folio calc in extract_xarray_to_sg()
netfs: Fix kdoc warning
netfs: Replace wb_lock with a bit lock for asynchronicity
netfs: Fix writethrough to use collection offload
netfs: Fix writeback error handling
netfs: Fix folio state after ENOMEM whilst under writeback iteration
netfs: Fix DIO write retry for filesystems without a ->prepare_write()
fs/afs/symlink.c | 4 +- fs/cachefiles/namei.c | 3 +- fs/netfs/buffered_read.c | 2 +- fs/netfs/buffered_write.c | 2 +- fs/netfs/direct_write.c | 18 +++----- fs/netfs/internal.h | 12 +++++ fs/netfs/locking.c | 95 ++++++++++++++++++++++++++++++++++++++ fs/netfs/write_collect.c | 10 ++++ fs/netfs/write_issue.c | 55 ++++++++++------------ include/linux/netfs.h | 13 ++++-- lib/iov_iter.c | 20 +++++++- lib/scatterlist.c | 1 + lib/tests/kunit_iov_iter.c | 5 +- 13 files changed, 185 insertions(+), 55 deletions(-)
David Howells
maintainerPATCH v3 1/15 netfs: Fix decision whether to disallow write-streaming due to fscache use
netfs_perform_write() buffers data by writing it into the pagecache for later writeback. If the folio it wants to write to isn't present, it uses "write streaming" in which is will store partial data in a non-uptodate, but dirty folio.
However, when fscache is in use, this is a potential problem as writes to the cache have to be aligned to the cache backend's DIO granularity, and so netfs_perform_write() attempts to suppress write-streaming in such a case, requiring the folio content to be fetched first unless the entire folio is going to be overwritten. This allows the content to be written to the cache too.
Unfortunately, the test netfs_perform_write() uses isn't correct because it doesn't take into account the fact that the object lookup is asynchronous and farmed off to a work queue, so there's a short window in which the cache is doing a lookup but the test fails because the answer is undefined.
This can be triggered by the generic/464 xfstest, and causes a warning to be emitted in cachefiles (in code not yet upstream) because it sees a write that doesn't have its bounds rounded out to DIO alignment.
Fix this by changing the condition to whether FSCACHE_COOKIE_IS_CACHING is set on a cookie rather than whether the cookie is marked enabled. Note that this is really just a hint as to whether we allow write streaming or not and no other aspects of the cookie or cache object are accessed.
Also apply the same fix to netfs_write_begin().
--- fs/netfs/buffered_read.c | 2 +- fs/netfs/buffered_write.c | 2 +- fs/netfs/internal.h | 12 ++++++++++++ 3 files changed, 14 insertions(+), 2 deletions(-)
fs/netfs/buffered_read.c +1 -1
@@ -659,7 +659,7 @@ int netfs_write_begin(struct netfs_inode *ctx,* within the cache granule containing the EOF, in which case we need * to preload the granule. */ -if (!netfs_is_cache_enabled (ctx) && +if (!netfs_is_cache_maybe_enabled (ctx) &&netfs_skip_folio_read (folio, pos, len, false)) {netfs_stat (&netfs_n_rh_write_zskip); goto have_folio_no_wait;
fs/netfs/buffered_write.c +1 -1
@@ -277,7 +277,7 @@ ssize_t netfs_perform_write(struct kiocb *iocb, struct iov_iter *iter,* caching service temporarily because the backing store got * culled. */ -if (netfs_is_cache_enabled (ctx)) { +if (netfs_is_cache_maybe_enabled (ctx)) {if (finfo) {netfs_stat (&netfs_n_wh_wstream_conflict); goto flush_content;
fs/netfs/internal.h +12 -0
@@ -239,6 +239,18 @@ static inline bool netfs_is_cache_enabled(struct netfs_inode *ctx)#endif } +static inline bool netfs_is_cache_maybe_enabled (struct netfs_inode *ctx) +{ +#if IS_ENABLED (CONFIG_FSCACHE ) +struct fscache_cookie *cookie = ctx->cache; + +return fscache_cookie_valid (cookie) && +test_bit (FSCACHE_COOKIE_IS_CACHING , &cookie->flags); +#else +return false; +#endif +} +/* * Get a ref on a netfs group attached to a dirty page (e.g. a ceph snap). */
David Howells
maintainerPATCH v3 2/15 netfs: Fix netfs_create_write_req() to handle async cache object creation
In reply to David Howellsnetfs_create_write_req() will skip caching if the fscache cookie is disabled, but this is a problem because async cache object creation might not have got far enough yet that has been enabled - thereby causing the call to fscache_begin_write_operation() to be skipped.
Fix this by removing the checks on the cookie and delegating this to fscache_begin_write_operation().
--- fs/netfs/write_issue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/netfs/write_issue.c +1 -1
@@ -106,7 +106,7 @@ struct netfs_io_request *netfs_create_write_req(struct address_space *mapping,_enter ("R=%x" , wreq->debug_id); ictx =netfs_inode (wreq->inode); -if (is_cacheable &&netfs_is_cache_enabled (ictx)) +if (is_cacheable)fscache_begin_write_operation (&wreq->cache_resources,netfs_i_cookie (ictx));if (rolling_buffer_init (&wreq->buffer, wreq->debug_id,ITER_SOURCE ) <0 ) goto nomem;
David Howells
maintainerPATCH v3 3/15 cachefiles: Fix double fput
In reply to David HowellsFix a double fput() in error handling in cachefiles_create_tmpfile().
--- fs/cachefiles/namei.c | 1 - 1 file changed, 1 deletion(-)
fs/cachefiles/namei.c +0 -1
@@ -467,7 +467,6 @@ struct file *cachefiles_create_tmpfile(struct cachefiles_object *object) ret = -EINVAL ;if (unlikely (!file->f_op->read_iter) ||unlikely (!file->f_op->write_iter)) { -fput (file);pr_notice ("Cache does not support read_iter and write_iter\n" ); goto err_unuse; }
David Howells
maintainerPATCH v3 4/15 cachefiles: Fix file burial to take lock when unsetting S_KERNEL_FILE
In reply to David HowellsFix cachefiles_bury_object() to lock the inode of the file being buried whilst it unsets the S_KERNEL_FILE flag.
--- fs/cachefiles/namei.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/cachefiles/namei.c +1 -1
@@ -375,7 +375,7 @@ int cachefiles_bury_object(struct cachefiles_cache *cache, "Rename failed with error %d", ret); } -__cachefiles_unmark_inode_in_use (object,d_inode (rep)); +cachefiles_do_unmark_inode_in_use (object,d_inode (rep));end_renaming (&rd);_leave (" = 0" );return 0 ;
Re: [PATCH v3 04/15] cachefiles: Fix file burial to take lock when unsetting S_KERNEL_FILE
Quoted 2 lines · David Howells
Fix
cachefiles_bury_object()to lock the inode of the file being buried whilst it unsets theS_KERNEL_FILEflag.
It might be worth adding
lockdep_assert_held(&inode->i_rwsem);
in __cachefiles_unmark_inode_in_use()
and __cachefiles_mark_inode_in_use()
though all uses look safe after this patch.
Thanks,
NeilBrown
Quoted 28 lines
Fixes 07a90e97400c ("cachefiles: Implement culling daemon commands")Signed-off-by David Howells <[email protected]>cc Paulo Alcantara <[email protected]>cc NeilBrown <[email protected]>--- fs/cachefiles/namei.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/cachefiles/namei.c +1
-1@@ -375,7 +375,7 @@ int cachefiles_bury_object(struct cachefiles_cache *cache, "Rename failed with error %d", ret); } -__cachefiles_unmark_inode_in_use (object,d_inode (rep)); +cachefiles_do_unmark_inode_in_use (object,d_inode (rep));end_renaming (&rd);_leave (" = 0" );return 0 ;
David Howells
maintainerPATCH v3 5/15 iov_iter: Fix potential underflow in iov_iter_extract_xarray_pages()
In reply to David HowellsIn iov_iter_extract_xarray_pages(), if no pages are extracted because there's a hole (or something otherwise unextractable) in the xarray, then the calculation of maxsize at the end can go wrong if the starting offset is not zero.
Fix this by returning 0 in such a case and freeing the page array if allocated here rather than being passed in.
Note that in the near future, ITER_XARRAY should be removed.
--- lib/iov_iter.c | 9 +++++++++ 1 file changed, 9 insertions(+)
lib/iov_iter.c +9 -0
@@ -1568,6 +1568,7 @@ static ssize_t iov_iter_extract_xarray_pages(struct iov_iter *i,struct folio *folio;unsigned int nr =0 , offset;loff_t pos = i->xarray_start + i->iov_offset; +bool will_alloc = !*pages;XA_STATE (xas, i->xarray, pos >>PAGE_SHIFT ); offset = pos & ~PAGE_MASK ;
@@ -1595,6 +1596,14 @@ static ssize_t iov_iter_extract_xarray_pages(struct iov_iter *i, }rcu_read_unlock (); +if (!nr) { +if (will_alloc) { +kvfree (*pages); + *pages =NULL ; + } +return 0 ; + } + maxsize =min_t (size_t, nr *PAGE_SIZE - offset, maxsize);iov_iter_advance (i, maxsize);return maxsize;
Re: [PATCH v3 05/15] iov_iter: Fix potential underflow in iov_iter_extract_xarray_pages()
Looks good:
David Howells
maintainerPATCH v3 6/15 iov_iter: Fix missing alloc fail check in iov_iter_extract_bvec_pages()
In reply to David HowellsFix iov_iter_extract_bvec_pages() to check if want_pages_array() fails and, if so, return -ENOMEM appropriately.
--- lib/iov_iter.c | 2 ++ 1 file changed, 2 insertions(+)
lib/iov_iter.c +2 -0
@@ -1637,6 +1637,8 @@ static ssize_t iov_iter_extract_bvec_pages(struct iov_iter *i, bi.bi_bvec_done = skip; maxpages =want_pages_array (pages, maxsize, skip, maxpages); +if (!maxpages) +return -ENOMEM ;while (bi.bi_size && bi.bi_idx < i->nr_segs) {struct bio_vec bv =bvec_iter_bvec (i->bvec, bi);
Re: [PATCH v3 06/15] iov_iter: Fix missing alloc fail check in iov_iter_extract_bvec_pages()
Looks good:
David Howells
maintainerPATCH v3 7/15 iov_iter: Fix a memory leak in iov_iter_extract_user_pages()
In reply to David HowellsThere's a potential memory leak in callers of iov_iter_extract_user_pages() whereby if a pages array is allocated in function, it isn't freed before returning of an error or 0.
Now, it's not a leak per se in iov_iter_extract_user_pages() as, if an array is allocated, it's returned through *pages, so it's incumbent on the caller to free it. However, not all callers do.
Fix this by freeing the table and clearing *pages before returning an error or 0. Note that iov_iter_extract_pages() and its subfunctions are allowed to return 0 without returning an array (for instance if the iterator count is 0).
--- lib/iov_iter.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
lib/iov_iter.c +8 -1
@@ -1756,6 +1756,7 @@ static ssize_t iov_iter_extract_user_pages(struct iov_iter *i,unsigned long addr;unsigned int gup_flags =0 ;size_t offset; +bool will_alloc = !*pages;int res;if (i->data_source ==ITER_DEST )
@@ -1772,8 +1773,14 @@ static ssize_t iov_iter_extract_user_pages(struct iov_iter *i,if (!maxpages)return -ENOMEM ; res =pin_user_pages_fast (addr, maxpages, gup_flags, *pages); -if (unlikely (res <=0 )) +if (unlikely (res <=0 )) { +if (will_alloc) { +kvfree (*pages); + *pages =NULL ; + }return res; + } + maxsize =min_t (size_t, maxsize, res *PAGE_SIZE - offset);iov_iter_advance (i, maxsize);return maxsize;
Re: [PATCH v3 07/15] iov_iter: Fix a memory leak in iov_iter_extract_user_pages()
Looks good:
David Howells
maintainerPATCH v3 8/15 iov_iter: Remove unused variable in kunit_iov_iter.c
In reply to David HowellsRemove the no longer used variable 'b' from iov_kunit_copy_to_bvec(). The variable is initialised and incremented, but nothing now makes use of the value.
--- lib/tests/kunit_iov_iter.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
lib/tests/kunit_iov_iter.c +2 -3
@@ -275,7 +275,7 @@ static void __init iov_kunit_copy_to_bvec(struct kunit *test)struct page **spages, **bpages;u8 *scratch, *buffer;size_t bufsize, npages, size, copied; -int i, b, patt; +int i, patt; bufsize =0x100000 ; npages = bufsize /PAGE_SIZE ;
@@ -298,10 +298,9 @@ static void __init iov_kunit_copy_to_bvec(struct kunit *test)KUNIT_EXPECT_EQ (test, iter.nr_segs,0 );/* Build the expected image in the scratch buffer. */ - b =0 ; patt =0 ;memset (scratch,0 , bufsize); -for (pr = bvec_test_ranges; pr->from >=0 ; pr++, b++) { +for (pr = bvec_test_ranges; pr->from >=0 ; pr++) {u8 *p = scratch + pr->page *PAGE_SIZE ;for (i = pr->from; i < pr->to; i++)
Re: [PATCH v3 08/15] iov_iter: Remove unused variable in kunit_iov_iter.c
Looks good:
David Howells
maintainerPATCH v3 9/15 scatterlist: Fix offset in folio calc in extract_xarray_to_sg()
In reply to David HowellsFix the calculation of the offset in the folio being extracted in extract_xarray_to_sg().
Note that in the near future, ITER_XARRAY should be removed.
--- lib/scatterlist.c | 1 + 1 file changed, 1 insertion(+)
lib/scatterlist.c +1 -0
@@ -1366,6 +1366,7 @@ static ssize_t extract_xarray_to_sg(struct iov_iter *iter, sg_max--; maxsize -= len; + start += len; ret += len;if (maxsize <=0 || sg_max ==0 )break ;
Re: [PATCH v3 09/15] scatterlist: Fix offset in folio calc in extract_xarray_to_sg()
Looks good:
David Howells
maintainerPATCH v3 10/15 netfs: Fix kdoc warning
In reply to David HowellsFix a kdoc warning due to a misnamed parameter in the description.
--- include/linux/netfs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/netfs.h +1 -1
@@ -753,7 +753,7 @@ static inline void netfs_inode_init(struct netfs_inode *ctx,/** * netfs_resize_file - Note that a file got resized -* @ctx: The netfs inode being resized +* @ictx: The netfs inode being resized * @new_i_size: The new file size * @changed_on_server: The change was applied to the server *
David Howells
maintainerPATCH v3 11/15 netfs: Replace wb_lock with a bit lock for asynchronicity
In reply to David HowellsThe netfs_inode::wb_lock mutex is used to prevent multiple simultaneous writebacks from fighting each other (a writeback thread will write multiple discontiguous regions within the same request). The mutex, however, only serialises the issuing of subrequests; it doesn't serialise the collection of results, and, in particular, the updating of file size information and fscache populatedness data.
Unfortunately, the mutex cannot be held around the entire process as it has to be unlocked in the same thread in which it is locked - and we don't want to hold up the allocator whilst we complete the writeback.
Fix this by replacing the mutex with a bit flag and a list of lock waiters so that the lock can be dropped in the collector thread after collection is complete.
--- fs/afs/symlink.c | 4 +- fs/netfs/locking.c | 95 ++++++++++++++++++++++++++++++++++++++++ fs/netfs/write_collect.c | 10 +++++ fs/netfs/write_issue.c | 37 +++++----------- include/linux/netfs.h | 11 ++++- 5 files changed, 126 insertions(+), 31 deletions(-)
fs/afs/symlink.c +2 -2
@@ -255,11 +255,11 @@ int afs_symlink_writepages(struct address_space *mapping, }if (ret ==0 ) { -mutex_lock (&vnode->netfs.wb_lock); +netfs_wb_begin (&vnode->netfs, false);netfs_free_folioq_buffer (vnode->directory); vnode->directory =NULL ; vnode->directory_size =0 ; -mutex_unlock (&vnode->netfs.wb_lock); +netfs_wb_end (&vnode->netfs); }else if (ret ==1 ) { ret =0 ;/* Skipped write due to lock conflict. */ }
fs/netfs/locking.c +95 -0
104 lines not on this page · raw message
fs/netfs/write_collect.c +10 -0
@@ -408,6 +408,16 @@ bool netfs_write_collection(struct netfs_io_request *wreq)netfs_wake_rreq_flag (wreq,NETFS_RREQ_IN_PROGRESS , netfs_rreq_trace_wake_ip);/* As we cleared NETFS_RREQ_IN_PROGRESS, we acquired its ref. */ +switch (wreq->origin) { +case NETFS_WRITEBACK : +case NETFS_WRITEBACK_SINGLE : +case NETFS_WRITETHROUGH : +netfs_wb_end (ictx); +break ; +default : +break ; + } +if (wreq->iocb) {size_t written =min (wreq->transferred, wreq->len); wreq->iocb->ki_pos += written;
fs/netfs/write_issue.c +10 -27
@@ -551,14 +551,8 @@ int netfs_writepages(struct address_space *mapping,struct folio *folio;int error =0 ; -if (!mutex_trylock (&ictx->wb_lock)) { -if (wbc->sync_mode ==WB_SYNC_NONE ) { -netfs_stat (&netfs_n_wb_lock_skip); -return 0 ; - } -netfs_stat (&netfs_n_wb_lock_wait); -mutex_lock (&ictx->wb_lock); - } +if (!netfs_wb_begin (ictx, wbc->sync_mode ==WB_SYNC_NONE )) +return 0 ;/* Need the first folio to be able to set up the op. */ folio =writeback_iter (mapping, wbc,NULL , &error);
@@ -593,8 +587,6 @@ int netfs_writepages(struct address_space *mapping, }while ((folio =writeback_iter (mapping, wbc, folio, &error)));netfs_end_issue_write (wreq); - -mutex_unlock (&ictx->wb_lock);netfs_wake_collector (wreq);netfs_put_request (wreq, netfs_rreq_trace_put_return);
@@ -604,7 +596,7 @@ int netfs_writepages(struct address_space *mapping, couldnt_start:netfs_kill_dirty_pages (mapping, wbc, folio); out: -mutex_unlock (&ictx->wb_lock); +netfs_wb_end (ictx);_leave (" = %d" , error);return error; }
@@ -618,12 +610,12 @@ struct netfs_io_request *netfs_begin_writethrough(struct kiocb *iocb, size_t lenstruct netfs_io_request *wreq =NULL ;struct netfs_inode *ictx =netfs_inode (file_inode (iocb->ki_filp)); -mutex_lock (&ictx->wb_lock); +netfs_wb_begin (ictx, false); wreq =netfs_create_write_req (iocb->ki_filp->f_mapping, iocb->ki_filp, iocb->ki_pos,NETFS_WRITETHROUGH );if (IS_ERR (wreq)) { -mutex_unlock (&ictx->wb_lock); +netfs_wb_end (ictx);return wreq; }
@@ -685,7 +677,6 @@ int netfs_advance_writethrough(struct netfs_io_request *wreq, struct writeback_cssize_t netfs_end_writethrough (struct netfs_io_request *wreq,struct writeback_control *wbc,struct folio *writethrough_cache) { -struct netfs_inode *ictx =netfs_inode (wreq->inode);ssize_t ret;_enter ("R=%x" , wreq->debug_id);
@@ -699,8 +690,6 @@ ssize_t netfs_end_writethrough(struct netfs_io_request *wreq, struct writeback_cnetfs_end_issue_write (wreq); -mutex_unlock (&ictx->wb_lock); -if (wreq->iocb) ret = -EIOCBQUEUED ;else
@@ -847,15 +836,10 @@ int netfs_writeback_single(struct address_space *mapping,if (WARN_ON_ONCE (!iov_iter_is_folioq (iter)))return -EIO ; -if (!mutex_trylock (&ictx->wb_lock)) { -if (wbc->sync_mode ==WB_SYNC_NONE ) { -/* The VFS will have undirtied the inode. */ -netfs_single_mark_inode_dirty (&ictx->inode); -netfs_stat (&netfs_n_wb_lock_skip); -return 1 ; - } -netfs_stat (&netfs_n_wb_lock_wait); -mutex_lock (&ictx->wb_lock); +if (!netfs_wb_begin (ictx, wbc->sync_mode ==WB_SYNC_NONE )) { +/* The VFS will have undirtied the inode. */ +netfs_single_mark_inode_dirty (&ictx->inode); +return 1 ; } wreq =netfs_create_write_req (mapping,NULL ,0 ,NETFS_WRITEBACK_SINGLE );
@@ -893,7 +877,6 @@ int netfs_writeback_single(struct address_space *mapping,smp_wmb ();/* Write lists before ALL_QUEUED. */ set_bit (NETFS_RREQ_ALL_QUEUED , &wreq->flags); -mutex_unlock (&ictx->wb_lock);netfs_wake_collector (wreq);netfs_put_request (wreq, netfs_rreq_trace_put_return);
@@ -901,7 +884,7 @@ int netfs_writeback_single(struct address_space *mapping,return ret; couldnt_start: -mutex_unlock (&ictx->wb_lock); +netfs_wb_end (ictx);_leave (" = %d" , ret);return ret; }
include/linux/netfs.h +9 -2
@@ -61,14 +61,16 @@ struct netfs_inode {#if IS_ENABLED (CONFIG_FSCACHE )struct fscache_cookie *cache;#endif -struct mutex wb_lock;/* Writeback serialisation */ +struct list_head wb_queue;/* Queue of processes wanting to do writeback */ loff_t _remote_i_size;/* Size of the remote file */ loff_t _zero_point;/* Size after which we assume there's no data * on the server */ +spinlock_t lock;/* Lock covering wb_queue */ atomic_t io_count;/* Number of outstanding reqs */ unsigned long flags;#define NETFS_ICTX_ODIRECT 0/* The file has DIO in progress */ #define NETFS_ICTX_UNBUFFERED 1/* I/O should not use the pagecache */ +#define NETFS_ICTX_WB_LOCK 2/* Writeback serialisation lock */ #define NETFS_ICTX_MODIFIED_ATTR 3/* Indicate change in mtime/ctime */ #define NETFS_ICTX_SINGLE_NO_UPLOAD 4/* Monolithic payload, cache but no upload */ };
@@ -462,6 +464,10 @@ int netfs_alloc_folioq_buffer(struct address_space *mapping, size_t *_cur_size, ssize_t size, gfp_t gfp);void netfs_free_folioq_buffer (struct folio_queue *fq); +/* Writeback exclusion API. */ +bool netfs_wb_begin (struct netfs_inode *ictx,bool nowait); +void netfs_wb_end (struct netfs_inode *ictx); +/** * netfs_inode - Get the netfs inode context from the inode * @inode: The inode to query
@@ -743,7 +749,8 @@ static inline void netfs_inode_init(struct netfs_inode *ctx,#if IS_ENABLED (CONFIG_FSCACHE ) ctx->cache =NULL ;#endif -mutex_init (&ctx->wb_lock); +INIT_LIST_HEAD (&ctx->wb_queue); +spin_lock_init (&ctx->lock);/* ->releasepage() drives zero_point */ if (use_zero_point) { ctx->_zero_point = ctx->_remote_i_size;
David Howells
maintainerPATCH v3 12/15 netfs: Fix writethrough to use collection offload
In reply to David HowellsFix writethrough write to set NETFS_RREQ_OFFLOAD_COLLECTION on the request so that collection is processed asynchronously rather than only right at the end - and also so that asynchronous O_SYNC writes get collected at all.
--- fs/netfs/write_issue.c | 1 + 1 file changed, 1 insertion(+)
fs/netfs/write_issue.c +1 -0
@@ -620,6 +620,7 @@ struct netfs_io_request *netfs_begin_writethrough(struct kiocb *iocb, size_t len } wreq->io_streams[0 ].avail = true; +__set_bit (NETFS_RREQ_OFFLOAD_COLLECTION , &wreq->flags);trace_netfs_write (wreq, netfs_write_trace_writethrough);return wreq; }
David Howells
maintainerPATCH v3 13/15 netfs: Fix writeback error handling
In reply to David HowellsFix the error handling in writeback_iter() loop. If an error occurs, writeback_iter() needs to be called again with *error set to the error so that it can clean up iteration state. Further, the current folio needs unlocking and redirtying.
--- fs/netfs/write_issue.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
fs/netfs/write_issue.c +8 -3
@@ -582,8 +582,6 @@ int netfs_writepages(struct address_space *mapping, } error =netfs_write_folio (wreq, wbc, folio); -if (error <0 ) -break ; }while ((folio =writeback_iter (mapping, wbc, folio, &error)));netfs_end_issue_write (wreq);
@@ -594,7 +592,14 @@ int netfs_writepages(struct address_space *mapping,return error; couldnt_start: -netfs_kill_dirty_pages (mapping, wbc, folio); +if (error == -ENOMEM ) { +folio_redirty_for_writepage (wbc, folio); +folio_unlock (folio); + folio =writeback_iter (mapping, wbc, folio, &error); +WARN_ON_ONCE (folio !=NULL ); + }else { +netfs_kill_dirty_pages (mapping, wbc, folio); + } out:netfs_wb_end (ictx);_leave (" = %d" , error);
David Howells
maintainerPATCH v3 14/15 netfs: Fix folio state after ENOMEM whilst under writeback iteration
In reply to David HowellsFix the state of the current folio when ENOMEM occurs during writeback iteration. The folio needs to be redirtied and unlocked before the terminal writeback_iter() is invoked.
--- fs/netfs/write_issue.c | 4 ++++ 1 file changed, 4 insertions(+)
fs/netfs/write_issue.c +4 -0
@@ -582,6 +582,10 @@ int netfs_writepages(struct address_space *mapping, } error =netfs_write_folio (wreq, wbc, folio); +if (error == -ENOMEM ) { +folio_redirty_for_writepage (wbc, folio); +folio_unlock (folio); + } }while ((folio =writeback_iter (mapping, wbc, folio, &error)));netfs_end_issue_write (wreq);
David Howells
maintainerPATCH v3 15/15 netfs: Fix DIO write retry for filesystems without a ->prepare_write()
In reply to David HowellsFix netfs_unbuffered_write() so that it doesn't re-issue a write twice when the filesystem doesn't have a ->prepare_write(). The resetting of the iterator and the call to netfs_reissue_write() should just be removed as almost everything it does is done again when the loop it's in goes back to the top.
It does, however, still need the IN_PROGRESS flag setting, so that (and the stat inc) are moved out of the if-statement.
Further, the MADE_PROGRESS flags should be cleared and wreq->transferred should be updated, so fix those too.
--- fs/netfs/direct_write.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
fs/netfs/direct_write.c +7 -11
@@ -166,13 +166,16 @@ static int netfs_unbuffered_write(struct netfs_io_request *wreq)*/ subreq->error = -EAGAIN ;trace_netfs_sreq (subreq, netfs_sreq_trace_retry); -if (subreq->transferred >0 ) +if (subreq->transferred >0 ) {iov_iter_advance (&wreq->buffer.iter, subreq->transferred); + wreq->transferred += subreq->transferred; + }if (stream->source ==NETFS_UPLOAD_TO_SERVER && wreq->netfs_ops->retry_request) wreq->netfs_ops->retry_request (wreq, stream); +__clear_bit (NETFS_SREQ_MADE_PROGRESS , &subreq->flags);__clear_bit (NETFS_SREQ_NEED_RETRY , &subreq->flags);__clear_bit (NETFS_SREQ_BOUNDARY , &subreq->flags);__clear_bit (NETFS_SREQ_FAILED , &subreq->flags);
@@ -186,17 +189,10 @@ static int netfs_unbuffered_write(struct netfs_io_request *wreq)netfs_get_subrequest (subreq, netfs_sreq_trace_get_resubmit); -if (stream->prepare_write) { +if (stream->prepare_write) stream->prepare_write (subreq); -__set_bit (NETFS_SREQ_IN_PROGRESS , &subreq->flags); -netfs_stat (&netfs_n_wh_retry_write_subreq); - }else { -struct iov_iter source; - -netfs_reset_iter (subreq); - source = subreq->io_iter; -netfs_reissue_write (stream, subreq, &source); - } +__set_bit (NETFS_SREQ_IN_PROGRESS , &subreq->flags); +netfs_stat (&netfs_n_wh_retry_write_subreq); }netfs_unbuffered_write_done (wreq);
Quoted 8 lines · David Howells
netfs: Miscellaneous fixes
Hi Christian,
Here are some miscellaneous fixes for netfslib. I separated them from my netfs-next branch. Various Sashiko review comments[1][2][3] are addressed:
[...]
Applied to the vfs.fixes branch of the vfs/vfs.git tree. Patches in the vfs.fixes branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.fixes
[01/15] netfs: Fix decision whether to disallow write-streaming due to fscache use
https://git.kernel.org/vfs/vfs/c/61c79e2cac37 [02/15] netfs: Fix netfs_create_write_req() to handle async cache object creation https://git.kernel.org/vfs/vfs/c/f43e809738d8 [03/15] cachefiles: Fix double fput https://git.kernel.org/vfs/vfs/c/4c6fe3b4ba06 [04/15] cachefiles: Fix file burial to take lock when unsetting S_KERNEL_FILE https://git.kernel.org/vfs/vfs/c/f1ef918f8183 [05/15] iov_iter: Fix potential underflow in iov_iter_extract_xarray_pages() https://git.kernel.org/vfs/vfs/c/72c00e84e611 [06/15] iov_iter: Fix missing alloc fail check in iov_iter_extract_bvec_pages() https://git.kernel.org/vfs/vfs/c/eea815a087cd [07/15] iov_iter: Fix a memory leak in iov_iter_extract_user_pages() https://git.kernel.org/vfs/vfs/c/11cf15dacff6 [08/15] iov_iter: Remove unused variable in kunit_iov_iter.c https://git.kernel.org/vfs/vfs/c/8cb836bfb87d [09/15] scatterlist: Fix offset in folio calc in extract_xarray_to_sg() https://git.kernel.org/vfs/vfs/c/132e70406412 [10/15] netfs: Fix kdoc warning https://git.kernel.org/vfs/vfs/c/46163ade2426 [11/15] netfs: Replace wb_lock with a bit lock for asynchronicity https://git.kernel.org/vfs/vfs/c/1b157b933689 [12/15] netfs: Fix writethrough to use collection offload https://git.kernel.org/vfs/vfs/c/6ed47a3ca47e [13/15] netfs: Fix writeback error handling https://git.kernel.org/vfs/vfs/c/70d669edd0bb [14/15] netfs: Fix folio state after ENOMEM whilst under writeback iteration https://git.kernel.org/vfs/vfs/c/a009d3cfb306 [15/15] netfs: Fix DIO write retry for filesystems without a ->prepare_write() https://git.kernel.org/vfs/vfs/c/b43370a12abb