Messages in this thread | | | Date | Mon, 28 Jun 2021 12:49:41 +0100 | From | Matthew Wilcox <> | Subject | Re: [PATCH v5 5/9] mm: Introduce mf_dax_kill_procs() for fsdax case |
| |
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?
| |