On Wed 13-01-16 00:08:29, Ross Zwisler wrote:
On Tue, Jan 12, 2016 at 10:34:58AM +0100, Jan Kara wrote:
> On Thu 07-01-16 22:27:51, Ross Zwisler wrote:
> > In __dax_pmd_fault() we currently assume that get_block() will always set
> > bh.b_bdev and we unconditionally dereference it in __dax_dbg(). This
> > assumption isn't always true - when called for reads of holes
> > ext4_dax_mmap_get_block() returns a buffer head where bh->b_bdev is never
> > set. I hit this BUG while testing the DAX PMD fault path.
> > Instead, initialize bh.b_bdev before passing bh into get_block(). It is
> > possible that the filesystem's get_block() will update bh.b_bdev, and this
> > is fine - we just want to initialize bh.b_bdev to something reasonable so
> > that the calls to __dax_dbg() work and print something useful.
> > Signed-off-by: Ross Zwisler <ross.zwisler(a)linux.intel.com>
> > Cc: Dan Williams <dan.j.williams(a)intel.com>
> Looks good. But don't you need to do the same for __dax_fault(),
> dax_zero_page_range() and similar places passing bh to dax functions?
I don't think we need it anywhere else. The only reason that we need to
initialize the bh.b_bdev manually in the __dax_pmd_fault() path is that if the
get_block() call ends up finding a hole (so doesn't fill out b_bdev) we still
go through the dax_pmd_dbg() path to print an error message, which uses
b_bdev. I believe that in the other paths where we hit a hole, such as
__dax_fault(), we don't use b_bdev because we don't have the same error path
prints, and the regular code for handling holes doesn't use b_bdev.
That being said, if you feel like it's cleaner to initialize it
everywhere so everything is consistent and we don't have to worry about
it, I'm fine to make the change.
Well, it seems more futureproof to me. In case someone decides to add some
debug message later on...
Jan Kara <jack(a)suse.com>
SUSE Labs, CR