[bug report] btrfs: subpage: only call btrfs_alloc_subpage() when sectorsize is smaller than PAGE_SIZE

[Dan Carpenter <dan.carpenter@oracle.com>]

Hello Qu Wenruo,

The patch 4c1e934ee490: "btrfs: subpage: only call
btrfs_alloc_subpage() when sectorsize is smaller than PAGE_SIZE" from
Aug 17, 2021, leads to the following
Smatch static checker warning:

	fs/btrfs/subpage.c:110 btrfs_attach_subpage()
	warn: sleeping in atomic context

fs/btrfs/subpage.c
    94 int btrfs_attach_subpage(const struct btrfs_fs_info *fs_info,
    95                          struct page *page, enum btrfs_subpage_type type)
    96 {
    97         struct btrfs_subpage *subpage;
    98 
    99         /*
    100          * We have cases like a dummy extent buffer page, which is not mappped
    101          * and doesn't need to be locked.
    102          */
    103         if (page->mapping)
    104                 ASSERT(PageLocked(page));
    105 
    106         /* Either not subpage, or the page already has private attached */
    107         if (fs_info->sectorsize == PAGE_SIZE || PagePrivate(page))
    108                 return 0;
    109 
--> 110         subpage = btrfs_alloc_subpage(fs_info, type);
    111         if (IS_ERR(subpage))
    112                 return  PTR_ERR(subpage);
    113 
    114         attach_page_private(page, subpage);
    115         return 0;
    116 }

The call tree is:

alloc_extent_buffer() <- disables preempt
-> attach_extent_buffer_page()
   -> btrfs_attach_subpage()

fs/btrfs/extent_io.c
  6132          for (i = 0; i < num_pages; i++, index++) {
  6133                  struct btrfs_subpage *prealloc = NULL;
  6134  
  6135                  p = find_or_create_page(mapping, index, GFP_NOFS|__GFP_NOFAIL);
  6136                  if (!p) {
  6137                          exists = ERR_PTR(-ENOMEM);
  6138                          goto free_eb;
  6139                  }
  6140  
  6141                  /*
  6142                   * Preallocate page->private for subpage case, so that we won't
  6143                   * allocate memory with private_lock hold.  The memory will be
  6144                   * freed by attach_extent_buffer_page() or freed manually if
  6145                   * we exit earlier.
  6146                   *
  6147                   * Although we have ensured one subpage eb can only have one
  6148                   * page, but it may change in the future for 16K page size
  6149                   * support, so we still preallocate the memory in the loop.
  6150                   */
  6151                  if (fs_info->sectorsize < PAGE_SIZE) {

The patch adds this check which means we only preallocate it when it's
small.

  6152                          prealloc = btrfs_alloc_subpage(fs_info, BTRFS_SUBPAGE_METADATA);
  6153                          if (IS_ERR(prealloc)) {
  6154                                  ret = PTR_ERR(prealloc);
  6155                                  unlock_page(p);
  6156                                  put_page(p);
  6157                                  exists = ERR_PTR(ret);
  6158                                  goto free_eb;
  6159                          }
  6160                  }
  6161  
  6162                  spin_lock(&mapping->private_lock);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Take a spinlock.

  6163                  exists = grab_extent_buffer(fs_info, p);
  6164                  if (exists) {
  6165                          spin_unlock(&mapping->private_lock);
  6166                          unlock_page(p);
  6167                          put_page(p);
  6168                          mark_extent_buffer_accessed(exists, p);
  6169                          btrfs_free_subpage(prealloc);
  6170                          goto free_eb;
  6171                  }
  6172                  /* Should not fail, as we have preallocated the memory */
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This comment is out of date.


  6173                  ret = attach_extent_buffer_page(eb, p, prealloc);
                                                               ^^^^^^^^
If we don't preallocate it, then it leads to a sleeping while holding
a spinlock bug.

  6174                  ASSERT(!ret);
  6175                  /*
  6176                   * To inform we have extra eb under allocation, so that

regards,
dan carpenter