Re: [PATCH v7 11/22] Replace ext2_clear_xip_target with dax_clear_blocks

[Jan Kara]

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/