On Thu 10-04-14 10:16:30, Matthew Wilcox wrote:
> On Wed, Apr 09, 2014 at 11:46:44AM +0200, Jan Kara wrote:
> > Another day, some more review ;) Comments below.
>
> I'm really grateful for all this review! It's killing me, though ;-)
Yeah, I know that feeling. :)
> > > +int dax_clear_blocks(struct inode *inode, sector_t block, long size)
> > > +{
> > > + struct block_device *bdev = inode->i_sb->s_bdev;
> > > + const struct block_device_operations *ops = bdev->bd_disk->fops;
> > > + sector_t sector = block << (inode->i_blkbits - 9);
> > > + unsigned long pfn;
> > > +
> > > + might_sleep();
> > > + do {
> > > + void *addr;
> > > + long count = ops->direct_access(bdev, sector, &addr, &pfn,
> > > + size);
> > So do you assume blocksize == PAGE_SIZE here? If not, addr could be in
> > the middle of the page AFAICT.
>
> You're right. Depending on how clear_page() is implemented, that
> might go badly wrong. Of course, both ext2 & ext4 require block_size
> == PAGE_SIZE right now, so anything else is by definition untested.
> I've been trying to keep DAX free from that assumption, but obviously
> haven't caught all the places.
>
> How does this look?
That looks fine.
Honza
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>