Hi Darrick
> > - 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);
>
> Hm. Could we change the aops ->bmap interface to return negative error
> codes too?
This could be done, yes, but, the goal here, is to get rid of ->bmap()
interface, so, this will (hopefully) soon be gone, so I don't see any reason to
change it by now. Could be done, yes, I am just not sure if it's worth, once the
main goal is to remove ->bmap calls and replace it by ->fiemap().
>
> Also... ioctl_fibmap blindly copies the bottom 32 bits of *block out to
> userspace without checking for overflows. Shouldn't that also be
> changed?
>
Eric suggested it, and this is in my road map to do, I just didn't want to
attempt implementing it in this patchset, which I aimed just to pave the road
for the next changes I'm working on.
Cheers.
> --D
>
> > + 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
> >
--
Carlos
