Re: [PATCH v5 11/11] xfs, dax: introduce xfs_break_dax_layouts()

From: Christoph Hellwig
Date: Sat Mar 10 2018 - 04:56:05 EST


> +static int xfs_wait_dax_page(
> + atomic_t *count,
> + unsigned int mode)
> +{

Normal XFS style would be:

static int
xfs_wait_dax_page(
atomic_t *count,
unsigned int mode)
{

> + struct page *page = refcount_to_page(count);
> + struct address_space *mapping = page->mapping;
> + struct inode *inode = mapping->host;
> + struct xfs_inode *ip = XFS_I(inode);

Looks we don't really need the mapping and inode variables:

struct page *page = refcount_to_page(count);
struct xfs_inode *ip = XFS_I(page->mapping->host);

> + do {
> + if (flags & XFS_BREAK_REMOTE)
> + ret = xfs_break_leased_layouts(inode, iolock);
> + if (ret)
> + return ret;
> + if (flags & XFS_BREAK_MAPS)
> + ret = xfs_break_dax_layouts(inode, *iolock);
> + /*
> + * EBUSY indicates that we dropped locks and waited for
> + * the dax layout to be released. When that happens we
> + * need to revalidate that no new leases or pinned dax
> + * mappings have been established.
> + */
> + } while (ret == -EBUSY);

Maybe instead of the flags argument this should be a type argument
of something like

enum layout_break_reason {
BREAK_WRITE, /* write to file */
BREAK_TRUNCATE, /* truncate or hole punch */
};

as that makes the intent more clear?