Skip to content

Commit 20d72b0

Browse files
dhowellsbrauner
dhowells
authored and
brauner
committed
netfs: Fix the request's work item to not require a ref
When the netfs_io_request struct's work item is queued, it must be supplied with a ref to the work item struct to prevent it being deallocated whilst on the queue or whilst it is being processed. This is tricky to manage as we have to get a ref before we try and queue it and then we may find it's already queued and is thus already holding a ref - in which case we have to try and get rid of the ref again. The problem comes if we're in BH or IRQ context and need to drop the ref: if netfs_put_request() reduces the count to 0, we have to do the cleanup - but the cleanup may need to wait. Fix this by adding a new work item to the request, ->cleanup_work, and dispatching that when the refcount hits zero. That can then synchronously cancel any outstanding work on the main work item before doing the cleanup. Adding a new work item also deals with another problem upstream where it's sometimes changing the work func in the put function and requeuing it - which has occasionally in the past caused the cleanup to happen incorrectly. As a bonus, this allows us to get rid of the 'was_async' parameter from a bunch of functions. This indicated whether the put function might not be permitted to sleep. Fixes: 3d3c950 ("netfs: Provide readahead and readpage netfs helpers") Signed-off-by: David Howells <[email protected]> Link: https://lore.kernel.org/[email protected] cc: Paulo Alcantara <[email protected]> cc: Marc Dionne <[email protected]> cc: Steve French <[email protected]> cc: [email protected] cc: [email protected] cc: [email protected] Signed-off-by: Christian Brauner <[email protected]>
1 parent 34eb98c commit 20d72b0

26 files changed

+159
-162
lines changed

‎fs/9p/vfs_addr.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ static void v9fs_issue_write(struct netfs_io_subrequest *subreq)
5959
len = p9_client_write(fid, subreq->start, &subreq->io_iter, &err);
6060
if (len > 0)
6161
__set_bit(NETFS_SREQ_MADE_PROGRESS, &subreq->flags);
62-
netfs_write_subrequest_terminated(subreq, len ?: err, false);
62+
netfs_write_subrequest_terminated(subreq, len ?: err);
6363
}
6464

6565
/**

‎fs/afs/write.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,17 +120,17 @@ static void afs_issue_write_worker(struct work_struct *work)
120120

121121
#if 0 // Error injection
122122
if (subreq->debug_index == 3)
123-
return netfs_write_subrequest_terminated(subreq, -ENOANO, false);
123+
return netfs_write_subrequest_terminated(subreq, -ENOANO);
124124

125125
if (!subreq->retry_count) {
126126
set_bit(NETFS_SREQ_NEED_RETRY, &subreq->flags);
127-
return netfs_write_subrequest_terminated(subreq, -EAGAIN, false);
127+
return netfs_write_subrequest_terminated(subreq, -EAGAIN);
128128
}
129129
#endif
130130

131131
op = afs_alloc_operation(wreq->netfs_priv, vnode->volume);
132132
if (IS_ERR(op))
133-
return netfs_write_subrequest_terminated(subreq, -EAGAIN, false);
133+
return netfs_write_subrequest_terminated(subreq, -EAGAIN);
134134

135135
afs_op_set_vnode(op, 0, vnode);
136136
op->file[0].dv_delta = 1;
@@ -166,7 +166,7 @@ static void afs_issue_write_worker(struct work_struct *work)
166166
break;
167167
}
168168

169-
netfs_write_subrequest_terminated(subreq, ret < 0 ? ret : subreq->len, false);
169+
netfs_write_subrequest_terminated(subreq, ret < 0 ? ret : subreq->len);
170170
}
171171

172172
void afs_issue_write(struct netfs_io_subrequest *subreq)

‎fs/cachefiles/io.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ static void cachefiles_read_complete(struct kiocb *iocb, long ret)
6363
ret = -ESTALE;
6464
}
6565

66-
ki->term_func(ki->term_func_priv, ret, ki->was_async);
66+
ki->term_func(ki->term_func_priv, ret);
6767
}
6868

6969
cachefiles_put_kiocb(ki);
@@ -188,7 +188,7 @@ static int cachefiles_read(struct netfs_cache_resources *cres,
188188

189189
presubmission_error:
190190
if (term_func)
191-
term_func(term_func_priv, ret < 0 ? ret : skipped, false);
191+
term_func(term_func_priv, ret < 0 ? ret : skipped);
192192
return ret;
193193
}
194194

@@ -271,7 +271,7 @@ static void cachefiles_write_complete(struct kiocb *iocb, long ret)
271271
atomic_long_sub(ki->b_writing, &object->volume->cache->b_writing);
272272
set_bit(FSCACHE_COOKIE_HAVE_DATA, &object->cookie->flags);
273273
if (ki->term_func)
274-
ki->term_func(ki->term_func_priv, ret, ki->was_async);
274+
ki->term_func(ki->term_func_priv, ret);
275275
cachefiles_put_kiocb(ki);
276276
}
277277

@@ -301,7 +301,7 @@ int __cachefiles_write(struct cachefiles_object *object,
301301
ki = kzalloc(sizeof(struct cachefiles_kiocb), GFP_KERNEL);
302302
if (!ki) {
303303
if (term_func)
304-
term_func(term_func_priv, -ENOMEM, false);
304+
term_func(term_func_priv, -ENOMEM);
305305
return -ENOMEM;
306306
}
307307

@@ -366,7 +366,7 @@ static int cachefiles_write(struct netfs_cache_resources *cres,
366366
{
367367
if (!fscache_wait_for_operation(cres, FSCACHE_WANT_WRITE)) {
368368
if (term_func)
369-
term_func(term_func_priv, -ENOBUFS, false);
369+
term_func(term_func_priv, -ENOBUFS);
370370
trace_netfs_sreq(term_func_priv, netfs_sreq_trace_cache_nowrite);
371371
return -ENOBUFS;
372372
}
@@ -665,7 +665,7 @@ static void cachefiles_issue_write(struct netfs_io_subrequest *subreq)
665665
pre = CACHEFILES_DIO_BLOCK_SIZE - off;
666666
if (pre >= len) {
667667
fscache_count_dio_misfit();
668-
netfs_write_subrequest_terminated(subreq, len, false);
668+
netfs_write_subrequest_terminated(subreq, len);
669669
return;
670670
}
671671
subreq->transferred += pre;
@@ -691,7 +691,7 @@ static void cachefiles_issue_write(struct netfs_io_subrequest *subreq)
691691
len -= post;
692692
if (len == 0) {
693693
fscache_count_dio_misfit();
694-
netfs_write_subrequest_terminated(subreq, post, false);
694+
netfs_write_subrequest_terminated(subreq, post);
695695
return;
696696
}
697697
iov_iter_truncate(&subreq->io_iter, len);
@@ -703,7 +703,7 @@ static void cachefiles_issue_write(struct netfs_io_subrequest *subreq)
703703
&start, &len, len, true);
704704
cachefiles_end_secure(cache, saved_cred);
705705
if (ret < 0) {
706-
netfs_write_subrequest_terminated(subreq, ret, false);
706+
netfs_write_subrequest_terminated(subreq, ret);
707707
return;
708708
}
709709

‎fs/ceph/addr.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ static void ceph_set_page_fscache(struct page *page)
539539
folio_start_private_2(page_folio(page)); /* [DEPRECATED] */
540540
}
541541

542-
static void ceph_fscache_write_terminated(void *priv, ssize_t error, bool was_async)
542+
static void ceph_fscache_write_terminated(void *priv, ssize_t error)
543543
{
544544
struct inode *inode = priv;
545545

‎fs/erofs/fscache.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,7 @@ static void erofs_fscache_req_io_put(struct erofs_fscache_io *io)
102102
erofs_fscache_req_put(req);
103103
}
104104

