History

mm, fs: obey gfp_mapping for add_to_page_cache()

Commit 6afdb859b710 ("mm: do not ignore mapping_gfp_mask in page cache allocation paths") has caught some users of hardcoded GFP_KERNEL used in the page cache allocation paths. This, however, wasn't complete and there were others which went unnoticed. Dave Chinner has reported the following deadlock for xfs on loop device: : With the recent merge of the loop device changes, I'm now seeing : XFS deadlock on my single CPU, 1GB RAM VM running xfs/073. : : The deadlocked is as follows: : : kloopd1: loop_queue_read_work : xfs_file_iter_read : lock XFS inode XFS_IOLOCK_SHARED (on image file) : page cache read (GFP_KERNEL) : radix tree alloc : memory reclaim : reclaim XFS inodes : log force to unpin inodes : <wait for log IO completion> : : xfs-cil/loop1: <does log force IO work> : xlog_cil_push : xlog_write : <loop issuing log writes> : xlog_state_get_iclog_space() : <blocks due to all log buffers under write io> : <waits for IO completion> : : kloopd1: loop_queue_write_work : xfs_file_write_iter : lock XFS inode XFS_IOLOCK_EXCL (on image file) : <wait for inode to be unlocked> : : i.e. the kloopd, with it's split read and write work queues, has : introduced a dependency through memory reclaim. i.e. that writes : need to be able to progress for reads make progress. : : The problem, fundamentally, is that mpage_readpages() does a : GFP_KERNEL allocation, rather than paying attention to the inode's : mapping gfp mask, which is set to GFP_NOFS. : : The didn't used to happen, because the loop device used to issue : reads through the splice path and that does: : : error = add_to_page_cache_lru(page, mapping, index, : GFP_KERNEL & mapping_gfp_mask(mapping)); This has changed by commit aa4d86163e4 ("block: loop: switch to VFS ITER_BVEC"). This patch changes mpage_readpage{s} to follow gfp mask set for the mapping. There are, however, other places which are doing basically the same. lustre:ll_dir_filler is doing GFP_KERNEL from the function which apparently uses GFP_NOFS for other allocations so let's make this consistent. cifs:readpages_get_pages is called from cifs_readpages and __cifs_readpages_from_fscache called from the same path obeys mapping gfp. ramfs_nommu_expand_for_mapping is hardcoding GFP_KERNEL as well regardless it uses mapping_gfp_mask for the page allocation. ext4_mpage_readpages is the called from the page cache allocation path same as read_pages and read_cache_pages As I've noticed in my previous post I cannot say I would be happy about sprinkling mapping_gfp_mask all over the place and it sounds like we should drop gfp_mask argument altogether and use it internally in __add_to_page_cache_locked that would require all the filesystems to use mapping gfp consistently which I am not sure is the case here. From a quick glance it seems that some file system use it all the time while others are selective.

Michal Hocko committed 11 years ago

Commit 063d99b1 parent 5b5f1456 files · +22 −18Browse files

Unified · Split

6 files
  1. drivers/staging/lustre/lustre/llite/dir.c +1 −1
  2. fs/cifs/file.c +3 −3
  3. fs/ext4/readpage.c +2 −2
  4. fs/mpage.c +9 −6
  5. fs/ramfs/file-nommu.c +3 −2
  6. mm/readahead.c +4 −4
