linux-kernel - Re: [PATCH v8 6/9] mm: Introduce mf_dax_kill

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Ybi8icn3W7vOEQV+@infradead.org>
Date:   Tue, 14 Dec 2021 07:47:21 -0800
From:   Christoph Hellwig <hch@...radead.org>
To:     Shiyang Ruan <ruansy.fnst@...itsu.com>
Cc:     linux-kernel@...r.kernel.org, linux-xfs@...r.kernel.org,
        nvdimm@...ts.linux.dev, linux-mm@...ck.org,
        linux-fsdevel@...r.kernel.org, djwong@...nel.org,
        dan.j.williams@...el.com, david@...morbit.com, hch@...radead.org,
        jane.chu@...cle.com
Subject: Re: [PATCH v8 6/9] mm: Introduce mf_dax_kill_procs() for fsdax case

On Thu, Dec 02, 2021 at 04:48:53PM +0800, Shiyang Ruan wrote:
> @@ -254,6 +254,15 @@ static inline bool dax_mapping(struct address_space *mapping)
>  {
>  	return mapping->host && IS_DAX(mapping->host);
>  }
> +static inline unsigned long pgoff_address(pgoff_t pgoff,
> +		struct vm_area_struct *vma)

Empty lines between functions please.

Also this name is a bit generic for something in dax.h, but then again
it does not seem to be DAX-specific, so it might want to move into
a generic MM header with a proper name and kerneldoc comment.

I think a calling conventions that puts the vma before the pgoff would
seem a little more logical as well.

Last but not least such a move should be in a separate patch.

> +extern int mf_dax_kill_procs(struct address_space *mapping, pgoff_t index,
> +			     unsigned long count, int mf_flags);

No need for the extern here.

> -static unsigned long dev_pagemap_mapping_shift(struct page *page,
> +static unsigned long dev_pagemap_mapping_shift(unsigned long address,
>  		struct vm_area_struct *vma)

Passing the vma first would seem more logical again.

> +	if (is_zone_device_page(p)) {
> +		/*
> +		 * Since page->mapping is no more used for fsdax, we should
> +		 * calculate the address in a fsdax way.
> +		 */

		/*
		 * Since page->mapping is not used for fsdax, we need
		 * calculate the address based on the vma.
		 */

> +static void collect_procs_fsdax(struct page *page, struct address_space *mapping,
> +		pgoff_t pgoff, struct list_head *to_kill)

Overly long line here.