From: Carlos Maiolino <cmaiolino@redhat.com>
To: linux-fsdevel@vger.kernel.org
Cc: hch@lst.de, sandeen@redhat.com, david@fromorbit.com
Subject: [PATCH 1/3] fs: Enable bmap() function to properly return errors
Date: Wed, 12 Sep 2018 14:25:34 +0200 [thread overview]
Message-ID: <20180912122536.31977-2-cmaiolino@redhat.com> (raw)
In-Reply-To: <20180912122536.31977-1-cmaiolino@redhat.com>
By now, bmap() will either return the physical block number related to
the requested file offset or 0 in case of error or the requested offset
maps into a hole.
This patch makes the needed changes to enable bmap() to proper return
errors, using the return value as an error return, and now, a pointer
must be passed to bmap() to be filled with the mapped physical block.
It will change the behavior of bmap() on return:
- negative value in case of error
- zero on success or if map fell into a hole
In case of a hole, the *block will be zero too
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
P.S. Since this is a prep patch, by now, the only error return is -EINVAL if
->bmap doesn't exist.
drivers/md/md-bitmap.c | 16 ++++++++++------
fs/inode.c | 30 +++++++++++++++++-------------
fs/jbd2/journal.c | 22 +++++++++++++++-------
include/linux/fs.h | 2 +-
mm/page_io.c | 11 +++++++----
5 files changed, 50 insertions(+), 31 deletions(-)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 2fc8c113977f..0bbd55f45b25 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -363,7 +363,7 @@ static int read_page(struct file *file, unsigned long index,
int ret = 0;
struct inode *inode = file_inode(file);
struct buffer_head *bh;
- sector_t block;
+ sector_t block, blk_cur;
pr_debug("read bitmap file (%dB @ %llu)\n", (int)PAGE_SIZE,
(unsigned long long)index << PAGE_SHIFT);
@@ -374,17 +374,21 @@ static int read_page(struct file *file, unsigned long index,
goto out;
}
attach_page_buffers(page, bh);
- block = index << (PAGE_SHIFT - inode->i_blkbits);
+ blk_cur = index << (PAGE_SHIFT - inode->i_blkbits);
while (bh) {
+ block = blk_cur;
+
if (count == 0)
bh->b_blocknr = 0;
else {
- bh->b_blocknr = bmap(inode, block);
- if (bh->b_blocknr == 0) {
- /* Cannot use this file! */
+ ret = bmap(inode, &block);
+ if (ret || !block) {
ret = -EINVAL;
+ bh->b_blocknr = 0;
goto out;
}
+
+ bh->b_blocknr = block;
bh->b_bdev = inode->i_sb->s_bdev;
if (count < (1<<inode->i_blkbits))
count = 0;
@@ -398,7 +402,7 @@ static int read_page(struct file *file, unsigned long index,
set_buffer_mapped(bh);
submit_bh(REQ_OP_READ, 0, bh);
}
- block++;
+ blk_cur++;
bh = bh->b_this_page;
}
page->index = index;
diff --git a/fs/inode.c b/fs/inode.c
index 41812a3e829e..99154ce81cd9 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1577,21 +1577,25 @@ EXPORT_SYMBOL(iput);
/**
* bmap - find a block number in a file
- * @inode: inode of file
- * @block: block to find
- *
- * Returns the block number on the device holding the inode that
- * is the disk block number for the block of the file requested.
- * That is, asked for block 4 of inode 1 the function will return the
- * disk block relative to the disk start that holds that block of the
- * file.
+ * @inode: inode owning the block number being requested
+ * @*block: pointer containing the block to find
+ *
+ * Replaces the value in *block with the block number on the device holding
+ * corresponding to the requested block number in the file.
+ * That is, asked for block 4 of inode 1 the function will replace the
+ * 4 in *block, with disk block relative to the disk start that holds that
+ * block of the file.
+ *
+ * Returns -EINVAL in case of error, 0 otherwise. If mapping falls into a
+ * hole, returns 0 and *block is also set to 0.
*/
-sector_t bmap(struct inode *inode, sector_t block)
+int bmap(struct inode *inode, sector_t *block)
{
- sector_t res = 0;
- if (inode->i_mapping->a_ops->bmap)
- res = inode->i_mapping->a_ops->bmap(inode->i_mapping, block);
- return res;
+ if (!inode->i_mapping->a_ops->bmap)
+ return -EINVAL;
+
+ *block = inode->i_mapping->a_ops->bmap(inode->i_mapping, *block);
+ return 0;
}
EXPORT_SYMBOL(bmap);
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 8ef6b6daaa7a..7acaf6f55404 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -814,18 +814,23 @@ int jbd2_journal_bmap(journal_t *journal, unsigned long blocknr,
{
int err = 0;
unsigned long long ret;
+ sector_t block = 0;
if (journal->j_inode) {
- ret = bmap(journal->j_inode, blocknr);
- if (ret)
- *retp = ret;
- else {
+ block = blocknr;
+ ret = bmap(journal->j_inode, &block);
+
+ if (ret || !block) {
printk(KERN_ALERT "%s: journal block not found "
"at offset %lu on %s\n",
__func__, blocknr, journal->j_devname);
err = -EIO;
__journal_abort_soft(journal, err);
+
+ } else {
+ *retp = block;
}
+
} else {
*retp = blocknr; /* +journal->j_blk_offset */
}
@@ -1251,11 +1256,14 @@ journal_t *jbd2_journal_init_dev(struct block_device *bdev,
journal_t *jbd2_journal_init_inode(struct inode *inode)
{
journal_t *journal;
+ sector_t blocknr;
char *p;
- unsigned long long blocknr;
+ int err = 0;
+
+ blocknr = 0;
+ err = bmap(inode, &blocknr);
- blocknr = bmap(inode, 0);
- if (!blocknr) {
+ if (err || !blocknr) {
pr_err("%s: Cannot locate journal superblock\n",
__func__);
return NULL;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index bf0cad65b9b7..15a79f67ad87 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2762,7 +2762,7 @@ static inline ssize_t generic_write_sync(struct kiocb *iocb, ssize_t count)
extern void emergency_sync(void);
extern void emergency_remount(void);
#ifdef CONFIG_BLOCK
-extern sector_t bmap(struct inode *, sector_t);
+extern int bmap(struct inode *, sector_t *);
#endif
extern int notify_change(struct dentry *, struct iattr *, struct inode **);
extern int inode_permission(struct inode *, int);
diff --git a/mm/page_io.c b/mm/page_io.c
index aafd19ec1db4..994c996414c3 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -177,8 +177,9 @@ int generic_swapfile_activate(struct swap_info_struct *sis,
cond_resched();
- first_block = bmap(inode, probe_block);
- if (first_block == 0)
+ first_block = probe_block;
+ ret = bmap(inode, &first_block);
+ if (ret || !first_block)
goto bad_bmap;
/*
@@ -193,9 +194,11 @@ int generic_swapfile_activate(struct swap_info_struct *sis,
block_in_page++) {
sector_t block;
- block = bmap(inode, probe_block + block_in_page);
- if (block == 0)
+ block = probe_block + block_in_page;
+ ret = bmap(inode, &block);
+ if (ret || !block)
goto bad_bmap;
+
if (block != first_block + block_in_page) {
/* Discontiguity */
probe_block++;
--
2.14.4
next prev parent reply other threads:[~2018-09-12 17:30 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-12 12:25 [PATCH 0/3] Replace direct ->bmap calls by bmap() with error support Carlos Maiolino
2018-09-12 12:25 ` Carlos Maiolino [this message]
2018-09-14 13:23 ` [PATCH 1/3] fs: Enable bmap() function to properly return errors Christoph Hellwig
2018-09-14 18:48 ` Carlos Maiolino
2018-09-17 20:55 ` Darrick J. Wong
2018-09-18 6:00 ` Carlos Maiolino
2018-09-12 12:25 ` [PATCH 2/3] cachefiles: drop direct usage of ->bmap method Carlos Maiolino
2018-09-14 13:23 ` Christoph Hellwig
2018-09-14 18:56 ` Carlos Maiolino
2018-09-12 12:25 ` [PATCH 3/3] ecryptfs: drop direct calls to ->bmap Carlos Maiolino
2018-09-14 13:26 ` Christoph Hellwig
2018-09-14 18:47 ` Carlos Maiolino
2018-09-17 11:04 ` [PATCH 3/3 V2] " Carlos Maiolino
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=20180912122536.31977-2-cmaiolino@redhat.com \
--to=cmaiolino@redhat.com \
--cc=david@fromorbit.com \
--cc=hch@lst.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=sandeen@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).