drivers/staging/lustre/lustre/llite/dir.c+1−1
@@ -224,7 +224,7 @@ static int ll_dir_filler(void *_hash, struct page *page0) · 223 unmodified lines
224224
225225 prefetchw(&page->flags);
226226 ret = add_to_page_cache_lru(page, inode->i_mapping, offset,
227− GFP_KERNEL);
227+ GFP_NOFS);
228228 if (ret == 0) {
229229 unlock_page(page);
230230 } else {
fs/cifs/file.c+3−3
@@ -3380,6 +3380,7 @@ readpages_get_pages(struct address_space *mapping, struct list_head *page_list, · 3379 unmodified lines
33803380 struct page *page, *tpage;
33813381 unsigned int expected_index;
33823382 int rc;
3383+ gfp_t gfp = GFP_KERNEL & mapping_gfp_mask(mapping);
33833384
33843385 INIT_LIST_HEAD(tmplist);
33853386
@@ -3392,7 +3393,7 @@ readpages_get_pages(struct address_space *mapping, struct list_head *page_list, · 6 unmodified lines
33923393 */
33933394 __set_page_locked(page);
33943395 rc = add_to_page_cache_locked(page, mapping,
3395− page->index, GFP_KERNEL);
3396+ page->index, gfp);
33963397
33973398 /* give up if we can't stick it in the cache */
33983399 if (rc) {
@@ -3418,8 +3419,7 @@ readpages_get_pages(struct address_space *mapping, struct list_head *page_list, · 19 unmodified lines
34183419 break;
34193420
34203421 __set_page_locked(page);
3421− if (add_to_page_cache_locked(page, mapping, page->index,
3422− GFP_KERNEL)) {
3422+ if (add_to_page_cache_locked(page, mapping, page->index, gfp)) {
34233423 __clear_page_locked(page);
34243424 break;
34253425 }
fs/ext4/readpage.c+2−2
@@ -165,8 +165,8 @@ int ext4_mpage_readpages(struct address_space *mapping, · 164 unmodified lines
165165 if (pages) {
166166 page = list_entry(pages->prev, struct page, lru);
167167 list_del(&page->lru);
168− if (add_to_page_cache_lru(page, mapping,
169− page->index, GFP_KERNEL))
168+ if (add_to_page_cache_lru(page, mapping, page->index,
169+ GFP_KERNEL & mapping_gfp_mask(mapping)))
170170 goto next_page;
171171 }
172172
fs/mpage.c+9−6
@@ -139,7 +139,8 @@ map_buffer_to_page(struct page *page, struct buffer_head *bh, int page_block) · 138 unmodified lines
139139 static struct bio *
140140 do_mpage_readpage(struct bio *bio, struct page *page, unsigned nr_pages,
141141 sector_t *last_block_in_bio, struct buffer_head *map_bh,
142− unsigned long *first_logical_block, get_block_t get_block)
142+ unsigned long *first_logical_block, get_block_t get_block,
143+ gfp_t gfp)
143144 {
144145 struct inode *inode = page->mapping->host;
145146 const unsigned blkbits = inode->i_blkbits;
@@ -277,8 +278,7 @@ alloc_new: · 131 unmodified lines
277278 goto out;
278279 }
279280 bio = mpage_alloc(bdev, blocks[0] << (blkbits - 9),
280− min_t(int, nr_pages, BIO_MAX_PAGES),
281− GFP_KERNEL);
281+ min_t(int, nr_pages, BIO_MAX_PAGES), gfp);
282282 if (bio == NULL)
283283 goto confused;
284284 }
@@ -361,6 +361,7 @@ mpage_readpages(struct address_space *mapping, struct list_head *pages, · 76 unmodified lines
361361 sector_t last_block_in_bio = 0;
362362 struct buffer_head map_bh;
363363 unsigned long first_logical_block = 0;
364+ gfp_t gfp = GFP_KERNEL & mapping_gfp_mask(mapping);
364365
365366 map_bh.b_state = 0;
366367 map_bh.b_size = 0;
@@ -370,12 +371,13 @@ mpage_readpages(struct address_space *mapping, struct list_head *pages, · 3 unmodified lines
370371 prefetchw(&page->flags);
371372 list_del(&page->lru);
372373 if (!add_to_page_cache_lru(page, mapping,
373− page->index, GFP_KERNEL)) {
374+ page->index,
375+ gfp)) {
374376 bio = do_mpage_readpage(bio, page,
375377 nr_pages - page_idx,
376378 &last_block_in_bio, &map_bh,
377379 &first_logical_block,
378− get_block);
380+ get_block, gfp);
379381 }
380382 page_cache_release(page);
381383 }
@@ -395,11 +397,12 @@ int mpage_readpage(struct page *page, get_block_t get_block) · 13 unmodified lines
395397 sector_t last_block_in_bio = 0;
396398 struct buffer_head map_bh;
397399 unsigned long first_logical_block = 0;
400+ gfp_t gfp = GFP_KERNEL & mapping_gfp_mask(page->mapping);
398401
399402 map_bh.b_state = 0;
400403 map_bh.b_size = 0;
401404 bio = do_mpage_readpage(bio, page, 1, &last_block_in_bio,
402− &map_bh, &first_logical_block, get_block);
405+ &map_bh, &first_logical_block, get_block, gfp);
403406 if (bio)
404407 mpage_bio_submit(READ, bio);
405408 return 0;
fs/ramfs/file-nommu.c+3−2
@@ -70,6 +70,7 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize) · 69 unmodified lines
7070 unsigned order;
7171 void *data;
7272 int ret;
73+ gfp_t gfp = mapping_gfp_mask(inode->i_mapping);
7374
7475 /* make various checks */
7576 order = get_order(newsize);
@@ -84,7 +85,7 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize) · 8 unmodified lines
8485
8586 /* allocate enough contiguous pages to be able to satisfy the
8687 * request */
87− pages = alloc_pages(mapping_gfp_mask(inode->i_mapping), order);
88+ pages = alloc_pages(gfp, order);
8889 if (!pages)
8990 return -ENOMEM;
9091
@@ -108,7 +109,7 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize) · 17 unmodified lines
108109 struct page *page = pages + loop;
109110
110111 ret = add_to_page_cache_lru(page, inode->i_mapping, loop,
111− GFP_KERNEL);
112+ gfp);
112113 if (ret < 0)
113114 goto add_error;
114115
mm/readahead.c+4−4
@@ -89,8 +89,8 @@ int read_cache_pages(struct address_space *mapping, struct list_head *pages, · 88 unmodified lines
8989 while (!list_empty(pages)) {
9090 page = list_to_page(pages);
9191 list_del(&page->lru);
92− if (add_to_page_cache_lru(page, mapping,
93− page->index, GFP_KERNEL)) {
92+ if (add_to_page_cache_lru(page, mapping, page->index,
93+ GFP_KERNEL & mapping_gfp_mask(mapping))) {
9494 read_cache_pages_invalidate_page(mapping, page);
9595 continue;
9696 }
@@ -127,8 +127,8 @@ static int read_pages(struct address_space *mapping, struct file *filp, · 30 unmodified lines
127127 for (page_idx = 0; page_idx < nr_pages; page_idx++) {
128128 struct page *page = list_to_page(pages);
129129 list_del(&page->lru);
130− if (!add_to_page_cache_lru(page, mapping,
131− page->index, GFP_KERNEL)) {
130+ if (!add_to_page_cache_lru(page, mapping, page->index,
131+ GFP_KERNEL & mapping_gfp_mask(mapping))) {
132132 mapping->a_ops->readpage(filp, page);
133133 }
134134 page_cache_release(page);

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t