* [PATCH] xfs: Convert from atomic_t to refcount_t on xlog_ticket->t_ref
@ 2021-07-17 10:21 Xiyu Yang
2021-07-20 2:20 ` Darrick J. Wong
0 siblings, 1 reply; 2+ messages in thread
From: Xiyu Yang @ 2021-07-17 10:21 UTC (permalink / raw)
To: Darrick J. Wong, linux-xfs, linux-kernel; +Cc: yuanxzhang, Xiyu Yang, Xin Tan
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/xfs/xfs_log.c | 10 +++++-----
fs/xfs/xfs_log_priv.h | 4 +++-
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 36fa2650b081..1da711f1a229 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -3347,8 +3347,8 @@ void
xfs_log_ticket_put(
xlog_ticket_t *ticket)
{
- ASSERT(atomic_read(&ticket->t_ref) > 0);
- if (atomic_dec_and_test(&ticket->t_ref))
+ ASSERT(refcount_read(&ticket->t_ref) > 0);
+ if (refcount_dec_and_test(&ticket->t_ref))
kmem_cache_free(xfs_log_ticket_zone, ticket);
}
@@ -3356,8 +3356,8 @@ xlog_ticket_t *
xfs_log_ticket_get(
xlog_ticket_t *ticket)
{
- ASSERT(atomic_read(&ticket->t_ref) > 0);
- atomic_inc(&ticket->t_ref);
+ ASSERT(refcount_read(&ticket->t_ref) > 0);
+ refcount_inc(&ticket->t_ref);
return ticket;
}
@@ -3477,7 +3477,7 @@ xlog_ticket_alloc(
unit_res = xlog_calc_unit_res(log, unit_bytes);
- atomic_set(&tic->t_ref, 1);
+ refcount_set(&tic->t_ref, 1);
tic->t_task = current;
INIT_LIST_HEAD(&tic->t_queue);
tic->t_unit_res = unit_res;
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index 4c41bbfa33b0..c4157d87cea4 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -6,6 +6,8 @@
#ifndef __XFS_LOG_PRIV_H__
#define __XFS_LOG_PRIV_H__
+#include <linux/refcount.h>
+
struct xfs_buf;
struct xlog;
struct xlog_ticket;
@@ -163,7 +165,7 @@ typedef struct xlog_ticket {
struct list_head t_queue; /* reserve/write queue */
struct task_struct *t_task; /* task that owns this ticket */
xlog_tid_t t_tid; /* transaction identifier : 4 */
- atomic_t t_ref; /* ticket reference count : 4 */
+ refcount_t t_ref; /* ticket reference count : 4 */
int t_curr_res; /* current reservation in bytes : 4 */
int t_unit_res; /* unit reservation in bytes : 4 */
char t_ocnt; /* original count : 1 */
--
2.7.4
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] xfs: Convert from atomic_t to refcount_t on xlog_ticket->t_ref
2021-07-17 10:21 [PATCH] xfs: Convert from atomic_t to refcount_t on xlog_ticket->t_ref Xiyu Yang
@ 2021-07-20 2:20 ` Darrick J. Wong
0 siblings, 0 replies; 2+ messages in thread
From: Darrick J. Wong @ 2021-07-20 2:20 UTC (permalink / raw)
To: Xiyu Yang; +Cc: linux-xfs, linux-kernel, yuanxzhang, Xin Tan
On Sat, Jul 17, 2021 at 06:21:02PM +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/xfs/xfs_log.c | 10 +++++-----
> fs/xfs/xfs_log_priv.h | 4 +++-
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 36fa2650b081..1da711f1a229 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -3347,8 +3347,8 @@ void
> xfs_log_ticket_put(
> xlog_ticket_t *ticket)
> {
> - ASSERT(atomic_read(&ticket->t_ref) > 0);
> - if (atomic_dec_and_test(&ticket->t_ref))
> + ASSERT(refcount_read(&ticket->t_ref) > 0);
> + if (refcount_dec_and_test(&ticket->t_ref))
I thought the refcount functions already had code to warn about
refcounts that get decremented below zero...
> kmem_cache_free(xfs_log_ticket_zone, ticket);
> }
>
> @@ -3356,8 +3356,8 @@ xlog_ticket_t *
> xfs_log_ticket_get(
> xlog_ticket_t *ticket)
> {
> - ASSERT(atomic_read(&ticket->t_ref) > 0);
> - atomic_inc(&ticket->t_ref);
> + ASSERT(refcount_read(&ticket->t_ref) > 0);
...and can't come back from the dead? Also, how much of a performance
impact does this have over the atomic_t functions? The last time anyone
proposed conversions similar to this, there were concerns about that.
--D
> + refcount_inc(&ticket->t_ref);
> return ticket;
> }
>
> @@ -3477,7 +3477,7 @@ xlog_ticket_alloc(
>
> unit_res = xlog_calc_unit_res(log, unit_bytes);
>
> - atomic_set(&tic->t_ref, 1);
> + refcount_set(&tic->t_ref, 1);
> tic->t_task = current;
> INIT_LIST_HEAD(&tic->t_queue);
> tic->t_unit_res = unit_res;
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index 4c41bbfa33b0..c4157d87cea4 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -6,6 +6,8 @@
> #ifndef __XFS_LOG_PRIV_H__
> #define __XFS_LOG_PRIV_H__
>
> +#include <linux/refcount.h>
> +
> struct xfs_buf;
> struct xlog;
> struct xlog_ticket;
> @@ -163,7 +165,7 @@ typedef struct xlog_ticket {
> struct list_head t_queue; /* reserve/write queue */
> struct task_struct *t_task; /* task that owns this ticket */
> xlog_tid_t t_tid; /* transaction identifier : 4 */
> - atomic_t t_ref; /* ticket reference count : 4 */
> + refcount_t t_ref; /* ticket reference count : 4 */
> int t_curr_res; /* current reservation in bytes : 4 */
> int t_unit_res; /* unit reservation in bytes : 4 */
> char t_ocnt; /* original count : 1 */
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-07-20 2:34 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-17 10:21 [PATCH] xfs: Convert from atomic_t to refcount_t on xlog_ticket->t_ref Xiyu Yang
2021-07-20 2:20 ` Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).