xfs: fix a couple of potential deadlocks
cover xfs: fix a couple of potential deadlocks
Hi folks,
These are a couple of small fixes for lockdep enabled kernels. The first changes the initialisation of the i_rwsem lockdep state in the XFS code instead of in unlock_new_inode() to avoid lockdep re-initialising the lock state after it can be found in the cache and may have other processes waiting on the lock.
The second is adding the correct memory allocation context to xfs_reflink_convert_cow() as it gets called in the IO path where we hold pages locked for IO and so we can't recurse back into memory reclaim.
Cheers,
Dave.
PATCH 1/2 xfs: setup VFS i_rwsem lockdep state correctly
From: Dave Chinner <[email protected]>
When lockdep is enabled, it changes the type of the inode i_rwsem semaphore before unlocking a newly instantiated inode. THere is the possibility that there is already a waiter on that inode lock by the time we unlock the new inode, so having lockdep re-initialise the lock is a vector for trouble.
Avoid this whole situation by setting up the i_rwsem lockdep class at the same time we set up the XFS inode i_ilock classes and so the VFS doesn't have to change the lock class itself when it is potentially unsafe.
--- fs/xfs/xfs_iops.c | 8 ++++++++ 1 file changed, 8 insertions(+)
fs/xfs/xfs_iops.c +8 -0
@@ -1258,6 +1258,14 @@ xfs_setup_inode(xfs_diflags_to_iflags (inode, ip);if (S_ISDIR (inode->i_mode)) { +/* +* We set the i_rwsem class here to avoid potential races with +* lockdep_annotate_inode_mutex_key() reinitialising the lock +* after a filehandle lookup has already found the inode in +* cache before it has been unlocked via unlock_new_inode(). +*/ +lockdep_set_class (&inode->i_rwsem, + &inode->i_sb->s_type->i_mutex_dir_key);lockdep_set_class (&ip->i_lock.mr_lock, &xfs_dir_ilock_class); ip->d_ops = ip->i_mount->m_dir_inode_ops; }else {
Signature · 1 line
2.17.0
Re: [PATCH 1/2] xfs: setup VFS i_rwsem lockdep state correctly
Quoted 8 lines · Dave Chinner
From: Dave Chinner <[email protected]>
When lockdep is enabled, it changes the type of the inode
i_rwsemsemaphore before unlocking a newly instantiated inode. THere is the possibility that there is already a waiter on that inode lock by the time we unlock the new inode, so having lockdep re-initialise the lock is a vector for trouble.
Quoted 6 lines
Avoid this whole situation by setting up the
i_rwsemlockdep class at the same time we set up the XFS inodei_ilockclasses and so the VFS doesn't have to change the lock class itself when it is potentially unsafe.Signed-off-by Dave Chinner <[email protected]>
I just realised that the VFS equivalent patch has made it upstream, too, which would help explain this a bit more. Darrick, can you add this to the commit message:
"This change is necessary because the equivalent fixes to the VFS code made in commit 1e2e547a93a0 ("do d_instantiate/unlock_new_inode combinations safely") are not relevant to XFS as it has it's own internal inode cache lookup and instantiation routines."
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Brian Foster
Reviewed-byRe: [PATCH 1/2] xfs: setup VFS i_rwsem lockdep state correctly
Quoted 16 lines · Dave Chinner
Quoted 8 lines · Dave Chinner
From: Dave Chinner <[email protected]>
When lockdep is enabled, it changes the type of the inode
i_rwsemsemaphore before unlocking a newly instantiated inode. THere is the possibility that there is already a waiter on that inode lock by the time we unlock the new inode, so having lockdep re-initialise the lock is a vector for trouble.Quoted 6 lines
Avoid this whole situation by setting up the
i_rwsemlockdep class at the same time we set up the XFS inodei_ilockclasses and so the VFS doesn't have to change the lock class itself when it is potentially unsafe.Signed-off-by Dave Chinner <[email protected]>
Quoted 9 lines
I just realised that the VFS equivalent patch has made it upstream, too, which would help explain this a bit more. Darrick, can you add this to the commit message:
"This change is necessary because the equivalent fixes to the VFS code made in commit
1e2e547a93a0("do d_instantiate/unlock_new_inode combinations safely") are not relevant to XFS as it has it's own internal inode cache lookup and instantiation routines."
The reference definitely helps, thanks. With that added:
Quoted 10 lines
Cheers,
Dave.
Signature · 6 lines
Dave Chinner [email protected] -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] xfs: setup VFS i_rwsem lockdep state correctly
In reply to Dave ChinnerQuoted 28 lines · Dave Chinner
From: Dave Chinner <[email protected]>
When lockdep is enabled, it changes the type of the inode
i_rwsemsemaphore before unlocking a newly instantiated inode. THere is the possibility that there is already a waiter on that inode lock by the time we unlock the new inode, so having lockdep re-initialise the lock is a vector for trouble.Avoid this whole situation by setting up the
i_rwsemlockdep class at the same time we set up the XFS inodei_ilockclasses and so the VFS doesn't have to change the lock class itself when it is potentially unsafe.Signed-off-by Dave Chinner <[email protected]>--- fs/xfs/xfs_iops.c | 8 ++++++++ 1 file changed, 8 insertions(+)fs/xfs/xfs_iops.c +1
-0@@ -1258,6 +1258,14 @@ xfs_setup_inode(xfs_diflags_to_iflags (inode, ip);if (S_ISDIR (inode->i_mode)) { +/*
fs/xfs/xfs_iops.c xfs_setup_inode( · line 1271
Quoted 10 lines
fs/xfs/xfs_iops.c +7
-0+* We set the i_rwsem class here to avoid potential races with +* lockdep_annotate_inode_mutex_key() reinitialising the lock +* after a filehandle lookup has already found the inode in +* cache before it has been unlocked via unlock_new_inode(). +*/ +lockdep_set_class (&inode->i_rwsem, + &inode->i_sb->s_type->i_mutex_dir_key);lockdep_set_class (&ip->i_lock.mr_lock, &xfs_dir_ilock_class); ip->d_ops = ip->i_mount->m_dir_inode_ops; }else {
Ok, you can add my review:
Re: [PATCH 1/2] xfs: setup VFS i_rwsem lockdep state correctly
In reply to Dave ChinnerQuoted 8 lines · Dave Chinner
From: Dave Chinner <[email protected]>
When lockdep is enabled, it changes the type of the inode
i_rwsemsemaphore before unlocking a newly instantiated inode. THere is the possibility that there is already a waiter on that inode lock by the time we unlock the new inode, so having lockdep re-initialise the lock is a vector for trouble.
Quoted 6 lines
Avoid this whole situation by setting up the
i_rwsemlockdep class at the same time we set up the XFS inodei_ilockclasses and so the VFS doesn't have to change the lock class itself when it is potentially unsafe.Signed-off-by Dave Chinner <[email protected]>
With the commit message changes added,
--D
Quoted 30 lines
--- fs/xfs/xfs_iops.c | 8 ++++++++ 1 file changed, 8 insertions(+)fs/xfs/xfs_iops.c +8
-0@@ -1258,6 +1258,14 @@ xfs_setup_inode(xfs_diflags_to_iflags (inode, ip);if (S_ISDIR (inode->i_mode)) { +/* +* We set the i_rwsem class here to avoid potential races with +* lockdep_annotate_inode_mutex_key() reinitialising the lock +* after a filehandle lookup has already found the inode in +* cache before it has been unlocked via unlock_new_inode(). +*/ +lockdep_set_class (&inode->i_rwsem, + &inode->i_sb->s_type->i_mutex_dir_key);lockdep_set_class (&ip->i_lock.mr_lock, &xfs_dir_ilock_class); ip->d_ops = ip->i_mount->m_dir_inode_ops; }else {Signature · 6 lines
2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 2/2 xfs: xfs_reflink_convert_cow() memory allocation deadlock
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
xfs_reflink_convert_cow() manipulates the incore extent list in GFP_KERNEL context in the IO submission path whilst holding locked pages under writeback. This is a memory reclaim deadlock vector. This code is not in a transaction, so any memory allocations it makes aren't protected via the memalloc_nofs_save() context that transactions carry.
Hence we need to run this call under memalloc_nofs_save() context to prevent potential memory allocations from being run as GFP_KERNEL and deadlocking.
--- fs/xfs/xfs_aops.c | 11 +++++++++++ fs/xfs/xfs_buf.c | 1 - fs/xfs/xfs_linux.h | 1 + 3 files changed, 12 insertions(+), 1 deletion(-)
fs/xfs/xfs_aops.c +11 -0
@@ -531,8 +531,19 @@ xfs_submit_ioend( {/* Convert CoW extents to regular */ if (!status && ioend->io_type ==XFS_IO_COW ) { +/* +* Yuk. This can do memory allocation, but is not a +* transactional operation so everything is done in GFP_KERNEL +* context. That can deadlock, because we hold pages in +* writeback state and GFP_KERNEL allocations can block on them. +* Hence we must operate in nofs conditions here. +*/ +unsigned nofs_flag; + + nofs_flag =memalloc_nofs_save (); status =xfs_reflink_convert_cow (XFS_I (ioend->io_inode), ioend->io_offset, ioend->io_size); +memalloc_nofs_restore (nofs_flag); }/* Reserve log space if we might write beyond the on-disk inode size. */
fs/xfs/xfs_buf.c +0 -1
@@ -21,7 +21,6 @@#include <linux/migrate.h> #include <linux/backing-dev.h> #include <linux/freezer.h> -#include <linux/sched/mm.h> #include "xfs_format.h" #include "xfs_log_format.h"
fs/xfs/xfs_linux.h +1 -0
@@ -26,6 +26,7 @@ typedef __u32 xfs_nlink_t;#include <linux/semaphore.h> #include <linux/mm.h> +#include <linux/sched/mm.h> #include <linux/kernel.h> #include <linux/blkdev.h> #include <linux/slab.h>
Signature · 1 line
2.17.0
Re: [PATCH 2/2] xfs: xfs_reflink_convert_cow() memory allocation deadlock
Quoted 62 lines · Dave Chinner
From: Dave Chinner <[email protected]>
xfs_reflink_convert_cow()manipulates the incore extent list inGFP_KERNELcontext in the IO submission path whilst holding locked pages under writeback. This is a memory reclaim deadlock vector. This code is not in a transaction, so any memory allocations it makes aren't protected via thememalloc_nofs_save()context that transactions carry.Hence we need to run this call under
memalloc_nofs_save()context to prevent potential memory allocations from being run asGFP_KERNELand deadlocking.Signed-off-by Dave Chinner <[email protected]>--- fs/xfs/xfs_aops.c | 11 +++++++++++ fs/xfs/xfs_buf.c | 1 - fs/xfs/xfs_linux.h | 1 + 3 files changed, 12 insertions(+), 1 deletion(-)fs/xfs/xfs_aops.c +11
-0@@ -531,8 +531,19 @@ xfs_submit_ioend( {/* Convert CoW extents to regular */ if (!status && ioend->io_type ==XFS_IO_COW ) { +/* +* Yuk. This can do memory allocation, but is not a +* transactional operation so everything is done in GFP_KERNEL +* context. That can deadlock, because we hold pages in +* writeback state and GFP_KERNEL allocations can block on them. +* Hence we must operate in nofs conditions here. +*/ +unsigned nofs_flag; + + nofs_flag =memalloc_nofs_save (); status =xfs_reflink_convert_cow (XFS_I (ioend->io_inode), ioend->io_offset, ioend->io_size); +memalloc_nofs_restore (nofs_flag); }/* Reserve log space if we might write beyond the on-disk inode size. */ fs/xfs/xfs_buf.c +0
-1@@ -21,7 +21,6 @@#include <linux/migrate.h> #include <linux/backing-dev.h> #include <linux/freezer.h> -#include <linux/sched/mm.h> #include "xfs_format.h" #include "xfs_log_format.h" fs/xfs/xfs_linux.h +0
-0
fs/xfs/xfs_linux.h typedef __u32 xfs_nlink_t; · line 32
Quoted 7 lines
fs/xfs/xfs_linux.h +1
-0@@ -26,6 +26,7 @@ typedef __u32 xfs_nlink_t;#include <linux/semaphore.h> #include <linux/mm.h> +#include <linux/sched/mm.h> #include <linux/kernel.h> #include <linux/blkdev.h> #include <linux/slab.h>
Looks, ok. Was moving the header include intentional? Just clean up maybe? Other than that, looks good.
Darrick J. Wong
Reviewed-byRe: [PATCH 2/2] xfs: xfs_reflink_convert_cow() memory allocation deadlock
Quoted 34 lines · Allison Henderson
Quoted 33 lines · Dave Chinner
From: Dave Chinner <[email protected]>
xfs_reflink_convert_cow()manipulates the incore extent list inGFP_KERNELcontext in the IO submission path whilst holding locked pages under writeback. This is a memory reclaim deadlock vector. This code is not in a transaction, so any memory allocations it makes aren't protected via thememalloc_nofs_save()context that transactions carry.Hence we need to run this call under
memalloc_nofs_save()context to prevent potential memory allocations from being run asGFP_KERNELand deadlocking.Signed-off-by Dave Chinner <[email protected]>--- fs/xfs/xfs_aops.c | 11 +++++++++++ fs/xfs/xfs_buf.c | 1 - fs/xfs/xfs_linux.h | 1 + 3 files changed, 12 insertions(+), 1 deletion(-)fs/xfs/xfs_aops.c +5
-0@@ -531,8 +531,19 @@ xfs_submit_ioend( {/* Convert CoW extents to regular */ if (!status && ioend->io_type ==XFS_IO_COW ) { +/* +* Yuk. This can do memory allocation, but is not a +* transactional operation so everything is done in GFP_KERNEL +* context. That can deadlock, because we hold pages in +* writeback state and GFP_KERNEL allocations can block on them.
Quoted 8 lines
fs/xfs/xfs_aops.c xfs_submit_ioend( · line 546
Quoted 8 lines
fs/xfs/xfs_aops.c +6
-0+* Hence we must operate in nofs conditions here. +*/ +unsigned nofs_flag; + + nofs_flag =memalloc_nofs_save (); status =xfs_reflink_convert_cow (XFS_I (ioend->io_inode), ioend->io_offset, ioend->io_size); +memalloc_nofs_restore (nofs_flag);
DOH. :)
Quoted 19 lines
Quoted 19 lines
fs/xfs/xfs_aops.c +0
-0}/* Reserve log space if we might write beyond the on-disk inode size. */ fs/xfs/xfs_buf.c +0
-1@@ -21,7 +21,6 @@#include <linux/migrate.h> #include <linux/backing-dev.h> #include <linux/freezer.h> -#include <linux/sched/mm.h> #include "xfs_format.h" #include "xfs_log_format.h" fs/xfs/xfs_linux.h +0
-0@@ -26,6 +26,7 @@ typedef __u32 xfs_nlink_t;#include <linux/semaphore.h>
Quoted 8 lines
fs/xfs/xfs_linux.h typedef __u32 xfs_nlink_t;
Quoted 6 lines
fs/xfs/xfs_linux.h +1
-0#include <linux/mm.h> +#include <linux/sched/mm.h> #include <linux/kernel.h> #include <linux/blkdev.h> #include <linux/slab.h> Looks, ok. Was moving the header include intentional? Just clean up maybe? Other than that, looks good.
I can't speak for Dave, but I'll point out that memalloc_nofs_restore is declared in linux/sched/mm.h, so the #include hoist makes the symbol available to the aops code in such a manner that now it's available to all the xfs code so that we don't have to remember this...
--D
Quoted 6 lines
Reviewed-by Allison Henderson <[email protected]>--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] xfs: xfs_reflink_convert_cow() memory allocation deadlock
Quoted 74 lines · Darrick J. Wong
Quoted 34 lines · Allison Henderson
Quoted 33 lines · Dave Chinner
From: Dave Chinner <[email protected]>
xfs_reflink_convert_cow()manipulates the incore extent list inGFP_KERNELcontext in the IO submission path whilst holding locked pages under writeback. This is a memory reclaim deadlock vector. This code is not in a transaction, so any memory allocations it makes aren't protected via thememalloc_nofs_save()context that transactions carry.Hence we need to run this call under
memalloc_nofs_save()context to prevent potential memory allocations from being run asGFP_KERNELand deadlocking.Signed-off-by Dave Chinner <[email protected]>--- fs/xfs/xfs_aops.c | 11 +++++++++++ fs/xfs/xfs_buf.c | 1 - fs/xfs/xfs_linux.h | 1 + 3 files changed, 12 insertions(+), 1 deletion(-)fs/xfs/xfs_aops.c +5
-0@@ -531,8 +531,19 @@ xfs_submit_ioend( {/* Convert CoW extents to regular */ if (!status && ioend->io_type ==XFS_IO_COW ) { +/* +* Yuk. This can do memory allocation, but is not a +* transactional operation so everything is done in GFP_KERNEL +* context. That can deadlock, because we hold pages in +* writeback state and GFP_KERNEL allocations can block on them. Quoted 8 lines
fs/xfs/xfs_aops.c xfs_submit_ioend( · line 546
Quoted 8 lines
fs/xfs/xfs_aops.c +6
-0+* Hence we must operate in nofs conditions here. +*/ +unsigned nofs_flag; + + nofs_flag =memalloc_nofs_save (); status =xfs_reflink_convert_cow (XFS_I (ioend->io_inode), ioend->io_offset, ioend->io_size); +memalloc_nofs_restore (nofs_flag);DOH. :)
Quoted 19 lines
Quoted 19 lines
fs/xfs/xfs_aops.c +0
-0}/* Reserve log space if we might write beyond the on-disk inode size. */ fs/xfs/xfs_buf.c +0
-1@@ -21,7 +21,6 @@#include <linux/migrate.h> #include <linux/backing-dev.h> #include <linux/freezer.h> -#include <linux/sched/mm.h> #include "xfs_format.h" #include "xfs_log_format.h" fs/xfs/xfs_linux.h +0
-0@@ -26,6 +26,7 @@ typedef __u32 xfs_nlink_t;#include <linux/semaphore.h> Quoted 8 lines
fs/xfs/xfs_linux.h typedef __u32 xfs_nlink_t;
Quoted 6 lines
fs/xfs/xfs_linux.h +1
-0#include <linux/mm.h> +#include <linux/sched/mm.h> #include <linux/kernel.h> #include <linux/blkdev.h> #include <linux/slab.h> Looks, ok. Was moving the header include intentional? Just clean up maybe? Other than that, looks good.
Quoted 4 lines
I can't speak for Dave, but I'll point out that
memalloc_nofs_restoreis declared inlinux/sched/mm.h, so the #include hoist makes the symbol available to the aops code in such a manner that now it's available to all the xfs code so that we don't have to remember this...
*nod*
That's historically how we've handled OS level includes - it keeps the XFS files to including XFS headers only and that makes things like userspace code syncs a lot easier.
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [PATCH 2/2] xfs: xfs_reflink_convert_cow() memory allocation deadlock
In reply to Dave ChinnerQuoted 9 lines · Dave Chinner
From: Dave Chinner <[email protected]>
xfs_reflink_convert_cow()manipulates the incore extent list inGFP_KERNELcontext in the IO submission path whilst holding locked pages under writeback. This is a memory reclaim deadlock vector. This code is not in a transaction, so any memory allocations it makes aren't protected via thememalloc_nofs_save()context that transactions carry.
Quoted 6 lines
Hence we need to run this call under
memalloc_nofs_save()context to prevent potential memory allocations from being run asGFP_KERNELand deadlocking.Signed-off-by Dave Chinner <[email protected]>---
Looks fine modulo the header thing Allison already pointed out:
BTW, shouldn't we also be using XFS_TRANS_NOFS inxfs_iomap_write_allocate()?
Brian
Quoted 60 lines
fs/xfs/xfs_aops.c | 11 +++++++++++ fs/xfs/xfs_buf.c | 1 - fs/xfs/xfs_linux.h | 1 + 3 files changed, 12 insertions(+), 1 deletion(-)fs/xfs/xfs_aops.c +11
-0@@ -531,8 +531,19 @@ xfs_submit_ioend( {/* Convert CoW extents to regular */ if (!status && ioend->io_type ==XFS_IO_COW ) { +/* +* Yuk. This can do memory allocation, but is not a +* transactional operation so everything is done in GFP_KERNEL +* context. That can deadlock, because we hold pages in +* writeback state and GFP_KERNEL allocations can block on them. +* Hence we must operate in nofs conditions here. +*/ +unsigned nofs_flag; + + nofs_flag =memalloc_nofs_save (); status =xfs_reflink_convert_cow (XFS_I (ioend->io_inode), ioend->io_offset, ioend->io_size); +memalloc_nofs_restore (nofs_flag); }/* Reserve log space if we might write beyond the on-disk inode size. */ fs/xfs/xfs_buf.c +0
-1@@ -21,7 +21,6 @@#include <linux/migrate.h> #include <linux/backing-dev.h> #include <linux/freezer.h> -#include <linux/sched/mm.h> #include "xfs_format.h" #include "xfs_log_format.h" fs/xfs/xfs_linux.h +1
-0@@ -26,6 +26,7 @@ typedef __u32 xfs_nlink_t;#include <linux/semaphore.h> #include <linux/mm.h> +#include <linux/sched/mm.h> #include <linux/kernel.h> #include <linux/blkdev.h> #include <linux/slab.h> Signature · 6 lines
2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] xfs: xfs_reflink_convert_cow() memory allocation deadlock
Quoted 17 lines · Brian Foster
Quoted 9 lines · Dave Chinner
From: Dave Chinner <[email protected]>
xfs_reflink_convert_cow()manipulates the incore extent list inGFP_KERNELcontext in the IO submission path whilst holding locked pages under writeback. This is a memory reclaim deadlock vector. This code is not in a transaction, so any memory allocations it makes aren't protected via thememalloc_nofs_save()context that transactions carry.Quoted 6 lines
Hence we need to run this call under
memalloc_nofs_save()context to prevent potential memory allocations from being run asGFP_KERNELand deadlocking.Signed-off-by Dave Chinner <[email protected]>---
Quoted 6 lines
Looks fine modulo the header thing Allison already pointed out:
Reviewed-by Brian Foster <[email protected]>BTW, shouldn't we also be using
XFS_TRANS_NOFSinxfs_iomap_write_allocate()?
/me squints at the usage and thinks ... yes probably. :)
--D
Quoted 66 lines
Brian
Quoted 60 lines
fs/xfs/xfs_aops.c | 11 +++++++++++ fs/xfs/xfs_buf.c | 1 - fs/xfs/xfs_linux.h | 1 + 3 files changed, 12 insertions(+), 1 deletion(-)fs/xfs/xfs_aops.c +11
-0@@ -531,8 +531,19 @@ xfs_submit_ioend( {/* Convert CoW extents to regular */ if (!status && ioend->io_type ==XFS_IO_COW ) { +/* +* Yuk. This can do memory allocation, but is not a +* transactional operation so everything is done in GFP_KERNEL +* context. That can deadlock, because we hold pages in +* writeback state and GFP_KERNEL allocations can block on them. +* Hence we must operate in nofs conditions here. +*/ +unsigned nofs_flag; + + nofs_flag =memalloc_nofs_save (); status =xfs_reflink_convert_cow (XFS_I (ioend->io_inode), ioend->io_offset, ioend->io_size); +memalloc_nofs_restore (nofs_flag); }/* Reserve log space if we might write beyond the on-disk inode size. */ fs/xfs/xfs_buf.c +0
-1@@ -21,7 +21,6 @@#include <linux/migrate.h> #include <linux/backing-dev.h> #include <linux/freezer.h> -#include <linux/sched/mm.h> #include "xfs_format.h" #include "xfs_log_format.h" fs/xfs/xfs_linux.h +1
-0@@ -26,6 +26,7 @@ typedef __u32 xfs_nlink_t;#include <linux/semaphore.h> #include <linux/mm.h> +#include <linux/sched/mm.h> #include <linux/kernel.h> #include <linux/blkdev.h> #include <linux/slab.h> Signature · 6 lines
2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] xfs: xfs_reflink_convert_cow() memory allocation deadlock
In reply to Brian FosterQuoted 17 lines · Brian Foster
Quoted 9 lines · Dave Chinner
From: Dave Chinner <[email protected]>
xfs_reflink_convert_cow()manipulates the incore extent list inGFP_KERNELcontext in the IO submission path whilst holding locked pages under writeback. This is a memory reclaim deadlock vector. This code is not in a transaction, so any memory allocations it makes aren't protected via thememalloc_nofs_save()context that transactions carry.Quoted 6 lines
Hence we need to run this call under
memalloc_nofs_save()context to prevent potential memory allocations from being run asGFP_KERNELand deadlocking.Signed-off-by Dave Chinner <[email protected]>---
Quoted 6 lines
Looks fine modulo the header thing Allison already pointed out:
Reviewed-by Brian Foster <[email protected]>BTW, shouldn't we also be using
XFS_TRANS_NOFSinxfs_iomap_write_allocate()?
Most likely, yes.
However, I think the whole ->writepages path should be moved under memalloc_nofs_save() context. I'm kinda waiting for the bufferhead removal to land before auditing it fully and determining the scope we should be covering by nofs alloc contexts. This was just a drive-by patch to fix a problem that was reported...
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]