All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: mfo@canonical.com
Cc: linux-ext4@vger.kernel.org
Subject: [bug report] ext4: data=journal: fixes for ext4_page_mkwrite()
Date: Fri, 30 Oct 2020 14:47:37 +0300	[thread overview]
Message-ID: <20201030114737.GC3251003@mwanda> (raw)

Hello Mauricio Faria de Oliveira,

The patch 64a9f1449950: "ext4: data=journal: fixes for
ext4_page_mkwrite()" from Oct 5, 2020, leads to the following static
checker warning:

	fs/ext4/inode.c:6136 ext4_page_mkwrite()
	error: uninitialized symbol 'get_block'.

fs/ext4/inode.c
  6040  vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf)
  6041  {
  6042          struct vm_area_struct *vma = vmf->vma;
  6043          struct page *page = vmf->page;
  6044          loff_t size;
  6045          unsigned long len;
  6046          int err;
  6047          vm_fault_t ret;
  6048          struct file *file = vma->vm_file;
  6049          struct inode *inode = file_inode(file);
  6050          struct address_space *mapping = inode->i_mapping;
  6051          handle_t *handle;
  6052          get_block_t *get_block;
                ^^^^^^^^^^^^^^^^^^^^^^

  6053          int retries = 0;
  6054  
  6055          if (unlikely(IS_IMMUTABLE(inode)))
  6056                  return VM_FAULT_SIGBUS;
  6057  
  6058          sb_start_pagefault(inode->i_sb);
  6059          file_update_time(vma->vm_file);
  6060  
  6061          down_read(&EXT4_I(inode)->i_mmap_sem);
  6062  
  6063          err = ext4_convert_inline_data(inode);
  6064          if (err)
  6065                  goto out_ret;
  6066  
  6067          /*
  6068           * On data journalling we skip straight to the transaction handle:
  6069           * there's no delalloc; page truncated will be checked later; the
  6070           * early return w/ all buffers mapped (calculates size/len) can't
  6071           * be used; and there's no dioread_nolock, so only ext4_get_block.
  6072           */
  6073          if (ext4_should_journal_data(inode))
  6074                  goto retry_alloc;
                        ^^^^^^^^^^^^^^^^
This goto is new.

  6075  
  6076          /* Delalloc case is easy... */
  6077          if (test_opt(inode->i_sb, DELALLOC) &&
  6078              !ext4_nonda_switch(inode->i_sb)) {
  6079                  do {
  6080                          err = block_page_mkwrite(vma, vmf,
  6081                                                     ext4_da_get_block_prep);
  6082                  } while (err == -ENOSPC &&
  6083                         ext4_should_retry_alloc(inode->i_sb, &retries));
  6084                  goto out_ret;
  6085          }
  6086  
  6087          lock_page(page);
  6088          size = i_size_read(inode);
  6089          /* Page got truncated from under us? */
  6090          if (page->mapping != mapping || page_offset(page) > size) {
  6091                  unlock_page(page);
  6092                  ret = VM_FAULT_NOPAGE;
  6093                  goto out;
  6094          }
  6095  
  6096          if (page->index == size >> PAGE_SHIFT)
  6097                  len = size & ~PAGE_MASK;
  6098          else
  6099                  len = PAGE_SIZE;
  6100          /*
  6101           * Return if we have all the buffers mapped. This avoids the need to do
  6102           * journal_start/journal_stop which can block and take a long time
  6103           *
  6104           * This cannot be done for data journalling, as we have to add the
  6105           * inode to the transaction's list to writeprotect pages on commit.
  6106           */
  6107          if (page_has_buffers(page)) {
  6108                  if (!ext4_walk_page_buffers(NULL, page_buffers(page),
  6109                                              0, len, NULL,
  6110                                              ext4_bh_unmapped)) {
  6111                          /* Wait so that we don't change page under IO */
  6112                          wait_for_stable_page(page);
  6113                          ret = VM_FAULT_LOCKED;
  6114                          goto out;
  6115                  }
  6116          }
  6117          unlock_page(page);
  6118          /* OK, we need to fill the hole... */
  6119          if (ext4_should_dioread_nolock(inode))
  6120                  get_block = ext4_get_block_unwritten;
  6121          else
  6122                  get_block = ext4_get_block;
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^
Initialized here.

  6123  retry_alloc:
  6124          handle = ext4_journal_start(inode, EXT4_HT_WRITE_PAGE,
  6125                                      ext4_writepage_trans_blocks(inode));
  6126          if (IS_ERR(handle)) {
  6127                  ret = VM_FAULT_SIGBUS;
  6128                  goto out;
  6129          }
  6130          /*
  6131           * Data journalling can't use block_page_mkwrite() because it
  6132           * will set_buffer_dirty() before do_journal_get_write_access()
  6133           * thus might hit warning messages for dirty metadata buffers.
  6134           */
  6135          if (!ext4_should_journal_data(inode)) {
  6136                  err = block_page_mkwrite(vma, vmf, get_block);
                                                           ^^^^^^^^^
Smatch warning here.

  6137          } else {
  6138                  lock_page(page);
  6139                  size = i_size_read(inode);
  6140                  /* Page got truncated from under us? */
  6141                  if (page->mapping != mapping || page_offset(page) > size) {
  6142                          ret = VM_FAULT_NOPAGE;
  6143                          goto out_error;

regards,
dan carpenter

             reply	other threads:[~2020-10-30 11:47 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-30 11:47 Dan Carpenter [this message]
2020-10-30 17:29 ` [bug report] ext4: data=journal: fixes for ext4_page_mkwrite() Mauricio Faria de Oliveira

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201030114737.GC3251003@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=mfo@canonical.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.