| Message ID | 1506543239-31470-2-git-send-email-axboe@kernel.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 27.09.2017 23:13, 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@kernel.dk> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > 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); >
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@kernel.dk> Looks good. You can add: Reviewed-by: Jan Kara <jack@suse.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 >
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);
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@kernel.dk> --- 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(-)