* Re: [PATCH] iomap: Convert from atomic_t to refcount_t on iomap_dio->ref
[not found] <1626683544-64155-1-git-send-email-xiyuyang19@fudan.edu.cn>
@ 2021-07-21 23:30 ` Darrick J. Wong
0 siblings, 0 replies; only message in thread
From: Darrick J. Wong @ 2021-07-21 23:30 UTC (permalink / raw)
To: Xiyu Yang
Cc: Christoph Hellwig, linux-xfs, linux-fsdevel, linux-kernel,
yuanxzhang, Xin Tan
On Mon, Jul 19, 2021 at 04:32:24PM +0800, Xiyu Yang wrote:
> refcount_t type and corresponding API can protect refcounters from
> accidental underflow and overflow and further use-after-free situations.
>
> Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
> ---
> fs/iomap/direct-io.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 9398b8c31323..d1393579a954 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -3,6 +3,7 @@
> * Copyright (C) 2010 Red Hat, Inc.
> * Copyright (c) 2016-2018 Christoph Hellwig.
> */
> +#include <linux/refcount.h>
> #include <linux/module.h>
> #include <linux/compiler.h>
> #include <linux/fs.h>
> @@ -28,7 +29,7 @@ struct iomap_dio {
> const struct iomap_dio_ops *dops;
> loff_t i_size;
> loff_t size;
> - atomic_t ref;
> + refcount_t ref;
> unsigned flags;
> int error;
> bool wait_for_completion;
> @@ -62,7 +63,7 @@ EXPORT_SYMBOL_GPL(iomap_dio_iopoll);
> static void iomap_dio_submit_bio(struct iomap_dio *dio, struct iomap *iomap,
> struct bio *bio, loff_t pos)
> {
> - atomic_inc(&dio->ref);
> + refcount_inc(&dio->ref);
Seems all right to me, though I wonder if any of the more
performance-minded people have comments about the overhead of refcount_t
vs. atomic_t?
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
>
> if (dio->iocb->ki_flags & IOCB_HIPRI)
> bio_set_polled(bio, dio->iocb);
> @@ -158,7 +159,7 @@ static void iomap_dio_bio_end_io(struct bio *bio)
> if (bio->bi_status)
> iomap_dio_set_error(dio, blk_status_to_errno(bio->bi_status));
>
> - if (atomic_dec_and_test(&dio->ref)) {
> + if (refcount_dec_and_test(&dio->ref)) {
> if (dio->wait_for_completion) {
> struct task_struct *waiter = dio->submit.waiter;
> WRITE_ONCE(dio->submit.waiter, NULL);
> @@ -471,7 +472,7 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> return ERR_PTR(-ENOMEM);
>
> dio->iocb = iocb;
> - atomic_set(&dio->ref, 1);
> + refcount_set(&dio->ref, 1);
> dio->size = 0;
> dio->i_size = i_size_read(inode);
> dio->dops = dops;
> @@ -611,7 +612,7 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> * after we got woken by the I/O completion handler.
> */
> dio->wait_for_completion = wait_for_completion;
> - if (!atomic_dec_and_test(&dio->ref)) {
> + if (!refcount_dec_and_test(&dio->ref)) {
> if (!wait_for_completion)
> return ERR_PTR(-EIOCBQUEUED);
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] only message in thread