| 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: <20171002150257.GC11879@quack2.suse.cz>
Date: Mon, 2 Oct 2017 17:02:57 +0200
From: Jan Kara <jack@...e.cz>
To: Jens Axboe <axboe@...nel.dk>
Cc: linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
hannes@...xchg.org, jack@...e.cz, torvalds@...ux-foundation.org
Subject: Re: [PATCH 01/12] buffer: have alloc_page_buffers() use __GFP_NOFAIL
On Wed 27-09-17 14:13:48, Jens Axboe wrote:
> Instead of adding weird retry logic in that function, utilize
> __GFP_NOFAIL to ensure that the vm takes care of handling any
> potential retries appropriately. This means we don't have to
> call free_more_memory() from here.
>
> Signed-off-by: Jens Axboe <axboe@...nel.dk>
Looks good. You can add:
Reviewed-by: Jan Kara <jack@...e.cz>
Honza
> ---
> drivers/md/bitmap.c | 2 +-
> fs/buffer.c | 33 ++++++++++-----------------------
> fs/ntfs/aops.c | 2 +-
> fs/ntfs/mft.c | 2 +-
> include/linux/buffer_head.h | 2 +-
> 5 files changed, 14 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> index d2121637b4ab..4d8ed74efadf 100644
> --- a/drivers/md/bitmap.c
> +++ b/drivers/md/bitmap.c
> @@ -368,7 +368,7 @@ static int read_page(struct file *file, unsigned long index,
> pr_debug("read bitmap file (%dB @ %llu)\n", (int)PAGE_SIZE,
> (unsigned long long)index << PAGE_SHIFT);
>
> - bh = alloc_page_buffers(page, 1<<inode->i_blkbits, 0);
> + bh = alloc_page_buffers(page, 1<<inode->i_blkbits, false);
> if (!bh) {
> ret = -ENOMEM;
> goto out;
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 170df856bdb9..1234ae343aef 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -861,16 +861,19 @@ int remove_inode_buffers(struct inode *inode)
> * which may not fail from ordinary buffer allocations.
> */
> struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size,
> - int retry)
> + bool retry)
> {
> struct buffer_head *bh, *head;
> + gfp_t gfp = GFP_NOFS;
> long offset;
>
> -try_again:
> + if (retry)
> + gfp |= __GFP_NOFAIL;
> +
> head = NULL;
> offset = PAGE_SIZE;
> while ((offset -= size) >= 0) {
> - bh = alloc_buffer_head(GFP_NOFS);
> + bh = alloc_buffer_head(gfp);
> if (!bh)
> goto no_grow;
>
> @@ -896,23 +899,7 @@ struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size,
> } while (head);
> }
>
> - /*
> - * Return failure for non-async IO requests. Async IO requests
> - * are not allowed to fail, so we have to wait until buffer heads
> - * become available. But we don't want tasks sleeping with
> - * partially complete buffers, so all were released above.
> - */
> - if (!retry)
> - return NULL;
> -
> - /* We're _really_ low on memory. Now we just
> - * wait for old buffer heads to become free due to
> - * finishing IO. Since this is an async request and
> - * the reserve list is empty, we're sure there are
> - * async buffer heads in use.
> - */
> - free_more_memory();
> - goto try_again;
> + return NULL;
> }
> EXPORT_SYMBOL_GPL(alloc_page_buffers);
>
> @@ -1021,7 +1008,7 @@ grow_dev_page(struct block_device *bdev, sector_t block,
> /*
> * Allocate some buffers for this page
> */
> - bh = alloc_page_buffers(page, size, 0);
> + bh = alloc_page_buffers(page, size, false);
> if (!bh)
> goto failed;
>
> @@ -1575,7 +1562,7 @@ void create_empty_buffers(struct page *page,
> {
> struct buffer_head *bh, *head, *tail;
>
> - head = alloc_page_buffers(page, blocksize, 1);
> + head = alloc_page_buffers(page, blocksize, true);
> bh = head;
> do {
> bh->b_state |= b_state;
> @@ -2638,7 +2625,7 @@ int nobh_write_begin(struct address_space *mapping,
> * Be careful: the buffer linked list is a NULL terminated one, rather
> * than the circular one we're used to.
> */
> - head = alloc_page_buffers(page, blocksize, 0);
> + head = alloc_page_buffers(page, blocksize, false);
> if (!head) {
> ret = -ENOMEM;
> goto out_release;
> diff --git a/fs/ntfs/aops.c b/fs/ntfs/aops.c
> index cc91856b5e2d..3a2e509c77c5 100644
> --- a/fs/ntfs/aops.c
> +++ b/fs/ntfs/aops.c
> @@ -1739,7 +1739,7 @@ void mark_ntfs_record_dirty(struct page *page, const unsigned int ofs) {
> spin_lock(&mapping->private_lock);
> if (unlikely(!page_has_buffers(page))) {
> spin_unlock(&mapping->private_lock);
> - bh = head = alloc_page_buffers(page, bh_size, 1);
> + bh = head = alloc_page_buffers(page, bh_size, true);
> spin_lock(&mapping->private_lock);
> if (likely(!page_has_buffers(page))) {
> struct buffer_head *tail;
> diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
> index b6f402194f02..ee8392aee9f6 100644
> --- a/fs/ntfs/mft.c
> +++ b/fs/ntfs/mft.c
> @@ -507,7 +507,7 @@ int ntfs_sync_mft_mirror(ntfs_volume *vol, const unsigned long mft_no,
> if (unlikely(!page_has_buffers(page))) {
> struct buffer_head *tail;
>
> - bh = head = alloc_page_buffers(page, blocksize, 1);
> + bh = head = alloc_page_buffers(page, blocksize, true);
> do {
> set_buffer_uptodate(bh);
> tail = bh;
> diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
> index c8dae555eccf..ae2d25f01b98 100644
> --- a/include/linux/buffer_head.h
> +++ b/include/linux/buffer_head.h
> @@ -156,7 +156,7 @@ void set_bh_page(struct buffer_head *bh,
> struct page *page, unsigned long offset);
> int try_to_free_buffers(struct page *);
> struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size,
> - int retry);
> + bool retry);
> void create_empty_buffers(struct page *, unsigned long,
> unsigned long b_state);
> void end_buffer_read_sync(struct buffer_head *bh, int uptodate);
> --
> 2.7.4
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists