Re: [PATCH v5 5/9] mm: Introduce mf_dax_kill_procs() for fsdax case

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel




[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux