| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20100824121549.GB3713@quack.suse.cz>
Date: Tue, 24 Aug 2010 14:15:51 +0200
From: Jan Kara <jack@...e.cz>
To: David Rientjes <rientjes@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Neil Brown <neilb@...e.de>, Alasdair G Kergon <agk@...hat.com>,
Chris Mason <chris.mason@...cle.com>,
Steven Whitehouse <swhiteho@...hat.com>,
Jens Axboe <jaxboe@...ionio.com>, Jan Kara <jack@...e.cz>,
Frederic Weisbecker <fweisbec@...il.com>,
linux-raid@...r.kernel.org, linux-btrfs@...r.kernel.org,
cluster-devel@...hat.com, linux-ext4@...r.kernel.org,
reiserfs-devel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [patch 1/5] mm: add nofail variants of kmalloc kcalloc and
kzalloc
On Tue 24-08-10 03:50:19, David Rientjes wrote:
> Add kmalloc_nofail(), kcalloc_nofail(), and kzalloc_nofail(). These
> functions are equivalent to kmalloc(), kcalloc(), and kzalloc(),
> respectively, except that they will never return NULL and instead loop
> forever trying to allocate memory.
>
> If the first allocation attempt fails, a warning will be emitted,
> including a call trace. Subsequent failures will suppress this warning.
>
> These were added as helper functions for documentation and auditability.
> No future callers should be added.
This looks like a cleaner approach than the previous one. You can add
Acked-by: Jan Kara <jack@...e.cz>
for the JBD part.
Honza
> Signed-off-by: David Rientjes <rientjes@...gle.com>
> ---
> drivers/md/dm-region-hash.c | 2 +-
> fs/btrfs/inode.c | 2 +-
> fs/gfs2/log.c | 2 +-
> fs/gfs2/rgrp.c | 18 ++++---------
> fs/jbd/transaction.c | 11 ++------
> fs/reiserfs/journal.c | 3 +-
> include/linux/slab.h | 55 +++++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 68 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/md/dm-region-hash.c b/drivers/md/dm-region-hash.c
> --- a/drivers/md/dm-region-hash.c
> +++ b/drivers/md/dm-region-hash.c
> @@ -290,7 +290,7 @@ static struct dm_region *__rh_alloc(struct dm_region_hash *rh, region_t region)
>
> nreg = mempool_alloc(rh->region_pool, GFP_ATOMIC);
> if (unlikely(!nreg))
> - nreg = kmalloc(sizeof(*nreg), GFP_NOIO | __GFP_NOFAIL);
> + nreg = kmalloc_nofail(sizeof(*nreg), GFP_NOIO);
>
> nreg->state = rh->log->type->in_sync(rh->log, region, 1) ?
> DM_RH_CLEAN : DM_RH_NOSYNC;
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1967,7 +1967,7 @@ void btrfs_add_delayed_iput(struct inode *inode)
> if (atomic_add_unless(&inode->i_count, -1, 1))
> return;
>
> - delayed = kmalloc(sizeof(*delayed), GFP_NOFS | __GFP_NOFAIL);
> + delayed = kmalloc_nofail(sizeof(*delayed), GFP_NOFS);
> delayed->inode = inode;
>
> spin_lock(&fs_info->delayed_iput_lock);
> diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
> --- a/fs/gfs2/log.c
> +++ b/fs/gfs2/log.c
> @@ -709,7 +709,7 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl)
> }
> trace_gfs2_log_flush(sdp, 1);
>
> - ai = kzalloc(sizeof(struct gfs2_ail), GFP_NOFS | __GFP_NOFAIL);
> + ai = kzalloc_nofail(sizeof(struct gfs2_ail), GFP_NOFS);
> INIT_LIST_HEAD(&ai->ai_ail1_list);
> INIT_LIST_HEAD(&ai->ai_ail2_list);
>
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -1440,8 +1440,8 @@ static struct gfs2_rgrpd *rgblk_free(struct gfs2_sbd *sdp, u64 bstart,
> rgrp_blk++;
>
> if (!bi->bi_clone) {
> - bi->bi_clone = kmalloc(bi->bi_bh->b_size,
> - GFP_NOFS | __GFP_NOFAIL);
> + bi->bi_clone = kmalloc_nofail(bi->bi_bh->b_size,
> + GFP_NOFS);
> memcpy(bi->bi_clone + bi->bi_offset,
> bi->bi_bh->b_data + bi->bi_offset,
> bi->bi_len);
> @@ -1759,9 +1759,6 @@ fail:
> * @block: the block
> *
> * Figure out what RG a block belongs to and add that RG to the list
> - *
> - * FIXME: Don't use NOFAIL
> - *
> */
>
> void gfs2_rlist_add(struct gfs2_sbd *sdp, struct gfs2_rgrp_list *rlist,
> @@ -1789,8 +1786,8 @@ void gfs2_rlist_add(struct gfs2_sbd *sdp, struct gfs2_rgrp_list *rlist,
> if (rlist->rl_rgrps == rlist->rl_space) {
> new_space = rlist->rl_space + 10;
>
> - tmp = kcalloc(new_space, sizeof(struct gfs2_rgrpd *),
> - GFP_NOFS | __GFP_NOFAIL);
> + tmp = kcalloc_nofail(new_space, sizeof(struct gfs2_rgrpd *),
> + GFP_NOFS);
>
> if (rlist->rl_rgd) {
> memcpy(tmp, rlist->rl_rgd,
> @@ -1811,17 +1808,14 @@ void gfs2_rlist_add(struct gfs2_sbd *sdp, struct gfs2_rgrp_list *rlist,
> * @rlist: the list of resource groups
> * @state: the lock state to acquire the RG lock in
> * @flags: the modifier flags for the holder structures
> - *
> - * FIXME: Don't use NOFAIL
> - *
> */
>
> void gfs2_rlist_alloc(struct gfs2_rgrp_list *rlist, unsigned int state)
> {
> unsigned int x;
>
> - rlist->rl_ghs = kcalloc(rlist->rl_rgrps, sizeof(struct gfs2_holder),
> - GFP_NOFS | __GFP_NOFAIL);
> + rlist->rl_ghs = kcalloc_nofail(rlist->rl_rgrps,
> + sizeof(struct gfs2_holder), GFP_NOFS);
> for (x = 0; x < rlist->rl_rgrps; x++)
> gfs2_holder_init(rlist->rl_rgd[x]->rd_gl,
> state, 0,
> diff --git a/fs/jbd/transaction.c b/fs/jbd/transaction.c
> --- a/fs/jbd/transaction.c
> +++ b/fs/jbd/transaction.c
> @@ -98,14 +98,9 @@ static int start_this_handle(journal_t *journal, handle_t *handle)
> }
>
> alloc_transaction:
> - if (!journal->j_running_transaction) {
> - new_transaction = kzalloc(sizeof(*new_transaction),
> - GFP_NOFS|__GFP_NOFAIL);
> - if (!new_transaction) {
> - ret = -ENOMEM;
> - goto out;
> - }
> - }
> + if (!journal->j_running_transaction)
> + new_transaction = kzalloc_nofail(sizeof(*new_transaction),
> + GFP_NOFS);
>
> jbd_debug(3, "New handle %p going live.\n", handle);
>
> diff --git a/fs/reiserfs/journal.c b/fs/reiserfs/journal.c
> --- a/fs/reiserfs/journal.c
> +++ b/fs/reiserfs/journal.c
> @@ -2593,8 +2593,7 @@ static int journal_read(struct super_block *sb)
> static struct reiserfs_journal_list *alloc_journal_list(struct super_block *s)
> {
> struct reiserfs_journal_list *jl;
> - jl = kzalloc(sizeof(struct reiserfs_journal_list),
> - GFP_NOFS | __GFP_NOFAIL);
> + jl = kzalloc_nofail(sizeof(struct reiserfs_journal_list), GFP_NOFS);
> INIT_LIST_HEAD(&jl->j_list);
> INIT_LIST_HEAD(&jl->j_working_list);
> INIT_LIST_HEAD(&jl->j_tail_bh_list);
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -334,6 +334,61 @@ static inline void *kzalloc_node(size_t size, gfp_t flags, int node)
> return kmalloc_node(size, flags | __GFP_ZERO, node);
> }
>
> +/**
> + * kmalloc_nofail - infinitely loop until kmalloc() succeeds.
> + * @size: how many bytes of memory are required.
> + * @flags: the type of memory to allocate (see kmalloc).
> + *
> + * NOTE: no new callers of this function should be implemented!
> + * All memory allocations should be failable whenever possible.
> + */
> +static inline void *kmalloc_nofail(size_t size, gfp_t flags)
> +{
> + void *ret;
> +
> + for (;;) {
> + ret = kmalloc(size, flags);
> + if (ret)
> + return ret;
> + WARN_ONCE(1, "Out of memory, no fallback implemented "
> + "(size=%lu flags=0x%x)\n",
> + size, flags);
> + }
> +}
> +
> +/**
> + * kcalloc_nofail - infinitely loop until kcalloc() succeeds.
> + * @n: number of elements.
> + * @size: element size.
> + * @flags: the type of memory to allocate (see kcalloc).
> + *
> + * NOTE: no new callers of this function should be implemented!
> + * All memory allocations should be failable whenever possible.
> + */
> +static inline void *kcalloc_nofail(size_t n, size_t size, gfp_t flags)
> +{
> + void *ret;
> +
> + for (;;) {
> + ret = kcalloc(n, size, flags);
> + if (ret)
> + return ret;
> + WARN_ONCE(1, "Out of memory, no fallback implemented "
> + "(n=%lu size=%lu flags=0x%x)\n",
> + n, size, flags);
> + }
> +}
> +
> +/**
> + * kzalloc_nofail - infinitely loop until kzalloc() succeeds.
> + * @size: how many bytes of memory are required.
> + * @flags: the type of memory to allocate (see kzalloc).
> + */
> +static inline void *kzalloc_nofail(size_t size, gfp_t flags)
> +{
> + return kmalloc_nofail(size, flags | __GFP_ZERO);
> +}
> +
> void __init kmem_cache_init_late(void);
>
> #endif /* _LINUX_SLAB_H */
--
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists