Message ID | 165290014021.1646290.13716646283504726941.stgit@magnolia (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
Series | xfs: fix buffer cancellation table leak during log recovery | expand |
On Wed, May 18, 2022 at 11:55:40AM -0700, Darrick J. Wong wrote: > + p = kmalloc_array(XLOG_BC_TABLE_SIZE, sizeof(struct list_head), > + GFP_KERNEL | __GFP_NOWARN); Why the __GFP_NOWARN?
On Thu, May 19, 2022 at 01:36:57AM -0700, Christoph Hellwig wrote: > On Wed, May 18, 2022 at 11:55:40AM -0700, Darrick J. Wong wrote: > > + p = kmalloc_array(XLOG_BC_TABLE_SIZE, sizeof(struct list_head), > > + GFP_KERNEL | __GFP_NOWARN); > > Why the __GFP_NOWARN? It's a straight port of xfs_km_flags_t==0, which is what the old code did. I suspect it doesn't make any practical difference since at most this will be allocating 1k of memory. Want me to make it GFP_KERNEL only? --D
On Thu, May 19, 2022 at 10:08:44AM -0700, Darrick J. Wong wrote: > On Thu, May 19, 2022 at 01:36:57AM -0700, Christoph Hellwig wrote: > > On Wed, May 18, 2022 at 11:55:40AM -0700, Darrick J. Wong wrote: > > > + p = kmalloc_array(XLOG_BC_TABLE_SIZE, sizeof(struct list_head), > > > + GFP_KERNEL | __GFP_NOWARN); > > > > Why the __GFP_NOWARN? > > It's a straight port of xfs_km_flags_t==0, which is what the old code > did. I suspect it doesn't make any practical difference since at most > this will be allocating 1k of memory. Want me to make it GFP_KERNEL > only? I think that would make more sense. With that: Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h index c96102484aa6..8227c74e75ec 100644 --- a/fs/xfs/libxfs/xfs_log_recover.h +++ b/fs/xfs/libxfs/xfs_log_recover.h @@ -128,7 +128,7 @@ int xlog_recover_iget(struct xfs_mount *mp, xfs_ino_t ino, struct xfs_inode **ipp); void xlog_recover_release_intent(struct xlog *log, unsigned short intent_type, uint64_t intent_id); -void xlog_alloc_buf_cancel_table(struct xlog *log); +int xlog_alloc_buf_cancel_table(struct xlog *log); void xlog_free_buf_cancel_table(struct xlog *log); #endif /* __XFS_LOG_RECOVER_H__ */ diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c index d1e484649a33..4b54b808fb07 100644 --- a/fs/xfs/xfs_buf_item_recover.c +++ b/fs/xfs/xfs_buf_item_recover.c @@ -994,19 +994,25 @@ const struct xlog_recover_item_ops xlog_buf_item_ops = { .commit_pass2 = xlog_recover_buf_commit_pass2, }; -void +int xlog_alloc_buf_cancel_table( struct xlog *log) { + void *p; int i; ASSERT(log->l_buf_cancel_table == NULL); - log->l_buf_cancel_table = kmem_zalloc(XLOG_BC_TABLE_SIZE * - sizeof(struct list_head), - 0); + p = kmalloc_array(XLOG_BC_TABLE_SIZE, sizeof(struct list_head), + GFP_KERNEL | __GFP_NOWARN); + if (!p) + return -ENOMEM; + + log->l_buf_cancel_table = p; for (i = 0; i < XLOG_BC_TABLE_SIZE; i++) INIT_LIST_HEAD(&log->l_buf_cancel_table[i]); + + return 0; } void diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index bb10686ef88a..f77e072426cd 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -3238,7 +3238,9 @@ xlog_do_log_recovery( * First do a pass to find all of the cancelled buf log items. * Store them in the buf_cancel_table for use in the second pass. */ - xlog_alloc_buf_cancel_table(log); + error = xlog_alloc_buf_cancel_table(log); + if (error) + return error; error = xlog_do_recovery_pass(log, head_blk, tail_blk, XLOG_RECOVER_PASS1, NULL);