On Sun, Nov 8, 2020 at 5:50 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Sun, Nov 08, 2020 at 05:33:22PM -0800, Darrick J. Wong wrote:
> > On Sun, Nov 08, 2020 at 05:15:55PM -0800, Amy Parker wrote:
> > > I've been writing a patch to migrate the defined DAX_ZERO_PAGE
> > > to XA_ZERO_ENTRY for representing holes in files.
> >
> > Why? IIRC XA_ZERO_ENTRY ("no mapping in the address space") isn't the
> > same as DAX_ZERO_PAGE ("the zero page is mapped into the address space
> > because we took a read fault on a sparse file hole").
>
> There's no current user of XA_ZERO_ENTRY in i_pages, whether it be
> DAX or non-DAX.
>
> > > XA_ZERO_ENTRY
> > > is defined in include/linux/xarray.h, where it's defined using
> > > xa_mk_internal(257). This function returns a void pointer, which
> > > is incompatible with the bitwise arithmetic it is performed on with.
>
> We don't really perform bitwise arithmetic on it, outside of:
>
> static int dax_is_zero_entry(void *entry)
> {
> return xa_to_value(entry) & DAX_ZERO_PAGE;
> }
We also have:
if (dax_is_zero_entry(entry) && !(flags & DAX_ZERO_PAGE)) {
unsigned long index = xas->xa_index;
/* we are replacing a zero page with block mapping */
if (dax_is_pmd_entry(entry))
unmap_mapping_pages(mapping, index & ~PG_PMD_COLOUR,
PG_PMD_NR, false);
else /* pte entry */
unmap_mapping_pages(mapping, index, 1, false);
}
and:
*entry = dax_insert_entry(xas, mapping, vmf, *entry, pfn,
DAX_PMD | DAX_ZERO_PAGE, false);
>
> > > Currently, DAX_ZERO_PAGE is defined as an unsigned long,
> > > so I considered typecasting it. Typecasting every time would be
> > > repetitive and inefficient. I thought about making a new definition
> > > for it which has the typecast, but this breaks the original point of
> > > using already defined terms.
> > >
> > > Should we go the route of adding a new definition, we might as
> > > well just change the definition of DAX_ZERO_PAGE. This would
> > > break the simplicity of the current DAX bit definitions:
> > >
> > > #define DAX_LOCKED (1UL << 0)
> > > #define DAX_PMD (1UL << 1)
> > > #define DAX_ZERO_PAGE (1UL << 2)
> > > #define DAX_EMPTY (1UL << 3)
>
> I was proposing deleting the entire bit and shifting DAX_EMPTY down.
That'd probably be a better idea - so what should we do about the type
issue? Not typecasting it causes it not to compile.
