lkml.org 
[lkml]   [2014]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v7 11/22] Replace ext2_clear_xip_target with dax_clear_blocks
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@suse.cz>
SUSE Labs, CR


\
 
 \ /
  Last update: 2014-04-10 21:01    [W:0.073 / U:1.748 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site