105-
static void erofs_fscache_req_end_io(void *priv,
106-
ssize_t transferred_or_error, bool was_async)
105+
static void erofs_fscache_req_end_io(void *priv, ssize_t transferred_or_error)
107106
{
108107
struct erofs_fscache_io *io = priv;
109108
struct erofs_fscache_rq *req = io->private;
@@ -180,8 +179,7 @@ struct erofs_fscache_bio {
180179
struct bio_vec bvecs[BIO_MAX_VECS];
181180
};
182181

183-
static void erofs_fscache_bio_endio(void *priv,
184-
ssize_t transferred_or_error, bool was_async)
182+
static void erofs_fscache_bio_endio(void *priv, ssize_t transferred_or_error)
185183
{
186184
struct erofs_fscache_bio *io = priv;
187185

‎fs/netfs/buffered_read.c

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,9 @@ static void netfs_read_to_pagecache(struct netfs_io_request *rreq)
262262
if (ret < 0) {
263263
subreq->error = ret;
264264
/* Not queued - release both refs. */
265-
netfs_put_subrequest(subreq, false,
265+
netfs_put_subrequest(subreq,
266266
netfs_sreq_trace_put_cancel);
267-
netfs_put_subrequest(subreq, false,
267+
netfs_put_subrequest(subreq,
268268
netfs_sreq_trace_put_cancel);
269269
break;
270270
}
@@ -297,8 +297,8 @@ static void netfs_read_to_pagecache(struct netfs_io_request *rreq)
297297
subreq->error = ret;
298298
trace_netfs_sreq(subreq, netfs_sreq_trace_cancel);
299299
/* Not queued - release both refs. */
300-
netfs_put_subrequest(subreq, false, netfs_sreq_trace_put_cancel);
301-
netfs_put_subrequest(subreq, false, netfs_sreq_trace_put_cancel);
300+
netfs_put_subrequest(subreq, netfs_sreq_trace_put_cancel);
301+
netfs_put_subrequest(subreq, netfs_sreq_trace_put_cancel);
302302
break;
303303
}
304304
size -= slice;
@@ -365,12 +365,10 @@ void netfs_readahead(struct readahead_control *ractl)
365365
goto cleanup_free;
366366
netfs_read_to_pagecache(rreq);
367367

368-
netfs_put_request(rreq, true, netfs_rreq_trace_put_return);
369-
return;
368+
return netfs_put_request(rreq, netfs_rreq_trace_put_return);
370369

371370
cleanup_free:
372-
netfs_put_request(rreq, false, netfs_rreq_trace_put_failed);
373-
return;
371+
return netfs_put_request(rreq, netfs_rreq_trace_put_failed);
374372
}
375373
EXPORT_SYMBOL(netfs_readahead);
376374

@@ -470,11 +468,11 @@ static int netfs_read_gaps(struct file *file, struct folio *folio)
470468
folio_mark_uptodate(folio);
471469
}
472470
folio_unlock(folio);
473-
netfs_put_request(rreq, false, netfs_rreq_trace_put_return);
471+
netfs_put_request(rreq, netfs_rreq_trace_put_return);
474472
return ret < 0 ? ret : 0;
475473

476474
discard:
477-
netfs_put_request(rreq, false, netfs_rreq_trace_put_discard);
475+
netfs_put_request(rreq, netfs_rreq_trace_put_discard);
478476
alloc_error:
479477
folio_unlock(folio);
480478
return ret;
@@ -530,11 +528,11 @@ int netfs_read_folio(struct file *file, struct folio *folio)
530528

531529
netfs_read_to_pagecache(rreq);
532530
ret = netfs_wait_for_read(rreq);
533-
netfs_put_request(rreq, false, netfs_rreq_trace_put_return);
531+
netfs_put_request(rreq, netfs_rreq_trace_put_return);
534532
return ret < 0 ? ret : 0;
535533

536534
discard:
537-
netfs_put_request(rreq, false, netfs_rreq_trace_put_discard);
535+
netfs_put_request(rreq, netfs_rreq_trace_put_discard);
538536
alloc_error:
539537
folio_unlock(folio);
540538
return ret;
@@ -689,7 +687,7 @@ int netfs_write_begin(struct netfs_inode *ctx,
689687
ret = netfs_wait_for_read(rreq);
690688
if (ret < 0)
691689
goto error;
692-
netfs_put_request(rreq, false, netfs_rreq_trace_put_return);
690+
netfs_put_request(rreq, netfs_rreq_trace_put_return);
693691

694692
have_folio:
695693
ret = folio_wait_private_2_killable(folio);
@@ -701,7 +699,7 @@ int netfs_write_begin(struct netfs_inode *ctx,
701699
return 0;
702700

703701
error_put:
704-
netfs_put_request(rreq, false, netfs_rreq_trace_put_failed);
702+
netfs_put_request(rreq, netfs_rreq_trace_put_failed);
705703
error:
706704
if (folio) {
707705
folio_unlock(folio);
@@ -752,11 +750,11 @@ int netfs_prefetch_for_write(struct file *file, struct folio *folio,
752750

753751
netfs_read_to_pagecache(rreq);
754752
ret = netfs_wait_for_read(rreq);
755-
netfs_put_request(rreq, false, netfs_rreq_trace_put_return);
753+
netfs_put_request(rreq, netfs_rreq_trace_put_return);
756754
return ret < 0 ? ret : 0;
757755

758756
error_put:
759-
netfs_put_request(rreq, false, netfs_rreq_trace_put_discard);
757+
netfs_put_request(rreq, netfs_rreq_trace_put_discard);
760758
error:
761759
_leave(" = %d", ret);
762760
return ret;

‎fs/netfs/direct_read.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ static int netfs_dispatch_unbuffered_reads(struct netfs_io_request *rreq)
8585
if (rreq->netfs_ops->prepare_read) {
8686
ret = rreq->netfs_ops->prepare_read(subreq);
8787
if (ret < 0) {
88-
netfs_put_subrequest(subreq, false, netfs_sreq_trace_put_cancel);
88+
netfs_put_subrequest(subreq, netfs_sreq_trace_put_cancel);
8989
break;
9090
}
9191
}
@@ -144,7 +144,7 @@ static ssize_t netfs_unbuffered_read(struct netfs_io_request *rreq, bool sync)
144144
ret = netfs_dispatch_unbuffered_reads(rreq);
145145

146146
if (!rreq->submitted) {
147-
netfs_put_request(rreq, false, netfs_rreq_trace_put_no_submit);
147+
netfs_put_request(rreq, netfs_rreq_trace_put_no_submit);
148148
inode_dio_end(rreq->inode);
149149
ret = 0;
150150
goto out;
@@ -236,7 +236,7 @@ ssize_t netfs_unbuffered_read_iter_locked(struct kiocb *iocb, struct iov_iter *i
236236
}
237237

238238
out:
239-
netfs_put_request(rreq, false, netfs_rreq_trace_put_return);
239+
netfs_put_request(rreq, netfs_rreq_trace_put_return);
240240
if (ret > 0)
241241
orig_count -= ret;
242242
return ret;

‎fs/netfs/direct_write.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ ssize_t netfs_unbuffered_write_iter_locked(struct kiocb *iocb, struct iov_iter *
117117
}
118118

119119
out:
120-
netfs_put_request(wreq, false, netfs_rreq_trace_put_return);
120+
netfs_put_request(wreq, netfs_rreq_trace_put_return);
121121
return ret;
122122
}
123123
EXPORT_SYMBOL(netfs_unbuffered_write_iter_locked);

‎fs/netfs/fscache_io.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,7 @@ EXPORT_SYMBOL(__fscache_clear_page_bits);
192192
/*
193193
* Deal with the completion of writing the data to the cache.
194194
*/
195-
static void fscache_wreq_done(void *priv, ssize_t transferred_or_error,
196-
bool was_async)
195+
static void fscache_wreq_done(void *priv, ssize_t transferred_or_error)
197196
{
198197
struct fscache_write_request *wreq = priv;
199198

@@ -202,8 +201,7 @@ static void fscache_wreq_done(void *priv, ssize_t transferred_or_error,
202201
wreq->set_bits);
203202

204203
if (wreq->term_func)
205-
wreq->term_func(wreq->term_func_priv, transferred_or_error,
206-
was_async);
204+
wreq->term_func(wreq->term_func_priv, transferred_or_error);
207205
fscache_end_operation(&wreq->cache_resources);
208206
kfree(wreq);
209207
}
@@ -255,14 +253,14 @@ void __fscache_write_to_cache(struct fscache_cookie *cookie,
255253
return;
256254

257255
abandon_end:
258-
return fscache_wreq_done(wreq, ret, false);
256+
return fscache_wreq_done(wreq, ret);
259257
abandon_free:
260258
kfree(wreq);
261259
abandon:
262260
if (using_pgpriv2)
263261
fscache_clear_page_bits(mapping, start, len, cond);
264262
if (term_func)
265-
term_func(term_func_priv, ret, false);
263+
term_func(term_func_priv, ret);
266264
}
267265
EXPORT_SYMBOL(__fscache_write_to_cache);
268266

‎fs/netfs/internal.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
/*
2424
* buffered_read.c
2525
*/
26-
void netfs_cache_read_terminated(void *priv, ssize_t transferred_or_error, bool was_async);
26+
void netfs_cache_read_terminated(void *priv, ssize_t transferred_or_error);
2727
int netfs_prefetch_for_write(struct file *file, struct folio *folio,
2828
size_t offset, size_t len);
2929

@@ -71,9 +71,8 @@ struct netfs_io_request *netfs_alloc_request(struct address_space *mapping,
7171
loff_t start, size_t len,
7272
enum netfs_io_origin origin);
7373
void netfs_get_request(struct netfs_io_request *rreq, enum netfs_rreq_ref_trace what);
74-
void netfs_clear_subrequests(struct netfs_io_request *rreq, bool was_async);
75-
void netfs_put_request(struct netfs_io_request *rreq, bool was_async,
76-
enum netfs_rreq_ref_trace what);
74+
void netfs_clear_subrequests(struct netfs_io_request *rreq);
75+
void netfs_put_request(struct netfs_io_request *rreq, enum netfs_rreq_ref_trace what);
7776
struct netfs_io_subrequest *netfs_alloc_subrequest(struct netfs_io_request *rreq);
7877

7978
static inline void netfs_see_request(struct netfs_io_request *rreq,
@@ -94,7 +93,7 @@ static inline void netfs_see_subrequest(struct netfs_io_subrequest *subreq,
9493
*/
9594
void netfs_read_collection_worker(struct work_struct *work);
9695
void netfs_wake_read_collector(struct netfs_io_request *rreq);
97-
void netfs_cache_read_terminated(void *priv, ssize_t transferred_or_error, bool was_async);
96+
void netfs_cache_read_terminated(void *priv, ssize_t transferred_or_error);
9897
ssize_t netfs_wait_for_read(struct netfs_io_request *rreq);
9998
void netfs_wait_for_pause(struct netfs_io_request *rreq);
10099

@@ -177,7 +176,7 @@ static inline void netfs_stat_d(atomic_t *stat)
177176
*/
178177
int netfs_folio_written_back(struct folio *folio);
179178
void netfs_write_collection_worker(struct work_struct *work);
180-
void netfs_wake_write_collector(struct netfs_io_request *wreq, bool was_async);
179+
void netfs_wake_write_collector(struct netfs_io_request *wreq);
181180

182181
/*
183182
* write_issue.c

0 commit comments

Comments
 (0)