On Thu, Jan 04, 2024 at 09:17:16PM +0000, Matthew Wilcox wrote:
> This is primarily a _FILESYSTEM_ track topic. All the work has already
> been done on the MM side; the FS people need to do their part. It could
> be a joint session, but I'm not sure there's much for the MM people
> to say.
>
> There are situations where we need to allocate memory, but cannot call
> into the filesystem to free memory. Generally this is because we're
> holding a lock or we've started a transaction, and attempting to write
> out dirty folios to reclaim memory would result in a deadlock.
>
> The old way to solve this problem is to specify GFP_NOFS when allocating
> memory. This conveys little information about what is being protected
> against, and so it is hard to know when it might be safe to remove.
> It's also a reflex -- many filesystem authors use GFP_NOFS by default
> even when they could use GFP_KERNEL because there's no risk of deadlock.
There are many uses in XFS where GFP_NOFS has been used because
__GFP_NOLOCKDEP did not exist. A large number of the remaining
GFP_NOFS and KM_NOFS uses in XFS fall under this category.
As a first step, I have a patchset that gets rid of KM_NOFS and
replaces it with either GFP_NOFS or __GFP_NOLOCKDEP:
$ git grep "GFP_NOFS\|KM_NOFS" fs/xfs |wc -l
64
$ git checkout guilt/xfs-kmem-cleanup
Switched to branch 'guilt/xfs-kmem-cleanup'
$ git grep "GFP_NOFS\|KM_NOFS" fs/xfs |wc -l
21
Some of these are in newly merged code that I haven't updated the
patch set to handle yet, others are in kthread/kworker contexts that
don't inherit any allocation context information. There isn't any
big issues remaining to be fixed in XFS, though.
> The new way is to use the scoped APIs -- memalloc_nofs_save() and
> memalloc_nofs_restore(). These should be called when we start a
> transaction or take a lock that would cause a GFP_KERNEL allocation to
> deadlock. Then just use GFP_KERNEL as normal. The memory allocators
> can see the nofs situation is in effect and will not call back into
> the filesystem.
Note that this is the only way to use vmalloc() safely with GFP_NOFS
context...
> This results in better code within your filesystem as you don't need to
> pass around gfp flags as much, and can lead to better performance from
> the memory allocators as GFP_NOFS will not be used unnecessarily.
>
> The memalloc_nofs APIs were introduced in May 2017, but we still have
For everyone else who doesn't know the history of this, the scoped
GFP_NOFS allocation code has been around for a lot longer than this
current API. PF_FSTRANS was added in early 2002 so we didn't have to
hack magic flags into current->journal_info to defermine if we were
in a transaction, and then this was added:
commit 957568938d4030414d71c583bc261fe3558d2c17
Author: Steve Lord <lord@xxxxxxx>
Date: Thu Jan 31 11:17:26 2002 +0000
Use PF_FSTRANS to detect being in a transaction
diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c
index 08a17984..282b724f 100644
--- a/fs/xfs/linux-2.6/xfs_super.c
+++ b/fs/xfs/linux-2.6/xfs_super.c
@@ -396,16 +396,11 @@ linvfs_release_buftarg(
static kmem_cache_t * linvfs_inode_cachep;
-#define XFS_TRANS_MAGIC 0x5452414E
-
static __inline__ unsigned int gfp_mask(void)
{
/* If we're not in a transaction, FS activity is ok */
- if (!current->journal_info) return GFP_KERNEL;
- /* could be set from some other filesystem */
- if ((int)current->journal_info != XFS_TRANS_MAGIC)
- return GFP_KERNEL;
- return GFP_NOFS;
+ if (current->flags & PF_FSTRANS) return GFP_NOFS;
+ return GFP_KERNEL;
}
> over 1000 uses of GFP_NOFS in fs/ today (and 200 outside fs/, which is
> really sad). This session is for filesystem developers to talk about
> what they need to do to fix up their own filesystem, or share stories
> about how they made their filesystem better by adopting the new APIs.
>
> My interest in this is that I'd like to get rid of the FGP_NOFS flag.
Isn't that flag redundant? i.e. we already have mapping_gfp_mask()
to indicate what gfp mask should be used with the mapping
operations, and at least the iomap code uses that.
Many filesystems call mapping_set_gfp_mask(GFP_NOFS) already, XFS is
the special one that does:
mapping_set_gfp_mask(inode->i_mapping, (gfp_mask & ~(__GFP_FS)));
so it doesn't actually use GFP_NOFS there.
Given that we already have a generic way of telling mapping
operations the scoped allocation context they should run under,
perhaps we could turn this into scoped context calls somewhere in
the generic IO/mapping operation paths? e.g.
call_read_iter()/call_write_iter()
> It'd also be good to get rid of the __GFP_FS flag since there's always
> demand for more GFP flags. I have a git branch with some work in this
> area, so there's a certain amount of conference-driven development going
> on here too.
Worry about that when everything is using scoped contexted. Then
nobody will be using GFP_NOFS or __GFP_FS externally, and the
allocator can then reclaim the flag.
> We could mutatis mutandi for GFP_NOIO, memalloc_noio_save/restore,
> __GFP_IO, etc, so maybe the block people are also interested. I haven't
> looked into that in any detail though. I guess we'll see what interest
> this topic gains.
That seems a whole lot simpler - just set the GFP_NOIO scope at
entry to the block layer and that should cover a large percentage of
the GFP_NOIO allocations...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
