On Sun 28-01-24 14:43:16, Kent Overstreet wrote:
> On Sun, Jan 28, 2024 at 04:45:32PM +0100, Michal Hocko wrote:
> > On Fri 26-01-24 17:07:56, Kent Overstreet wrote:
> > > If we're using PF_MEMALLOC, we might have a fallback and might not to
> > > warn about a failing allocation - thus we need a PF_* equivalent of
> > > __GFP_NOWARN.
> >
> > Could you be more specific about the user? Is this an allocation from
> > the reclaim path or an explicit PF_MEMALLOC one? It would be also really
> > helpful to explain why GFP_NOWARN cannot be used directly.
>
> Explicit PF_MEMALLOC.
>
> It's for a call to alloc_inode(), which doesn't take gfp flags, and
> plumbing it would require modifying a s_ops callback and would touch
> every filesystem -
OK, I see. This should be part of the changelog.
> but we want to get away from gfp flags anyways :)
>
> More specifically, the code where I'm using it is doing a "try
> GFP_NOWAIT first; if that fails drop locks and do GFP_KERNEL" dance;
> it's part of a cleanup for some weird lifetime stuff related to
> fs/inode.c.
>
> #define memalloc_flags_do(_flags, _do) \
> ({ \
> unsigned _saved_flags = memalloc_flags_save(_flags); \
> typeof(_do) _ret = _do; \
> memalloc_noreclaim_restore(_saved_flags); \
> _ret; \
> })
>
> /*
> * Allocate a new inode, dropping/retaking btree locks if necessary:
> */
> static struct bch_inode_info *bch2_new_inode(struct btree_trans *trans)
> {
> struct bch_fs *c = trans->c;
>
> struct bch_inode_info *inode =
> memalloc_flags_do(PF_MEMALLOC|PF_MEMALLOC_NOWARN, to_bch_ei(new_inode(c->vfs_sb)));
Is this what you meant by GFP_NOWAIT allocation? You would be right
about this allocation not entering the direct reclaim but do you realize
this is not an ordinary NOWAIT request beause PF_MEMALLOC will allow to
dip into memory reserves without any limits? (Unless the specific
allocation down the road is explicitly GFP_NOMEMALLOC)
A failure here means that the system is really in a desparate state with all
the memory reserves gone which migt break reclaimers who rely on those
reserves.
Are you sure it is a good idea? Unless I am missing something you are
just giving an ordinary user access to those reserves by creating inodes
without any bounds.
--
Michal Hocko
SUSE Labs
