Re: [PATCH v7 07/22] Replace the XIP page fault handler with the DAX page fault handler

[Jan Kara]

On Wed 09-04-14 16:51:11, Matthew Wilcox wrote:
> On Wed, Apr 09, 2014 at 12:27:58PM +0200, Jan Kara wrote:
> > > +	if (unlikely(vmf->pgoff >= size)) {
> > > +		mutex_unlock(&mapping->i_mmap_mutex);
> > > +		goto sigbus;
> >   You need to release the block you've got from the filesystem in case of
> > error here an below.
> 
> What's the API to do that?  Call inode->i_op->setattr()?
  That's a great question. Yes, ->setattr() is the only API you have for
that but you cannot use that because of locking constraints (it needs
i_mutex and that's not possible to get in the fault path). Let me read
again what the handler does...

So there are three places that can fail after we allocate the block:
1) We race with truncate reducing i_size
2) dax_get_pfn() fails
3) vm_insert_mixed() fails

I would guess that 2) can fail only if the HW has problems and leaking
block in that case could be acceptable (please correct me if I'm wrong).
3) shouldn't fail because of ENOMEM because fault has already allocated all
the page tables and EBUSY should be handled as well. So the only failure we
have to care about is 1). And we could move ->get_block() call under
i_mmap_mutex after the i_size check.  Lock ordering should be fine because
i_mmap_mutex ranks above page lock under which we do block mapping in
standard ->page_mkwrite callbacks. The only (big) drawback is that
i_mmap_mutex will now be held for much longer time and thus the contention
would be much higher. But hopefully once we resolve our problems with
mmap_sem and introduce mapping range lock we could scale reasonably.

								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/