On Mon, Jun 28, 2021 at 08:02:14AM +0800, Shiyang Ruan wrote:
> +/*
> + * dax_load_pfn - Load pfn of the DAX entry corresponding to a page
> + * @mapping: The file whose entry we want to load
> + * @index: offset where the DAX entry located in
> + *
> + * Return: pfn number of the DAX entry
> + */
This is an externally visible function; why not add the second '*' and
make this kernel-doc?
> +unsigned long dax_load_pfn(struct address_space *mapping, unsigned long index)
> +{
> + XA_STATE(xas, &mapping->i_pages, index);
> + void *entry;
> + unsigned long pfn;
> +
> + xas_lock_irq(&xas);
> + entry = xas_load(&xas);
> + pfn = dax_to_pfn(entry);
> + xas_unlock_irq(&xas);
Why do you need the i_pages lock to do this? is the rcu_read_lock()
insufficient? For that matter, why use the xas functions? Why not
simply:
void *entry = xa_load(&mapping->i_pages, index);
return dax_to_pfn(entry);
Looking at it more though, how do you know this is a PFN entry?
It could be locked, for example. Or the zero page, or empty.
But I think this is unnecessary; why not just pass the PFN into
mf_dax_kill_procs?
